Commit graph

2 commits

Author SHA1 Message Date
varutasu
0d14278021
refactor(glass): migrate Button.secondary + Input + MobileNav off bespoke glass-surface (#131)
Closes the migrate-button-input-mobilenav-to-glass-primitive convoy
(seeded by PR #127). All 3 residual handrolled var(--glass-surface-*)
inline-style usages migrated to either purpose-built utility classes
or the <GlassSurface> primitive. CI allowlist reduced from 6 entries
to 3 (chrome only).

Architect decisions (D1-D3, ratified):

D1 — Button.secondary → new .btn-glass-secondary utility class.
  NOT <GlassSurface>: the primitive sets `background` inline via
  composedStyle, which CSS :hover rules can't override without
  !important. The new class composes the same high-tint
  gradient-border that .glass-panel-strong uses, plus a pure-CSS
  :hover swap (high → mid fill on the padding-box layer).
  Identical visual contract; the hover behavior is now driven by
  CSS, not Tailwind's `hover:bg-[var(...)]` arbitrary class.

D2 — Input → new .glass-input utility class.
  NOT <GlassSurface as="input"> and NOT <GlassSurface as="div"> wrap.
  Reason: <GlassSurface>'s gradient-border trick requires
  `border: 1px solid transparent` to expose the border-box layers,
  which conflicts with <Input>'s conditional error-state
  `1px solid #dc2626` red border. The new class adopts only the
  tint + blur layer; the visible 1px border + focus ring stay in
  JSX (class-controlled, not inline). Same visual contract as
  before for both normal AND error states.

D3 — MobileNavigation → <GlassSurface as="div" tint="mid" blur="mid"
  rim="subtle" elevation="flat" cornerLights="chrome">.
  NOT .page-header-glass (the seed's first recommendation):
  .page-header-glass uses var(--glass-surface-high) (wrong tint —
  MobileNav uses mid) and sets a bottom-border separator (wrong
  for a fixed-bottom-nav where the bottom edge is the viewport
  edge). <GlassSurface> is the better fit AND brings the
  chrome-tier corner-light bleed that the parent convoy is
  unifying across all chrome surfaces.

Implementation choice — single PR (not 3 parallel briefs):
  The seed recommended 3 small parallel-safe briefs (one per file).
  D1 and D2 both need styles/globals.css to gain new utility
  classes, so those 2 changes can't run truly in parallel without
  merge conflicts. Single PR is faster, simpler to review
  end-to-end, and the natural shape for a 2-3 hour convoy with
  tightly-coupled artifacts.

Files changed (4):

styles/globals.css (+50 / -1):
  - Adds .btn-glass-secondary (with :hover variant) — D1.
  - Adds .glass-input — D2.
  - Both classes documented inline with architect-decision references.

components/ui/Button.js (+2 / -10):
  - Replaces inline variantStyle + Tailwind hover arbitrary class
    for `variant === 'secondary'` with `variantClass =
    'btn-glass-secondary font-medium'`. variantStyle now `{}`.
  - Other variants (primary, danger, ghost) UNCHANGED.

components/ui/Input.js (+1 / -7):
  - Adds `glass-input` to the className list.
  - Removes inline `background` + `backdropFilter` +
    `WebkitBackdropFilter` from the input's style block.
  - Conditional `border: inputBorder` stays in JSX (error swap).
  - All other props/behavior preserved.

components/MobileNavigation.js (+11 / -8):
  - Adds `import { GlassSurface } from './ui'`.
  - Replaces the inline-styled backdrop <div> with
    <GlassSurface as="div" ...>. Same className ("absolute inset-0"),
    same visible behavior, plus the chrome-tier corner-light bleed.
  - Comment block updated to reference the convoy + decision.

.github/workflows/ci.yml (+8 / -22):
  - forbidden-patterns Check 7/7 GLASS_ALLOWLIST reduced from 6
    entries to 3 (chrome only). The TODO comments referencing this
    convoy are deleted (work is done).

.convoys/migrate-button-input-mobilenav-to-glass-primitive.md
  (+74 / -3):
  - status: queued → closed, closed: 2026-06-05, prs: [131].
  - Architect ratifications D1-D3 written into front-matter docs.
  - Closeout checklist with all acceptance criteria checked.
  - Note that parent convoy unify-glass-panel-surfaces is now
    fully closed — no residual handrolled glass-surface usage
    outside the 3 chrome blocks.

Verification:
- POSITIVE TEST: post-migration grep with the reduced 3-entry
  allowlist returns 0 violations. 
- grep on raw files: only Layout.js + TopSearchBar.js still match
  the literal regex (GlassSurface.js uses template literal which
  doesn't match — intentional, allowlist is forward-compat).
- YAML parses (python3 yaml.safe_load).
- npm run lint passes (1 pre-existing unrelated warning).
- npm run test:run: 118/118 tests pass.

Visual diff to be verified by reviewer in light + dark mode for:
- <Button variant="secondary"> default + hover state.
- <Input> default + error state.
- Mobile bottom-nav backdrop.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-06-05 06:23:55 -05:00
varutasu
b82fd6cb6a
ci(forbidden-patterns): add Check 7/7 — no bespoke var(--glass-surface-*) (#127)
Brief 7 (final brief) of unify-glass-panel-surfaces convoy. Adds the
regression gate that prevents reintroduction of bespoke
var(--glass-surface-low|mid|high) inline styles outside the
documented allowlist.

IMPLEMENTATION DEVIATION FROM BRIEF (DOCUMENTED):

Brief 7 was authored before PR #126 (slash-ci-minutes convoy)
consolidated the 6 grep-only forbidden-* jobs into a single
forbidden-patterns job with sequential ::group:: sections. Adding
Brief 7 as a standalone forbidden-bespoke-glass-surface job (the
brief's verbatim shape) would partially undo PR #126's checkout
amortization win. Instead, this PR adds the check as Check 7/7
inside the existing forbidden-patterns job — semantics, allowlist,
and grep pattern are exactly as Brief 7 specifies; only the wrapper
changes. Job display name updated: "Forbidden patterns (6 checks)"
→ "Forbidden patterns (7 checks)".

ALLOWLIST EXPANSION (DOCUMENTED):

Brief 7's planned 3-entry allowlist (the 3 chrome blocks) turned out
to undercount the residual surface area. Three additional files
still handroll their own var(--glass-surface-*) inline styles:
- components/ui/Button.js (secondary variant)
- components/ui/Input.js (input wrapper)
- components/MobileNavigation.js (bottom-nav background)

Per Brief 7's own note ("If you need to add a fourth allowlist
entry, that's a design-system decision — open a new convoy"), the
right call is to ship the gate NOW with a 6-entry allowlist
(3 chrome + 3 pending-migration) and track the cleanup in a
follow-up. This PR therefore also seeds
`.convoys/migrate-button-input-mobilenav-to-glass-primitive.md`
with the migration plan, open questions for the architect, and
acceptance criteria. The 3 pending entries are tagged with inline
`# TODO:` comments in ci.yml referencing the follow-up convoy.

Local verification (per Brief 7's pre-merge negative test):
- Positive test: grep on clean main → 0 violations outside the
  6-entry allowlist. 
- Negative test: injected a scratch
  `const scratch = { background: 'var(--glass-surface-low)' }` line
  at EOF of pages/profile.js; grep correctly flagged it. 
- Revert verified: removing the scratch line returns the grep to
  0 violations. 

The scratch change was NOT committed (per Brief 7's instructions).

Convoy closeout:
- `.convoys/unify-glass-panel-surfaces.md` status moved from
  `open` to `closing`; queued_followup field names the new convoy.
  The convoy lands fully when this PR merges.

Verification:
- npm run lint passes (1 pre-existing unrelated warning).
- npm run test:run: 118/118 tests pass.
- YAML parses (python3 yaml.safe_load).

Files:
- .github/workflows/ci.yml: +~70 lines (Check 7/7 step + final-exit
  copy edit).
- .convoys/migrate-button-input-mobilenav-to-glass-primitive.md:
  new file, 113 lines.
- .convoys/unify-glass-panel-surfaces.md: +2 lines (status +
  queued_followup fields).

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-06-04 16:47:26 -05:00