ci(forbidden-patterns): add Check 7/7 — no bespoke var(--glass-surface-*)
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>
This commit is contained in:
parent
a4dca47642
commit
e3ba58139d
3 changed files with 203 additions and 3 deletions
132
.convoys/migrate-button-input-mobilenav-to-glass-primitive.md
Normal file
132
.convoys/migrate-button-input-mobilenav-to-glass-primitive.md
Normal file
|
|
@ -0,0 +1,132 @@
|
||||||
|
---
|
||||||
|
status: queued
|
||||||
|
classification: server-only-no-actually-just-frontend-styles-cleanup
|
||||||
|
parent_convoy: unify-glass-panel-surfaces
|
||||||
|
blocked_by: []
|
||||||
|
size: small
|
||||||
|
budget_hours: 2-3
|
||||||
|
---
|
||||||
|
|
||||||
|
# migrate-button-input-mobilenav-to-glass-primitive
|
||||||
|
|
||||||
|
## Why
|
||||||
|
|
||||||
|
The `unify-glass-panel-surfaces` convoy's Brief 7 added a CI gate that
|
||||||
|
forbids bespoke `var(--glass-surface-*)` inline-style usage outside a
|
||||||
|
documented allowlist. When the gate was being added, three files
|
||||||
|
turned out to still handroll their own glass surfaces and had to be
|
||||||
|
admitted to the allowlist to ship the gate now:
|
||||||
|
|
||||||
|
- `components/ui/Button.js` — the `secondary` variant carries
|
||||||
|
`style={{ background: 'var(--glass-surface-high)', backdropFilter:
|
||||||
|
'...' }}` and a Tailwind arbitrary class
|
||||||
|
`hover:bg-[var(--glass-surface-mid)]`.
|
||||||
|
- `components/ui/Input.js` — the input fill is `style={{ background:
|
||||||
|
'var(--glass-surface-high)', backdropFilter: '...' }}` on the
|
||||||
|
outer wrapper of the input control.
|
||||||
|
- `components/MobileNavigation.js` — the bottom-nav background
|
||||||
|
layer is `style={{ background: 'var(--glass-surface-mid)',
|
||||||
|
backdropFilter: '...' }}`.
|
||||||
|
|
||||||
|
The pattern (inline `background: var(--glass-surface-X)` + inline
|
||||||
|
`backdropFilter`) is exactly what the convoy spent six briefs
|
||||||
|
eliminating elsewhere. These three are the residual.
|
||||||
|
|
||||||
|
## Goal (1 sentence)
|
||||||
|
|
||||||
|
Migrate `components/ui/Button.js`'s `secondary` variant,
|
||||||
|
`components/ui/Input.js`, and `components/MobileNavigation.js` to
|
||||||
|
compose `<GlassSurface>` (with the right `cornerLights` + `blur` +
|
||||||
|
`tint` props from Brief 1) or the appropriate `.glass-panel-*` /
|
||||||
|
`.page-header-glass` class, then **delete** the three entries from
|
||||||
|
the `forbidden-bespoke-glass-surface` allowlist in
|
||||||
|
`.github/workflows/ci.yml` so the gate covers them too.
|
||||||
|
|
||||||
|
## Files in scope
|
||||||
|
|
||||||
|
- `components/ui/Button.js` — the `secondary` variant block only;
|
||||||
|
leave `primary`, `danger`, `ghost`, etc. as-is unless they
|
||||||
|
legitimately need the same migration (they don't today).
|
||||||
|
- `components/ui/Input.js` — the outer wrapper style only.
|
||||||
|
- `components/MobileNavigation.js` — the bottom-nav backdrop layer
|
||||||
|
only.
|
||||||
|
- `.github/workflows/ci.yml` — the `forbidden-bespoke-glass-surface`
|
||||||
|
check (now Check 7/7 of the consolidated `forbidden-patterns`
|
||||||
|
job). Delete the three pending entries from `GLASS_ALLOWLIST`,
|
||||||
|
leaving only the 3 chrome blocks.
|
||||||
|
|
||||||
|
## Open questions for the architect
|
||||||
|
|
||||||
|
1. **`<Button variant="secondary">` — `<GlassSurface>` or class?**
|
||||||
|
The button uses a complex `backdropFilter` + `boxShadow` stack
|
||||||
|
matching `glass-panel-strong`'s look. Composing
|
||||||
|
`<GlassSurface tint="high" blur="low" cornerLights="subtle">`
|
||||||
|
keeps it tokenized and means `cornerLights` ripples in for free.
|
||||||
|
The hover variant (`hover:bg-[var(--glass-surface-mid)]`) needs
|
||||||
|
a different solution — either a `hover` prop on `<GlassSurface>`,
|
||||||
|
or wrap the hover state in a separate utility class. Recommend
|
||||||
|
pulling the hover into a CSS variable swap on the `:hover`
|
||||||
|
pseudo-class of a new utility class (`.glass-surface-hover-shift`
|
||||||
|
or similar), authored in `styles/globals.css`.
|
||||||
|
|
||||||
|
2. **`<Input>` — `<GlassSurface as="div">` wrapping the native
|
||||||
|
`<input>`?** That's the most consistent shape, but the current
|
||||||
|
`<Input>` API takes inline-style props the wrapper would have to
|
||||||
|
forward. Easier alternative: add `.glass-input` utility class to
|
||||||
|
`styles/globals.css` mirroring `.glass-panel-strong`'s shape but
|
||||||
|
with `border-radius: 8px` and the input-specific focus ring.
|
||||||
|
|
||||||
|
3. **`<MobileNavigation>` — `.page-header-glass`?** That class was
|
||||||
|
designed for the desktop top-of-page strip; the bottom-nav has
|
||||||
|
the same "full-bleed translucent chrome" semantics inverted
|
||||||
|
vertically. Either reuse the class (simplest), or introduce a
|
||||||
|
`.glass-bottom-nav` mirror. Recommend reuse since the visual
|
||||||
|
contract is identical aside from vertical anchoring (controlled
|
||||||
|
by the consumer's `<div className="fixed bottom-0 ...">`).
|
||||||
|
|
||||||
|
## Acceptance criteria (draft — architect to ratify)
|
||||||
|
|
||||||
|
- [ ] `Button.js` secondary variant uses `<GlassSurface>` or a
|
||||||
|
documented `.glass-*` class.
|
||||||
|
- [ ] `Input.js` outer wrapper uses `<GlassSurface>` or a
|
||||||
|
documented `.glass-input` class.
|
||||||
|
- [ ] `MobileNavigation.js` bottom-nav backdrop uses
|
||||||
|
`.page-header-glass` (or `.glass-bottom-nav` if the architect
|
||||||
|
decides on a mirror).
|
||||||
|
- [ ] `grep -lE "var\(--glass-surface-(low|mid|high)\)" pages
|
||||||
|
components -r --include='*.js'` returns **only the 3 chrome
|
||||||
|
files** (Layout, TopSearchBar, GlassSurface).
|
||||||
|
- [ ] `.github/workflows/ci.yml`'s `GLASS_ALLOWLIST` is reduced
|
||||||
|
from 6 entries to 3.
|
||||||
|
- [ ] Visual diff against `main` shows no regression in the
|
||||||
|
`secondary` button, the `<Input>` control, or the bottom-nav
|
||||||
|
surface in both themes.
|
||||||
|
- [ ] `npm run lint` + `npm run test:run` both green.
|
||||||
|
|
||||||
|
## Out of scope
|
||||||
|
|
||||||
|
- Other `Button` variants (primary, danger, ghost) — they don't
|
||||||
|
use `var(--glass-surface-*)`.
|
||||||
|
- The `<GlassSurface>` primitive itself — Brief 1 already shipped
|
||||||
|
the `cornerLights` prop; this convoy just adopts it in 3 places.
|
||||||
|
- Any other component the grep doesn't currently flag — if a new
|
||||||
|
file appears in the grep result after this convoy lands, that's
|
||||||
|
a separate convoy (per the `forbidden-bespoke-glass-surface`
|
||||||
|
gate's own friction principle).
|
||||||
|
|
||||||
|
## Pre-work the conductor should verify
|
||||||
|
|
||||||
|
- Brief 1 (`<GlassSurface>` cornerLights prop) has merged. ✅ — PR #123.
|
||||||
|
- Brief 7 (this convoy's parent gate) has merged.
|
||||||
|
- The 3 target files still contain `var(--glass-surface-*)` inline
|
||||||
|
styles (re-run the grep at kickoff).
|
||||||
|
|
||||||
|
## Notes for future agents
|
||||||
|
|
||||||
|
The 3 files are independent — there's no shared abstraction across
|
||||||
|
them. Recommend treating this as 3 small briefs (one per file) the
|
||||||
|
architect can dispatch in parallel after deciding the migration
|
||||||
|
shape per file in the open questions above. If the architect chooses
|
||||||
|
the "add `.glass-input` and `.glass-bottom-nav` mirror utility
|
||||||
|
classes" path, those style additions belong in a 4th brief that
|
||||||
|
ships first.
|
||||||
|
|
@ -13,7 +13,9 @@ success_metric: |
|
||||||
outside the documented exception list.
|
outside the documented exception list.
|
||||||
skip:
|
skip:
|
||||||
- ia
|
- ia
|
||||||
status: open
|
status: closing
|
||||||
|
queued_followup:
|
||||||
|
- migrate-button-input-mobilenav-to-glass-primitive
|
||||||
created: 2026-06-04
|
created: 2026-06-04
|
||||||
depends_on:
|
depends_on:
|
||||||
- tone-down-card-corner-lights # PR #118 — establishes the subtle token tier
|
- tone-down-card-corner-lights # PR #118 — establishes the subtle token tier
|
||||||
|
|
|
||||||
70
.github/workflows/ci.yml
vendored
70
.github/workflows/ci.yml
vendored
|
|
@ -116,7 +116,7 @@ jobs:
|
||||||
# (matches the previous independent-jobs behavior; nicer than
|
# (matches the previous independent-jobs behavior; nicer than
|
||||||
# stop-at-first-failure).
|
# stop-at-first-failure).
|
||||||
forbidden-patterns:
|
forbidden-patterns:
|
||||||
name: Forbidden patterns (6 checks)
|
name: Forbidden patterns (7 checks)
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
steps:
|
steps:
|
||||||
- uses: actions/checkout@v4
|
- uses: actions/checkout@v4
|
||||||
|
|
@ -306,12 +306,78 @@ jobs:
|
||||||
fi
|
fi
|
||||||
echo "::endgroup::"
|
echo "::endgroup::"
|
||||||
|
|
||||||
|
# ---------- (7) No bespoke var(--glass-surface-*) inline styles ----------
|
||||||
|
echo "::group::Check 7/7: No bespoke var(--glass-surface-*) inline styles"
|
||||||
|
# unify-glass-panel-surfaces convoy, Brief 7 (PR sequence after #126).
|
||||||
|
# After Briefs 1-6 land, every panel-shaped surface in the app
|
||||||
|
# composes via .glass-panel / .glass-panel-strong /
|
||||||
|
# .page-header-glass / <GlassSurface>. Bespoke inline-style use
|
||||||
|
# of var(--glass-surface-low|mid|high) under pages/ or
|
||||||
|
# components/ JSX is the regression vector this gate prevents.
|
||||||
|
#
|
||||||
|
# Allowlist (explicit, documented exceptions):
|
||||||
|
# --- Chrome blocks (D4 of architect plan, ratified) ---
|
||||||
|
# - components/ui/GlassSurface.js # the primitive itself
|
||||||
|
# - components/Layout.js # sidebar nav-chip chrome (L850 area)
|
||||||
|
# - components/ui/TopSearchBar.js # <header> chrome
|
||||||
|
# --- Pending migrations (queued follow-up convoy) ---
|
||||||
|
# TODO: migrate-button-input-mobilenav-to-glass-primitive convoy.
|
||||||
|
# The 3 entries below currently handroll their own glass
|
||||||
|
# surfaces — they should compose <GlassSurface> (cornerLights
|
||||||
|
# default 'subtle' from Brief 1) or the appropriate
|
||||||
|
# .glass-panel-* class. They are allowlisted so this gate can
|
||||||
|
# ship NOW and prevent future regressions; the cleanup is
|
||||||
|
# tracked in `.convoys/migrate-button-input-mobilenav-to-glass-primitive.md`.
|
||||||
|
# When that convoy lands, remove the 3 entries below.
|
||||||
|
# - components/ui/Button.js # secondary variant inline + hover:bg-[var(...)]
|
||||||
|
# - components/ui/Input.js # input fill background
|
||||||
|
# - components/MobileNavigation.js # bottom-nav background
|
||||||
|
#
|
||||||
|
# Adding a fourth chrome-tier entry is a design-system decision
|
||||||
|
# — open a new convoy. Removing an entry from the pending-
|
||||||
|
# migration tier requires the matching convoy to ship.
|
||||||
|
GLASS_ALLOWLIST=(
|
||||||
|
"components/ui/GlassSurface.js"
|
||||||
|
"components/Layout.js"
|
||||||
|
"components/ui/TopSearchBar.js"
|
||||||
|
"components/ui/Button.js"
|
||||||
|
"components/ui/Input.js"
|
||||||
|
"components/MobileNavigation.js"
|
||||||
|
)
|
||||||
|
GLASS_FOUND=()
|
||||||
|
while IFS= read -r file; do
|
||||||
|
skip=false
|
||||||
|
for allowed in "${GLASS_ALLOWLIST[@]}"; do
|
||||||
|
if [ "$file" = "$allowed" ]; then
|
||||||
|
skip=true
|
||||||
|
break
|
||||||
|
fi
|
||||||
|
done
|
||||||
|
if [ "$skip" = true ]; then
|
||||||
|
continue
|
||||||
|
fi
|
||||||
|
GLASS_FOUND+=("$file")
|
||||||
|
done < <(grep -lE "var\\(--glass-surface-(low|mid|high)\\)" \
|
||||||
|
pages components -r --include='*.js' 2>/dev/null \
|
||||||
|
| sort -u || true)
|
||||||
|
if [ ${#GLASS_FOUND[@]} -gt 0 ]; then
|
||||||
|
echo "::error::Bespoke var(--glass-surface-*) inline styles detected outside the documented allowlist."
|
||||||
|
echo "Use .glass-panel / .glass-panel-strong / .page-header-glass or compose <GlassSurface> from components/ui/ instead."
|
||||||
|
for f in "${GLASS_FOUND[@]}"; do
|
||||||
|
echo "::error file=${f}::Replace inline var(--glass-surface-*) with the appropriate class or primitive."
|
||||||
|
done
|
||||||
|
FAIL=1
|
||||||
|
else
|
||||||
|
echo "OK: no bespoke glass-surface inline styles outside the allowlist."
|
||||||
|
fi
|
||||||
|
echo "::endgroup::"
|
||||||
|
|
||||||
# ---------- Final exit ----------
|
# ---------- Final exit ----------
|
||||||
if [ "$FAIL" -ne 0 ]; then
|
if [ "$FAIL" -ne 0 ]; then
|
||||||
echo "::error::One or more forbidden-pattern checks failed. See annotations above."
|
echo "::error::One or more forbidden-pattern checks failed. See annotations above."
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
echo "All 6 forbidden-pattern checks passed."
|
echo "All 7 forbidden-pattern checks passed."
|
||||||
|
|
||||||
migrate:
|
migrate:
|
||||||
name: Migrations apply (node-pg-migrate)
|
name: Migrations apply (node-pg-migrate)
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue