From 5a7c905ce4a31e992ef4947a3955a6ab6f39cea0 Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 2 Sep 2026 11:42:30 -0700 Subject: [PATCH 1/9] docs: design and implementer runbook for the SPA elevation and caps pass (#651, #652) --- .../designs/651-652-spa-elevation-and-caps.md | 206 +++++++ .../01-implementer-runbook.md | 563 ++++++++++++++++++ 2 files changed, 769 insertions(+) create mode 100644 docs/designs/651-652-spa-elevation-and-caps.md create mode 100644 docs/plans/651-652-spa-elevation-and-caps/01-implementer-runbook.md diff --git a/docs/designs/651-652-spa-elevation-and-caps.md b/docs/designs/651-652-spa-elevation-and-caps.md new file mode 100644 index 000000000..8f3291bc1 --- /dev/null +++ b/docs/designs/651-652-spa-elevation-and-caps.md @@ -0,0 +1,206 @@ +# #651 + #652 — Elevation hierarchy and the end of tracked all-caps + +**Slice:** #651 (elevation hierarchy) and #652 (retire tracked all-caps labels), shipped as one PR. +**Mode:** feature. **Base:** `396ba233c04a84fa3452e9fd7901e4f2429d4bd7` on `main`. +**Why one slice:** both change `web/src/styles.css` and both add a guard to the same `web/src/styles*.test.ts` +family. Split into two PRs they conflict on every hunk. #650 and #657 shipped together for the same reason. + +## 1. What already shipped — the fact base + +Read at the base commit. **Both issue bodies were written before #654 merged and are stale in ways that +change the work.** + +| Issue claim | Verified state at `396ba23` | +|---|---| +| #651: `.stat` carries `--r-card` + `--shadow-card` | **No `.stat` or `.stat-label` selector exists.** #654 replaced the dashboard tiles with `.capture-tile*` (L1564-1620), which are already border-only, no shadow, sentence case. Its own comment names "#651's direction" and "#652's". | +| #651: `.entry-pane` carries `--shadow-card` | **`.entry-pane` (L757) already has no `box-shadow`** — surface + hairline + `--r-card` only. | +| #652: seven rules set `text-transform: uppercase` | **Six do.** `.stat-label` is gone with `.stat`. | +| #652: "audit `.eyebrow`'s call sites" | **`.eyebrow` has zero call sites.** The only `eyebrow` in the SPA is an i18n *key* (`en.ts:2306`) rendered through `.help-kicker` (`HelpPage.tsx:244`), which #657 already made sentence case. `.help-toc .eyebrow` (L2133) is a descendant rule with nothing to select. | + +So the live surface is smaller than either issue describes, and `.eyebrow` is dead CSS to delete rather +than a call-site audit to perform. + +## 2. Live inventory — every `box-shadow` in the stylesheet + +Walked, not recalled. Eleven declarations: + +| Line | Selector | Value | Verdict | +|---|---|---|---| +| 555 | `.dialog` | `--shadow-dialog` | float — **keep** | +| 1006 | `.entry-foot` | `--shadow-bar` | float (sticky action bar) — **keep** | +| 1061 | `.toolbar` | `--shadow-card` | **remove; go inset** | +| 1310 | `.tabbar` (`@media`) | `--shadow-bar` | float (mobile tab bar) — **keep** | +| 1522 | `.card, .panel, .order-panel` | `--shadow-card` | **remove** | +| 1715 | `.auth .card` | `--auth-card-shadow` | float — a card on the auth gradient, its own token — **keep** | +| 2064 | `.glossary-entry:target` | `0 0 0 0.6rem var(--tint-accent)` | a focus/target ring, not elevation — **keep** | +| 2167 / 2172 | `.help-toc li a.active` | `inset 2px 0 0` | an inset marker bar — **keep** | +| 2676 | `.palette-option:has(input:checked)` | `inset 0 0 0 1px` | an inset selection ring — **keep** | +| 2712 | `.update-banner` | `--shadow-bar` | float — **keep** | +| 2880 | `.named-picker-listbox` | `--shadow-card` | float (a popover over content) — **keep, but re-point at `--shadow-dialog`** | + +**Consequence: `--shadow-card` ends the slice with no consumer.** It is retired — both declarations +(L84 light, L209 dark) deleted. A token with no consumer is precisely the invitation to re-add the +card shadow this slice exists to remove. + +## 3. Live inventory — every `text-transform: uppercase` + +| Line | Selector | Verdict | +|---|---|---| +| 738 | `.step-n` | sentence case, `letter-spacing: 0`; the pill and its tint stay | +| 1102 | `.badge` | sentence case, ~0.78rem, `letter-spacing: 0`; the tinted pill and weight stay | +| 1139 | `.eyebrow` (rule opens 1134) | **delete the rule**, and delete `.help-toc .eyebrow` (L2133) with it — dead | +| 1214 | `.nav-group-label` | **KEEP** — a real structural divider | +| 1352 | `.more-group-label` | **KEEP** — the same divider in the More sheet | +| 1469 | `table.data th` | ~0.8rem, sentence case, weight 600, `color: var(--ink)`, `letter-spacing: 0` | + +## 4. Decisions, and the facts behind them + +**D1 — the toolbar keeps its hairline border.** #651 offers "no border or a hairline". Run the +repo's own `contrast()` from `web/src/test/cssTokens.ts` over `--canvas` against `--surface-2` in every +palette and both modes — not eyeballed hex, the same tool §7 uses: + +| Palette | light | dark | +|---|---|---| +| default | 1.049 | 1.209 | +| forest | 1.054 | 1.210 | +| slate | 1.058 | 1.213 | +| terracotta | 1.074 | 1.187 | + +**No palette separates, in either mode** — dark is marginally better and still nowhere near a usable +edge. So the inset toolbar is `--surface-2` fill **plus** `1px solid var(--hairline)`, no shadow, +`--r-panel` radius. Dropping the border would make the filter bar disappear everywhere, not only on the +light palettes. *(Corrected at Phase 4: the first draft claimed the dark palettes separated cleanly and +reasoned from eyeballed hex values. They do not, and the numbers above are why the border is not +optional in night mode either.)* + +**D2 — the radius scale is 6 / 10 / 16.** `--r-input: 8px` becomes `6px`, `--r-panel: 10px` is new, +`--r-card: 16px` is unchanged. Keeping `--r-input` at 8 would make an 8/10/16 scale whose first two +steps are indistinguishable, which is not a hierarchy. `--r-pill: 999px` is orthogonal and untouched. +Depth mapping: page-level container 16, nested panel / toolbar 10, controls and chips 6. + +**D3 — `--shadow-card` is retired** (see §2). `--shadow-dialog` and `--shadow-bar` stay. + +**D4 — no literal radius values are added.** Twelve literal `border-radius` values already exist +(L688, 1193, 1745, 1761, 1908, 2065, 2208, 2570, 2710, 2775, plus `inherit` and `0`). This slice does +not fix them — that is out of scope — so the guard is scoped to the surfaces this slice touches rather +than pinning a repo-wide count. + +**D5 — `.auth .card` is unaffected.** It sets `--auth-card-shadow` at L1715, later and more specific +than the group rule at L1516, so removing the group's `box-shadow` leaves it intact. + +## 5. Guards — walk everything, exclude deliberately + +Per AGENTS.md's guard rules: neither guard is a hand-maintained list of the selectors the author +happened to think of. Both walk the parsed stylesheet with `postcss` and assert over the whole set, +in the idiom `styles.num.test.ts` and `styles.help.test.ts` already use. + +**G1 — `web/src/styles.elevation.test.ts`** +- Walk every rule. For each `box-shadow`, **split the value on top-level commas and require EVERY + layer to be `inset`-prefixed** before excluding the rule. A prefix test on the whole value is not + enough: `box-shadow: inset 0 0 0 1px var(--hairline), 0 8px 24px rgba(29,21,33,0.12);` starts with + `inset` and still carries a real drop shadow, which is exactly the regression this guard exists to + block. Collect the selector of every rule with at least one non-inset layer, and assert that set + **equals** `{.dialog, .entry-foot, .tabbar, .auth .card, .update-banner, .named-picker-listbox}` plus + `.glossary-entry:target`. + *Deliberate exclusion:* all-inset values are markers and selection rings, not elevation — excluded by + the value, not by name, so a new inset marker does not trip the guard and a new drop shadow does. +- Assert no rule anywhere references `--shadow-card`, and neither `:root` block declares it. +- Assert `.toolbar` declares `background: var(--surface-2)`, a `--hairline` border, `--r-panel`, and + no `box-shadow`. +- Assert `.card, .panel, .order-panel` declares no `box-shadow`. + +**G2 — `web/src/styles.caps.test.ts`** +- Walk every rule. Collect the selector of every `text-transform: uppercase`. Assert that set + **equals** `{.nav-group-label, .more-group-label}`. No exclusions. +- **Recorded limitation, deliberately not closed here:** G2 guards the CSS layer only. Shouty labels + can return without tripping it — a `.toUpperCase()` in a component, an uppercase literal typed into + an i18n string, or `font-variant-caps: all-small-caps` on a new rule. Closing that needs a check on + the i18n bundles and the TSX, which is outside this slice's stated scope (§9). Named here so a later + reader does not mistake a green G2 for a guarantee it never made. +- Assert `table.data th` sets `color: var(--ink)` and `letter-spacing: 0`. + +**G3 — radius scale**, added to `styles.elevation.test.ts` +- Assert `--r-input`, `--r-panel`, `--r-card` are declared and are three distinct values in + increasing order. +- Assert `--r-input` resolves to `6px`, `--r-panel` to `10px`, `--r-card` to `16px`. +- Assert each of `.toolbar`, `.card`, `.panel`, `.order-panel`, `.entry-pane`, `.capture-tile` + resolves its `border-radius` through a `var(--r-…)` reference, not a literal, **and the same for at + least two `--r-input` consumers — `input, select, textarea` (L337) and `.named-picker-trigger` + (L2810)**. Without those the "radius scale" guard asserts nothing about the one token whose *value* + this slice changes, and reads as safety it does not provide. `.named-picker-trigger` additionally + loses its dead `var(--r-input, 4px)` fallback: `--r-input` is always declared on `:root`, so the + fallback can never fire and it is the only literal on a surface this slice owns. + +**Mutation rows** (each must be run and seen red before the guard is claimed): + +| # | Mutation | Guard that must redden | On the assertion named | +|---|---|---|---| +| M1 | re-add `box-shadow: var(--shadow-dialog)` to `.panel` | G1 | the non-inset selector set | +| M2 | re-add `text-transform: uppercase` to `table.data th` | G2 | the uppercase selector set | +| M3 | remove `text-transform: uppercase` from `.nav-group-label` | G2 | the same set (it must be an equality, not a subset) | +| M4 | change `.toolbar` fill back to `var(--surface)` | G1 | the toolbar inset assertion | +| M5 | set `--r-panel` to `16px` (equal to `--r-card`) | G3 | the three-distinct-increasing assertion | +| M6 | give `.toolbar` a literal `border-radius: 10px` | G3 | the token-reference assertion | +| M7 | add an all-`inset` shadow to a new rule | G1 | must stay **green** — proves the exclusion is by value, not an accident | +| M8 | give `.panel` `box-shadow: inset 0 0 0 1px var(--hairline), 0 8px 24px rgba(29,21,33,0.12)` | G1 | the non-inset selector set — proves the guard splits layers rather than testing the value's prefix | +| M9 | give `input, select, textarea` a literal `border-radius: 6px` | G3 | the token-reference assertion on an `--r-input` consumer | + +## 6. Callers — what #394 says to read rather than run + +Eight Playwright call sites query a table header by accessible name: `i18n.spec.ts:211`, +`reports-range.spec.ts:83`, `worker-sale-allocation.spec.ts:88`, `owner.spec.ts:70,88`, +`manager.spec.ts:330`, `sales.spec.ts:107,145`. All eight pass a `t(...)`/`tEn(...)` translation +lookup rather than a literal, and **none passes `exact: true`**. + +That omission is the safety argument, and it is the only one that holds. Playwright's `name` option +matches the accessible name **case-insensitively and as a substring** unless `exact: true` is set — +a behaviour this repo already knows and uses deliberately, with `reports-range.spec.ts:71` carrying a +comment explaining why "From" and "To" need the flag. So these eight sites resolve the same header +whether or not the engine folds `text-transform` into the accessible name, before the change and after +it. The DOM text they ultimately match is untouched by this slice. + +*(Corrected at Phase 4: the first draft argued that the specs passing today proved Chromium does not +fold the transform into the accessible name. It proves no such thing — case-insensitive matching makes +both answers consistent with a green run. The conclusion survives; the reasoning that reached it did +not.)* + +No spec hardcodes an uppercase literal — `grep -rn "toUpperCase\|uppercase"` over +`tools/simulation/ui/` returns nothing outside `node_modules`. **`.badge` has no Playwright caller at +all.** Net caller risk identified from reading: none. The `caller-breakage` review seat exists to +refute that, not to confirm it. + +**This reading does not replace a run.** `tools/simulation/ui/` is deliberately not in CI, so nothing +else will exercise it. Before the merge ask, the six specs above — +`{owner,manager,sales,reports-range,worker-sale-allocation,i18n}.spec.ts` — are run against the sim +harness and the result recorded in the PR body. A read is the argument; the run is the evidence. + +## 7. Contrast + +`table.data th` moves from `--muted` to `--ink`. Light: `#1d1d1d` on `#ffffff`. Night: `#f2ecf4` on +`#221a26`. Both are far above 4.5:1, and the move is strictly an increase — `--muted` (`#696969` on +white, `#b3a8b8` on `#221a26`) already cleared it. The repo's own `contrast()` helper in +`web/src/test/cssTokens.ts` is the check; assert it across all four palettes in both modes rather than +reasoning about it. + +## 8. Documentation + +`specs/product/GLOSSARY.md` and the SPA Help page take **no change**, and the reason is recorded rather +than assumed: this slice introduces no concept, renames nothing, and changes no term's meaning — it +changes the rendering of labels whose text is unchanged. Per AGENTS.md's docs-in-sync rule a missing +doc update is treated like a missing test, so the justification belongs in the PR body. + +## 9. Simplicity ceiling + +**Smallest viable implementation:** delete two `box-shadow` declarations, re-point one at +`--shadow-dialog`, retire `--shadow-card`, add `--r-panel` and lower `--r-input`, make the toolbar +inset, sentence-case four rules, delete two dead `.eyebrow` rules, and add two guard files. + +**Complexity budget:** `web/src/styles.css` and two new `web/src/styles.*.test.ts` files. No new +concepts. No component or TSX change. **Non-goals:** the twelve pre-existing literal radii, any +`--shadow-bar`/`--shadow-dialog` retuning, the auth screen, motion, and the other five sibling issues. +Anything beyond this stops and reports rather than absorbing more design. + +## 10. Out of scope + +#653 (table layout), #655 (empty states), #656 (visual identity) — sibling slices with their own +surfaces. A finding against one of those is filed there. diff --git a/docs/plans/651-652-spa-elevation-and-caps/01-implementer-runbook.md b/docs/plans/651-652-spa-elevation-and-caps/01-implementer-runbook.md new file mode 100644 index 000000000..f5c72cd7c --- /dev/null +++ b/docs/plans/651-652-spa-elevation-and-caps/01-implementer-runbook.md @@ -0,0 +1,563 @@ +# Runbook — #651 + #652: elevation hierarchy, and the end of tracked all-caps + +You are an autonomous coding agent with FULL tools (read, edit, write, bash) in the +`/home/mforce/dev/cluckwork` repo (.NET 10 backend, React 19 + Vite SPA in `web/`; cwd = repo root). +Execute this runbook top to bottom. You do everything except the merge: branch, edit, test, commit, +push, open the PR. + +**Goal:** elevation encodes what floats and radius encodes nesting depth (#651); tracked all-caps +survives only on the two nav group dividers (#652). + +**Design:** `docs/designs/651-652-spa-elevation-and-caps.md` — read it first. It carries the verified +inventory this runbook acts on, and the reasons behind every decision here. + +## Rules + +- Transcribe the exact code blocks VERBATIM (comments and whitespace included). Do not reformat, + rename, or "improve" them. Blocks marked **PROTECTED** are the guards themselves: transcribe or + STOP, never repair. A guard that has been locally adjusted to pass is worse than no guard. +- Run the commands EXACTLY as given. Do not invent flags. +- After every test command, if it is not clean, STOP and fix before continuing. **An expected RED is a + clean result** — but only that exact RED: the command as written, the named test, failing at the + named assertion. Anything else is a STOP however red it looks: a TypeScript error, zero tests + collected, a different test failing, or a baseline failure that has changed shape. +- **Do NOT touch:** + - `web/src/styles.test.ts`, `web/src/styles.num.test.ts`, `web/src/styles.help.test.ts`, + `web/src/components/styles.dialog.test.ts`, `web/src/components/FarmBrand.styles.test.ts` — the + existing style guards. If one of them goes red, that is a finding, not a thing to edit. + - Any `.tsx` file. This slice changes no component. If you believe one must change, STOP and report. + - `web/src/i18n/*` — no string changes. The translations are already sentence case. + - `tools/simulation/ui/**` — the Playwright suite. The driver runs it at verification; you neither + run it nor edit it. + - `web/vite.config.ts` — the coverage floors. See G3: on a trip you STOP, you never re-baseline. + - `specs/product/GLOSSARY.md` and `web/src/routes/HelpPage.tsx` — see the documentation table below + for why this slice owes them nothing. + - `table.data th[scope="row"]` (styles.css ~L1494): it declares `text-transform: none` and + `letter-spacing: 0`, which become redundant once `table.data th` stops uppercasing. Redundant is + not wrong, and removing them is churn outside this slice. + - `.entry-pane`'s `border-radius` (styles.css ~L760): it stays `var(--r-card)`. It is a top-level + pane on the Daily entry page, not a panel nested inside a card, so the depth mapping puts it at + 16. The guard asserts it resolves through a token, not which token. + - The twelve pre-existing literal `border-radius` values (L688, 1193, 1745, 1761, 1908, 2065, 2208, + 2570, 2710, 2775). Out of scope, named explicitly in the design. +- **Files you may create or edit — anything else, STOP and report:** + - `web/src/styles.css` (edit) + - `web/src/styles.elevation.test.ts` (create) + - `web/src/styles.caps.test.ts` (create) +- Work only on the new branch. Never commit to `main`. +- A **mutation check** means: plant a bug on purpose, run the suite, and see whether a test notices. + RED means that test guards the code; GREEN means nothing was watching. Then restore and re-run to + confirm you are clean. **Restore before re-running, never `--no-build`-style shortcuts** — Vitest + reads the stylesheet from disk at import time, so a stale run is a real hazard here. +- Run the full test suite in the FOREGROUND and report its final summary line verbatim. Do not + background it and report before it finishes. +- If a code block here conflicts with an existing test, STOP and report the conflict. Do NOT relax or + delete that test. +- **Stop-and-report limit:** if you run the same test more than about five times without progress, + STOP and report. That is information the driver needs, not a failure on your part. + +**Protected-block probe — what the driver read before authoring the PROTECTED blocks:** +`postcss` is already a devDependency and already used this way by `web/src/styles.num.test.ts` and +`web/src/styles.help.test.ts` — both parse `src/styles.css` with `postcss.parse` and walk it, because +jsdom computes no layout. Probed and settled from those two files plus `web/src/test/cssTokens.ts`: +(1) `root.walkDecls(prop, cb)` visits declarations inside `@media` at-rules as well as top-level +rules, which is why the shadow walk needs no separate at-rule pass — `styles.help.test.ts` needed an +explicit `walkAtRules` only because it wanted to *distinguish* media context, which this guard does +not; (2) a declaration's `.parent` inside a media query is the `Rule`, not the `AtRule`, so +`(d.parent as Rule).selectors` is correct there too; (3) `rule.selectors` splits a grouped selector +list into its members, which is how `.card, .panel, .order-panel` yields three entries; (4) comments +survive inside selector text, which is why both existing files strip them with the same +`replace(/\/\*[\s\S]*?\*\//g, "")` before comparing — copied verbatim rather than reinvented; (5) there are **two** literal +`:root` blocks, not one — the token block at styles.css L9 and a second inside +`@media (max-width: 900px)` at ~L2231 declaring `--tabbar-h` alone. `declarationsFor(":root")` merges +both, which is harmless here because the key sets do not overlap: `--r-input`, `--r-panel` and +`--r-card` are declared only in the first. Left merged deliberately rather than special-cased. **Do +not "fix" it.** (The driver's first probe for this was `grep -n "^:root {"`, which is anchored to +column zero and therefore could not see the indented second block — corrected before dispatch.) + +**Existing instances of this pattern:** `web/src/styles.num.test.ts` (declaration lookup by selector), +`web/src/styles.help.test.ts` (the same, plus media-scoped lookup), `web/src/components/styles.dialog.test.ts`. +The blocks below differ from all three in one way, deliberately: those look **up** declarations for +selectors they name, while these walk **every** declaration and assert over the complete set. That is +AGENTS.md's "walk everything, exclude deliberately" — the named-selector lookups here exist only as +supplementary assertions beside the set equalities, never in place of them. + +## Verify prerequisites (run first) + +```bash +git -C /home/mforce/dev/cluckwork rev-parse --abbrev-ref HEAD # expect: main +git -C /home/mforce/dev/cluckwork status --porcelain # expect: empty, or only untracked docs/ files +node --version # expect: v26.x (CI pins node-version: 26) +``` + +Observed by the driver on 2026-09-02 on this host: branch `main` at +`396ba233c04a84fa3452e9fd7901e4f2429d4bd7`, tree clean apart from the two untracked planning documents +this runbook is one of. + +**The commit gate:** `.githooks/pre-commit` is opt-in via `git config core.hooksPath .githooks`. Check +whether it is enabled with `git config --get core.hooksPath` and report what you find. It runs +`npm run typecheck` when `web/` files are staged; it does not run the CSS guards. Do not enable or +disable it. + +## Caller ledger + +| Increment | Contract changed | Every production caller | What each does AT THIS COMMIT | Same-commit or later? | Observed at that commit (driver, Phase 11) | +|---|---|---|---|---|---| +| 1 | none — no contract change. CSS custom properties are not a code contract; `--shadow-card` is removed and every reference to it is removed in the same commit. | `web/src/styles.css` is the only file that references `--shadow-card` (verified repo-wide) | compiles and renders; no consumer is left dangling | same commit | *(driver fills)* | +| 2 | none — no contract change. `text-transform` is presentational; no DOM text changes. | The eight Playwright `columnheader` queries (`i18n.spec.ts:211`, `reports-range.spec.ts:83`, `worker-sale-allocation.spec.ts:88`, `owner.spec.ts:70,88`, `manager.spec.ts:330`, `sales.spec.ts:107,145`) | all eight omit `exact: true`, so name matching is case-insensitive substring; they resolve the same header before and after | same commit | *(driver fills — the sim-harness run the owner required)* | + +## Gate commands + +| ID | Gate | Source | Command, verbatim | Baseline on `396ba233` | Clean looks like — and, on a trip | +|---|---|---|---|---|---| +| G1 | typecheck + build | `.github/workflows/ci.yml`, job `web`, step `Typecheck and build` | `npm run build` (in `web/`) | clean — driver-verified 2026-09-02 | exits 0. `tsc -b` first, then `vite build`. **Bare `npx tsc --noEmit` is vacuous here — never substitute it.** | +| G2 | test | `.github/workflows/ci.yml`, job `web`, step `Test with coverage gate` | `npm run test:coverage` (in `web/`) | `Test Files 107 passed (107)` / `Tests 2370 passed (2370)` — driver-verified 2026-09-02, nothing already red | the same two lines with counts risen by the tests you added, and the coverage summary present. Read against the baseline, not against absolute green. | +| G3 | coverage floors | `web/vite.config.ts`, `thresholds` | (same command as G2 — the floors are enforced inside it) | measured on `396ba233`: statements **90.77** (floor 89, headroom 1.77), branches **86.04** (floor 80, headroom 6.04), functions **85.84** (floor 85, **headroom 0.84**), lines **93.67** (floor 92, headroom 1.67) | no threshold error printed. **Functions has under one point of headroom.** Your two new files are `*.test.ts` and are excluded from coverage by `vite.config.ts`, so they should not move any denominator. **On a trip: STOP and report the four measured numbers. Do NOT edit `vite.config.ts` and do NOT add a test to pad a number.** | +| G4 | service-worker guarantees | `.github/workflows/ci.yml`, job `web`, step `Verify service-worker guarantees` | `npm run verify:sw` (in `web/`) | clean — CI-attested on `396ba233`, not driver-verified; runnable by anyone with `web/` deps installed | exits 0. Unaffected by CSS, run once at the end as a regression check. | +| — | GitGuardian, CodeQL, dependency-review | server-side integrations | no command to paste | n/a | reported on the PR; handled by the driver's reviewer sweep, not by you. | + +## Documentation surfaces + +| Surface | Path / key | Locales | Increment | Verification procedure (run at Phase 11) | Verified by + SHA | +|---|---|---|---|---|---| +| product glossary | `specs/product/GLOSSARY.md` | n/a | **none** | **none owed, and this is the reason, not an omission:** the slice introduces no concept, renames nothing, and changes no term's meaning. Every label's *text* is unchanged — only its rendering is. AGENTS.md treats a missing doc update like a missing test, so the justification goes in the PR body. | *(driver)* | +| in-app Help + glossary | `web/src/routes/HelpPage.tsx`, `web/src/i18n/{en,es,tl}.ts` | en / es / tl (resolved from `web/src/i18n/` — those three files are the whole set; `ls web/src/i18n/*.ts` is the command) | **none** | as above. Additionally: the three bundles are already sentence case, so nothing relied on CSS to capitalise. Confirm by reading, not by rendering: `grep -n "eyebrow" web/src/i18n/*.ts` returns one key per locale, all sentence case. | *(driver)* | +| the screens themselves | every SPA route | all three | 1 and 2 | before/after screenshots, light and night, produced by the **driver** from the sim harness — you do not run it. | *(driver)* | + +## Step 0 — branch + +```bash +cd /home/mforce/dev/cluckwork +git checkout main +git pull --ff-only +git checkout -b feat/651-652-spa-elevation-and-caps +``` + +=================================================================================== +# INCREMENT 1 — elevation and the radius scale (#651) +=================================================================================== + +RED → GREEN. Do not reorder: the failing run is the proof the guard can fail at all. + +## 1a. RED — add the elevation guard + +Create `web/src/styles.elevation.test.ts` with EXACTLY this content. **PROTECTED — transcribe or +STOP.** + +```ts +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import postcss from "postcss"; +import type { Rule } from "postcss"; + +// #651 — elevation encodes what floats; radius encodes nesting depth. +// +// Both halves WALK the whole parsed stylesheet and assert over the complete +// set. They are deliberately not a lookup of the selectors this slice touched: +// AGENTS.md's guard rules call a hand-maintained list of what the author +// happened to think of exactly the thing a guard exists to stop anyone +// trusting. jsdom computes no layout, so the declarations are read from the +// parsed stylesheet — the same approach as styles.num.test.ts. +const css = readFileSync(resolve(process.cwd(), "src/styles.css"), "utf8"); +const root = postcss.parse(css); +const clean = (s: string) => s.replace(/\/\*[\s\S]*?\*\//g, "").trim(); + +// Split a box-shadow value into its comma-separated layers, ignoring commas +// inside parentheses: rgba(29, 21, 33, 0.04) is one token, not four layers. +function layers(value: string): string[] { + const out: string[] = []; + let depth = 0; + let current = ""; + for (const ch of value) { + if (ch === "(") depth += 1; + else if (ch === ")") depth -= 1; + if (ch === "," && depth === 0) { + out.push(current); + current = ""; + continue; + } + current += ch; + } + out.push(current); + return out.map((layer) => layer.trim()).filter((layer) => layer.length > 0); +} + +// A rule casts a drop shadow if ANY layer is non-inset. Testing only the +// value's prefix would pass +// box-shadow: inset 0 0 0 1px var(--hairline), 0 8px 24px rgba(0,0,0,.12); +// which is a real drop shadow wearing an inset first layer — and that is +// precisely the regression this guard exists to block. +function castsShadow(value: string): boolean { + if (clean(value) === "none") return false; + return layers(value).some((layer) => !layer.startsWith("inset")); +} + +function selectorsCastingShadow(): string[] { + const found = new Set(); + root.walkDecls("box-shadow", (d) => { + if (!castsShadow(d.value)) return; + for (const sel of (d.parent as Rule).selectors) found.add(clean(sel)); + }); + return [...found].sort(); +} + +function declarationsFor(selector: string): Map { + const decls = new Map(); + root.walkRules((rule: Rule) => { + if (!rule.selectors.map(clean).includes(selector)) return; + rule.walkDecls((d) => { decls.set(d.prop, d.value); }); + }); + return decls; +} + +// Everything allowed to cast a shadow, and why. Six floats plus one ring. +const SHADOW_ALLOWED = [ + ".auth .card", // the sign-in card, floating on the auth gradient + ".dialog", // a modal, over its backdrop + ".entry-foot", // the Daily entry sticky action bar + ".glossary-entry:target", // not elevation: a spread-only deep-link halo + ".named-picker-listbox", // the picker popover, over the form beneath it + ".tabbar", // the mobile tab bar + ".update-banner", // the service-worker update prompt +].sort(); + +describe("#651 elevation: only a float casts a shadow", () => { + it("no rule outside the float set casts a drop shadow", () => { + expect(selectorsCastingShadow()).toEqual(SHADOW_ALLOWED); + }); + + it("a panel, a card and an order panel carry a border and nothing else", () => { + for (const selector of [".card", ".panel", ".order-panel"]) + expect(declarationsFor(selector).get("box-shadow")).toBeUndefined(); + }); + + it("the toolbar reads as inset, not as a floating card", () => { + const toolbar = declarationsFor(".toolbar"); + expect(toolbar.get("background")).toBe("var(--surface-2)"); + expect(toolbar.get("border")).toBe("1px solid var(--hairline)"); + expect(toolbar.get("border-radius")).toBe("var(--r-panel)"); + expect(toolbar.get("box-shadow")).toBeUndefined(); + }); + + it("the picker popover uses the float shadow, not the retired card one", () => { + expect(declarationsFor(".named-picker-listbox").get("box-shadow")) + .toBe("var(--shadow-dialog)"); + }); + + it("--shadow-card is retired: not declared, and referenced nowhere", () => { + expect(css).not.toContain("--shadow-card"); + }); +}); + +describe("#651 radius: a three-step scale, declared as tokens", () => { + const tokenValue = (name: string): string | undefined => + declarationsFor(":root").get(name); + + it("declares three distinct steps in increasing order", () => { + expect(tokenValue("--r-input")).toBe("6px"); + expect(tokenValue("--r-panel")).toBe("10px"); + expect(tokenValue("--r-card")).toBe("16px"); + }); + + // Every surface this slice owns, INCLUDING two --r-input consumers. Without + // those the scale guard would assert nothing about the one token whose value + // actually changes, and would read as safety it does not provide. + it.each([ + ".toolbar", + ".card", + ".panel", + ".order-panel", + ".entry-pane", + ".capture-tile", + "input", + ".named-picker-trigger", + ])("%s resolves its radius through a token, not a literal", (selector) => { + const radius = declarationsFor(selector).get("border-radius"); + expect(radius).toMatch(/^var\(--r-[a-z]+\)$/); + }); +}); +``` + +Note on the last block: the `--r-input` consumer is listed as `"input"`, not as the grouped selector +`"input, select, textarea"`. `declarationsFor` matches on the MEMBERS of `rule.selectors`, which +postcss splits a grouped list into — so `"input"` matches the `input, select, textarea` rule at +styles.css ~L414 and the grouped string would match nothing. Do not "fix" it to the grouped form. + +Run it and RECORD THE FAILURE: + +```bash +npx vitest run src/styles.elevation.test.ts +``` + +| Gate row + narrowing | Named test | Assertion | Stable discriminator | Generated fragments | Also expected to fail | +|---|---|---|---|---|---| +| G2, narrowed to `src/styles.elevation.test.ts` | `#651 elevation: only a float casts a shadow > no rule outside the float set casts a drop shadow` | `expect(selectorsCastingShadow()).toEqual(SHADOW_ALLOWED)` | the received array contains `".toolbar"`, `".card"`, `".panel"` and `".order-panel"` — four entries the expected array does not have | none — every value here is read from a committed file | the toolbar test (`background` is `var(--surface)`), the `--shadow-card` retirement test, the picker-popover test, the three-distinct-steps test (`--r-panel` is `undefined`), and the **`.named-picker-trigger`** radius case (`var(--r-input, 4px)` at styles.css ~L2810 — the `, 4px` fallback breaks the regex's `$` anchor). **`.toolbar`'s radius case PASSES before the edit** — its pre-edit `var(--r-card)` already matches the token regex; it is the toolbar's *background* and *box-shadow* that fail, inside the inset test. **Seven failures out of fourteen tests is the expected RED. Zero failures, a different count, or a failure in another file, is a STOP.** | + +**If it passes, STOP and report — do not continue to 1b.** A guard that is green before its code +exists proves nothing. + +## 1b. GREEN — apply the elevation and radius changes + +Edit `web/src/styles.css` only. Six changes: + +1. **The radius tokens** (~L74). Replace: + ```css + --r-input: 8px; + --r-card: 16px; + ``` + with: + ```css + --r-input: 6px; + --r-panel: 10px; + --r-card: 16px; + ``` + Leave `--r-pill: 999px` exactly where it is. + +2. **Retire `--shadow-card`.** Delete the whole declaration at ~L84 (light `:root`) and the whole + declaration at ~L209 (the dark base). Leave `--shadow-dialog` and `--shadow-bar` untouched in both. + +3. **`.toolbar`** (~L1052). Replace `background: var(--surface);` with + `background: var(--surface-2);`, replace `border-radius: var(--r-card);` with + `border-radius: var(--r-panel);`, and delete the `box-shadow: var(--shadow-card);` line. Keep the + border, the padding, the flex rules and the margin exactly as they are. Update the section comment + above it — it currently reads `Toolbar (carded filter bar)` — to say it is inset, and say why the + border stays: `--canvas` and `--surface-2` sit at 1.05-1.21:1 in every palette, so a borderless + fill would be invisible. + +4. **`.card, .panel, .order-panel`** (~L1516). Delete the `box-shadow: var(--shadow-card);` line. + Keep the background, border and radius. + +5. **`.named-picker-listbox`** (~L2880). Replace `box-shadow: var(--shadow-card);` with + `box-shadow: var(--shadow-dialog);`. + +6. **`.named-picker-trigger`** (~L2810). Replace `border-radius: var(--r-input, 4px);` with + `border-radius: var(--r-input);`. The fallback can never fire — `--r-input` is always declared on + `:root` — and it is the only literal on a surface this slice owns. + +Then: + +```bash +cd /home/mforce/dev/cluckwork/web && npx vitest run src/styles.elevation.test.ts +``` +Expect: all tests in that file pass. + +Then the full gates, **G1 then G2**, citing their rows above. Both must be clean against the baseline. + +## 1c. Commit + +```bash +git add web/src/styles.css web/src/styles.elevation.test.ts +git commit -m "feat(web): elevation encodes what floats, radius encodes depth (#651)" +``` + +=================================================================================== +# INCREMENT 2 — sentence-case labels (#652) +=================================================================================== + +## 2a. RED — add the caps guard + +Create `web/src/styles.caps.test.ts` with EXACTLY this content. **PROTECTED — transcribe or STOP.** + +```ts +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import postcss from "postcss"; +import type { Rule } from "postcss"; +import { contrast, resolveTokens, type Mode } from "./test/cssTokens"; +import { BRANDS, DEFAULT_BRAND } from "./lib/brand"; + +// #652 — caps mark structure, they do not decorate every label. The walk is +// over the WHOLE stylesheet: an equality, not a subset, so both re-adding caps +// somewhere new and dropping them from a divider are failures. +const css = readFileSync(resolve(process.cwd(), "src/styles.css"), "utf8"); +const root = postcss.parse(css); +const clean = (s: string) => s.replace(/\/\*[\s\S]*?\*\//g, "").trim(); + +const attrFor = (brand: string) => (brand === DEFAULT_BRAND ? null : brand); +const MODES: Mode[] = ["light", "dark"]; + +// The sidebar and the More-sheet group dividers. Nothing else. +const CAPS_ALLOWED = [".more-group-label", ".nav-group-label"].sort(); + +function selectorsUppercasing(): string[] { + const found = new Set(); + root.walkDecls("text-transform", (d) => { + if (clean(d.value) !== "uppercase") return; + for (const sel of (d.parent as Rule).selectors) found.add(clean(sel)); + }); + return [...found].sort(); +} + +function declarationsFor(selector: string): Map { + const decls = new Map(); + root.walkRules((rule: Rule) => { + if (!rule.selectors.map(clean).includes(selector)) return; + rule.walkDecls((d) => { decls.set(d.prop, d.value); }); + }); + return decls; +} + +describe("#652 caps: only the nav group dividers shout", () => { + it("no rule outside the two dividers uppercases", () => { + expect(selectorsUppercasing()).toEqual(CAPS_ALLOWED); + }); + + it("table headers are readable ink at a readable size, not tracked caps", () => { + const th = declarationsFor("table.data th"); + expect(th.get("color")).toBe("var(--ink)"); + expect(th.get("letter-spacing")).toBe("0"); + expect(th.get("font-weight")).toBe("600"); + }); + + it("the badge and the step pill keep their pill and drop the tracking", () => { + expect(declarationsFor(".badge").get("letter-spacing")).toBe("0"); + expect(declarationsFor(".badge").get("border-radius")).toBe("var(--r-pill)"); + expect(declarationsFor(".step-n").get("letter-spacing")).toBe("0"); + expect(declarationsFor(".step-n").get("border-radius")).toBe("var(--r-pill)"); + }); + + it("the dead .eyebrow rules are gone", () => { + expect(css).not.toContain(".eyebrow"); + }); +}); + +// A header only becomes more legible if the colour it moves to is legible. +// Asserted with the repo's own contrast(), across every palette and both +// modes, rather than reasoned about from two hex values. +describe.each(BRANDS)("#652 contrast: table headers on %s", (brand) => { + it.each(MODES)("%s: --ink on --surface clears WCAG AA", (mode) => { + const t = resolveTokens(attrFor(brand), mode); + expect(contrast(t.get("--ink")!, t.get("--surface")!)).toBeGreaterThanOrEqual(4.5); + }); +}); +``` + +Run it and RECORD THE FAILURE: + +```bash +cd /home/mforce/dev/cluckwork/web && npx vitest run src/styles.caps.test.ts +``` + +| Gate row + narrowing | Named test | Assertion | Stable discriminator | Generated fragments | Also expected to fail | +|---|---|---|---|---|---| +| G2, narrowed to `src/styles.caps.test.ts` | `#652 caps: only the nav group dividers shout > no rule outside the two dividers uppercases` | `expect(selectorsUppercasing()).toEqual(CAPS_ALLOWED)` | the received array contains `"table.data th"`, `".badge"`, `".step-n"` and `".eyebrow"` — four entries the expected array does not have | none | the table-header test (`color` is `var(--muted)`), the badge/step-pill tracking test, and the `.eyebrow` deletion test. **The eight contrast cases must PASS already** — `--ink` on `--surface` is unchanged by this slice, and a red there means the palette is broken independently of #652, which is a STOP and a report, not something to fix here. Four failures, eight passes, is the expected RED. | + +**If the set-equality test passes, STOP and report.** + +## 2b. GREEN — apply the caps changes + +Edit `web/src/styles.css` only. Four changes: + +1. **`.step-n`** (~L730). Delete `text-transform: uppercase;` and change `letter-spacing: 0.08em;` to + `letter-spacing: 0;`. Keep the pill, the tint, the colour and the weight. The comment above the + letter-spacing line explains the tracking — rewrite it to say the label is sentence case now. + +2. **`.badge`** (~L1096). Delete `text-transform: uppercase;`, change `letter-spacing: 0.03em;` to + `letter-spacing: 0;`, and change `font-size: 0.72rem;` to `font-size: 0.78rem;`. Keep the pill, + the weight, the tint and the `white-space: nowrap`. + +3. **Delete `.eyebrow` entirely** (~L1134, the whole rule) **and delete `.help-toc .eyebrow`** + (~L2133, the whole rule). Both are dead: the class has no call site anywhere in `web/src`. The + only `eyebrow` in the SPA is an i18n key rendered through `.help-kicker`, which is unaffected. + Remove the section comment that introduces `.eyebrow` along with it. + +4. **`table.data th`** (~L1463). Change `color: var(--muted);` to `color: var(--ink);`, + `font-weight: 700;` to `font-weight: 600;`, `font-size: 0.68rem;` to `font-size: 0.8rem;`, delete + `text-transform: uppercase;`, and change `letter-spacing: 0.06em;` to `letter-spacing: 0;`. Do NOT + touch `table.data th[scope="row"]` below it. + +Then: + +```bash +cd /home/mforce/dev/cluckwork/web && npx vitest run src/styles.caps.test.ts +``` +Expect: all tests in that file pass, contrast cases included. + +Then **G1** and **G2**, both clean against the baseline. Report G2's summary line and the four +coverage numbers verbatim. + +## 2c. Commit + +```bash +git add web/src/styles.css web/src/styles.caps.test.ts +git commit -m "feat(web): sentence-case every label outside the nav group dividers (#652)" +``` + +=================================================================================== +# INCREMENT 3 — the local adversarial pass (mutation checks) +=================================================================================== + +AGENTS.md requires this before the first push: *mutation first, claim second*. Nine rows. For each: +apply the mutation to `web/src/styles.css`, run the named command, record the result, then **restore +the file and re-run to confirm you are clean** before the next row. `git diff` after every restore +must be empty for `styles.css`. + +Run each with `cd /home/mforce/dev/cluckwork/web && npx vitest run src/styles.elevation.test.ts src/styles.caps.test.ts`. + +| # | Mutation | Must go | On which assertion | +|---|---|---|---| +| M1 | add `box-shadow: var(--shadow-dialog);` to `.panel` | RED | the non-inset selector set | +| M2 | add `text-transform: uppercase;` back to `table.data th` | RED | the uppercase selector set | +| M3 | delete `text-transform: uppercase;` from `.nav-group-label` | RED | the uppercase selector set — it must be an equality, not a subset | +| M4 | change `.toolbar`'s `background` back to `var(--surface)` | RED | the toolbar-inset assertion | +| M5 | set `--r-panel: 16px` | RED | the three-distinct-steps assertion | +| M6 | give `.toolbar` a literal `border-radius: 10px` | RED | the token-reference assertion | +| M7 | add `box-shadow: inset 0 0 0 1px var(--hairline);` to **`.entry-pane`** | **GREEN** | proves the exclusion is by value, not an accident. **Not `.panel`**: a second assertion in the same file requires `.panel`'s `box-shadow` to be undefined whatever its value, so an inset shadow there goes red on that assertion instead and proves nothing about the classifier. `.entry-pane` has no box-shadow assertion of its own, so it isolates the classifier cleanly. If this goes RED the guard is wrong. | +| M8 | give `.panel` `box-shadow: inset 0 0 0 1px var(--hairline), 0 8px 24px rgba(29, 21, 33, 0.12);` | RED | the non-inset selector set — proves the guard splits layers instead of testing the value's prefix. **This is the row that matters most**; a naive prefix check passes it. Expect **two** failures, not one: the `.panel` no-shadow assertion trips as well, because M8 targets a selector it also covers. Both are correct; the named one is the non-inset set. | +| M9 | give `input, select, textarea` a literal `border-radius: 6px` | RED | the token-reference assertion on an `--r-input` consumer | + +**A row that does not do what this table says is a finding about the guard, not about the mutation.** +STOP and report it; do not adjust the guard to make the row come out right. + +After the last restore, run **G2** once more and confirm the suite is clean. + +**The driver already ran this whole runbook once**, on 2026-09-02, in a throwaway worktree at +`396ba233` with `node_modules` symlinked: guards pre-edit gave elevation 7 failures of 14 and caps 4 of +12 with all 8 contrast cases passing; the 1b and 2b edits applied literally gave 26/26; M3, M7 and M8 +behaved exactly as this table says; the full suite came to **109 files / 2396 tests** with coverage +unmoved from the baseline. So the numbers above are observed, not predicted. **If yours differ, that is +a real finding — report it rather than adjusting anything.** + +=================================================================================== +# FINISH — push and open the PR +=================================================================================== + +```bash +cd /home/mforce/dev/cluckwork +npm --prefix web run verify:sw # G4 +git push -u origin feat/651-652-spa-elevation-and-caps +``` + +Open the PR with `gh pr create`. **The title is the release note and becomes the squashed commit +subject**, so it must be a conventional-commit subject. Use exactly: + +``` +feat(web): elevation hierarchy and sentence-case labels (#651, #652) +``` + +The body must carry, in this order: + +1. What changed, split by issue. +2. **The corrections to the issue bodies**: `.stat`/`.stat-label` no longer exist (#654 replaced them + with `.capture-tile*`, already border-only and sentence case); `.entry-pane` already had no shadow; + six uppercase rules existed, not seven; `.eyebrow` had zero call sites and was deleted rather than + audited. Point at `docs/designs/651-652-spa-elevation-and-caps.md` for the verified inventory. +3. **The mutation table from increment 3, with the result you actually observed in each row.** Report + honestly — never write a red you did not see. +4. G2's summary line verbatim and the four coverage numbers. +5. **Why no glossary or Help change is owed** — the reason from the documentation table above, stated + in full. A reviewer will otherwise read it as an omission. +6. A line saying the `tools/simulation/ui/` Playwright specs and the before/after screenshots are the + **driver's** verification step and are not yet done. + +Do NOT merge. Do NOT mark the PR ready-for-review or draft either way without being told — leave it in +whatever state `gh pr create` gives you and report which it is. + +## Report back + +- The branch, the two commit SHAs, and `git status --short` pasted verbatim. +- G1, G2, G3, G4 results, each cited by row ID. +- The mutation table with observed results. +- Anything you stopped on, and anything in this runbook that turned out to be wrong. From 1cf689a9d9cf7ebac464b61dd452ac982bfc5e85 Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 2 Sep 2026 11:48:37 -0700 Subject: [PATCH 2/9] feat(web): elevation encodes what floats, radius encodes depth (#651) --- web/src/styles.css | 20 ++--- web/src/styles.elevation.test.ts | 132 +++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 10 deletions(-) create mode 100644 web/src/styles.elevation.test.ts diff --git a/web/src/styles.css b/web/src/styles.css index 62346e566..06e1b9872 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -71,7 +71,8 @@ radial-gradient(50% 50% at 18% 90%, #f7e4ff 0%, transparent 60%), var(--lavender); - --r-input: 8px; + --r-input: 6px; + --r-panel: 10px; --r-card: 16px; --r-pill: 999px; --sidebar-w: 244px; @@ -81,7 +82,6 @@ --tabbar-h: 0px; --font: "Inter Variable", Inter, system-ui, -apple-system, "Segoe UI", Roboto, sans-serif; - --shadow-card: 0 1px 2px rgba(29, 21, 33, 0.04), 0 8px 24px rgba(29, 21, 33, 0.05); --scrim: rgba(29, 21, 33, 0.42); --shadow-dialog: 0 24px 64px rgba(29, 21, 33, 0.24); --shadow-bar: 0 -8px 24px rgba(29, 21, 33, 0.08); @@ -206,7 +206,6 @@ radial-gradient(55% 50% at 88% 12%, rgba(74, 21, 75, 0.6) 0%, transparent 60%), radial-gradient(55% 55% at 82% 88%, rgba(45, 90, 70, 0.35) 0%, transparent 62%), var(--canvas); - --shadow-card: 0 1px 2px rgba(0, 0, 0, 0.3), 0 10px 30px rgba(0, 0, 0, 0.35); --scrim: rgba(0, 0, 0, 0.62); --shadow-dialog: 0 24px 64px rgba(0, 0, 0, 0.6); --shadow-bar: 0 -8px 24px rgba(0, 0, 0, 0.45); @@ -1048,17 +1047,19 @@ input.cell { padding: 0; } -/* -------------------------------------------- Toolbar (carded filter bar) */ +/* -------------------------------------------- Toolbar (inset filter bar) */ +/* --canvas and --surface-2 sit at 1.05-1.21:1 in every palette, so a + borderless fill would be invisible — the hairline border stays for that + reason, not because this is a floating card. */ .toolbar { display: flex; flex-wrap: wrap; gap: 1rem; align-items: end; padding: 1rem 1.25rem; - background: var(--surface); + background: var(--surface-2); border: 1px solid var(--hairline); - border-radius: var(--r-card); - box-shadow: var(--shadow-card); + border-radius: var(--r-panel); margin: 0.5rem 0 1.5rem; } @@ -1519,7 +1520,6 @@ label.check { background: var(--surface); border: 1px solid var(--hairline); border-radius: var(--r-card); - box-shadow: var(--shadow-card); } .panel { @@ -2807,7 +2807,7 @@ button:disabled[aria-busy="true"] { color: var(--ink); background: var(--surface); border: 1px solid var(--hairline); - border-radius: var(--r-input, 4px); + border-radius: var(--r-input); padding: 0.55rem 0.7rem; text-align: left; cursor: pointer; @@ -2877,7 +2877,7 @@ button:disabled[aria-busy="true"] { background: var(--surface); border: 1px solid var(--hairline); border-radius: var(--r-input); - box-shadow: var(--shadow-card); + box-shadow: var(--shadow-dialog); } .named-picker-option { diff --git a/web/src/styles.elevation.test.ts b/web/src/styles.elevation.test.ts new file mode 100644 index 000000000..b0f0cfafe --- /dev/null +++ b/web/src/styles.elevation.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import postcss from "postcss"; +import type { Rule } from "postcss"; + +// #651 — elevation encodes what floats; radius encodes nesting depth. +// +// Both halves WALK the whole parsed stylesheet and assert over the complete +// set. They are deliberately not a lookup of the selectors this slice touched: +// AGENTS.md's guard rules call a hand-maintained list of what the author +// happened to think of exactly the thing a guard exists to stop anyone +// trusting. jsdom computes no layout, so the declarations are read from the +// parsed stylesheet — the same approach as styles.num.test.ts. +const css = readFileSync(resolve(process.cwd(), "src/styles.css"), "utf8"); +const root = postcss.parse(css); +const clean = (s: string) => s.replace(/\/\*[\s\S]*?\*\//g, "").trim(); + +// Split a box-shadow value into its comma-separated layers, ignoring commas +// inside parentheses: rgba(29, 21, 33, 0.04) is one token, not four layers. +function layers(value: string): string[] { + const out: string[] = []; + let depth = 0; + let current = ""; + for (const ch of value) { + if (ch === "(") depth += 1; + else if (ch === ")") depth -= 1; + if (ch === "," && depth === 0) { + out.push(current); + current = ""; + continue; + } + current += ch; + } + out.push(current); + return out.map((layer) => layer.trim()).filter((layer) => layer.length > 0); +} + +// A rule casts a drop shadow if ANY layer is non-inset. Testing only the +// value's prefix would pass +// box-shadow: inset 0 0 0 1px var(--hairline), 0 8px 24px rgba(0,0,0,.12); +// which is a real drop shadow wearing an inset first layer — and that is +// precisely the regression this guard exists to block. +function castsShadow(value: string): boolean { + if (clean(value) === "none") return false; + return layers(value).some((layer) => !layer.startsWith("inset")); +} + +function selectorsCastingShadow(): string[] { + const found = new Set(); + root.walkDecls("box-shadow", (d) => { + if (!castsShadow(d.value)) return; + for (const sel of (d.parent as Rule).selectors) found.add(clean(sel)); + }); + return [...found].sort(); +} + +function declarationsFor(selector: string): Map { + const decls = new Map(); + root.walkRules((rule: Rule) => { + if (!rule.selectors.map(clean).includes(selector)) return; + rule.walkDecls((d) => { decls.set(d.prop, d.value); }); + }); + return decls; +} + +// Everything allowed to cast a shadow, and why. Six floats plus one ring. +const SHADOW_ALLOWED = [ + ".auth .card", // the sign-in card, floating on the auth gradient + ".dialog", // a modal, over its backdrop + ".entry-foot", // the Daily entry sticky action bar + ".glossary-entry:target", // not elevation: a spread-only deep-link halo + ".named-picker-listbox", // the picker popover, over the form beneath it + ".tabbar", // the mobile tab bar + ".update-banner", // the service-worker update prompt +].sort(); + +describe("#651 elevation: only a float casts a shadow", () => { + it("no rule outside the float set casts a drop shadow", () => { + expect(selectorsCastingShadow()).toEqual(SHADOW_ALLOWED); + }); + + it("a panel, a card and an order panel carry a border and nothing else", () => { + for (const selector of [".card", ".panel", ".order-panel"]) + expect(declarationsFor(selector).get("box-shadow")).toBeUndefined(); + }); + + it("the toolbar reads as inset, not as a floating card", () => { + const toolbar = declarationsFor(".toolbar"); + expect(toolbar.get("background")).toBe("var(--surface-2)"); + expect(toolbar.get("border")).toBe("1px solid var(--hairline)"); + expect(toolbar.get("border-radius")).toBe("var(--r-panel)"); + expect(toolbar.get("box-shadow")).toBeUndefined(); + }); + + it("the picker popover uses the float shadow, not the retired card one", () => { + expect(declarationsFor(".named-picker-listbox").get("box-shadow")) + .toBe("var(--shadow-dialog)"); + }); + + it("--shadow-card is retired: not declared, and referenced nowhere", () => { + expect(css).not.toContain("--shadow-card"); + }); +}); + +describe("#651 radius: a three-step scale, declared as tokens", () => { + const tokenValue = (name: string): string | undefined => + declarationsFor(":root").get(name); + + it("declares three distinct steps in increasing order", () => { + expect(tokenValue("--r-input")).toBe("6px"); + expect(tokenValue("--r-panel")).toBe("10px"); + expect(tokenValue("--r-card")).toBe("16px"); + }); + + // Every surface this slice owns, INCLUDING two --r-input consumers. Without + // those the scale guard would assert nothing about the one token whose value + // actually changes, and would read as safety it does not provide. + it.each([ + ".toolbar", + ".card", + ".panel", + ".order-panel", + ".entry-pane", + ".capture-tile", + "input", + ".named-picker-trigger", + ])("%s resolves its radius through a token, not a literal", (selector) => { + const radius = declarationsFor(selector).get("border-radius"); + expect(radius).toMatch(/^var\(--r-[a-z]+\)$/); + }); +}); From 5cd6c38755fa916dd372e1caa458b4c803db2b23 Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 2 Sep 2026 11:50:46 -0700 Subject: [PATCH 3/9] feat(web): sentence-case every label outside the nav group dividers (#652) --- web/src/styles.caps.test.ts | 72 +++++++++++++++++++++++++++++++++++++ web/src/styles.css | 34 +++++------------- 2 files changed, 80 insertions(+), 26 deletions(-) create mode 100644 web/src/styles.caps.test.ts diff --git a/web/src/styles.caps.test.ts b/web/src/styles.caps.test.ts new file mode 100644 index 000000000..281d78d15 --- /dev/null +++ b/web/src/styles.caps.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import postcss from "postcss"; +import type { Rule } from "postcss"; +import { contrast, resolveTokens, type Mode } from "./test/cssTokens"; +import { BRANDS, DEFAULT_BRAND } from "./lib/brand"; + +// #652 — caps mark structure, they do not decorate every label. The walk is +// over the WHOLE stylesheet: an equality, not a subset, so both re-adding caps +// somewhere new and dropping them from a divider are failures. +const css = readFileSync(resolve(process.cwd(), "src/styles.css"), "utf8"); +const root = postcss.parse(css); +const clean = (s: string) => s.replace(/\/\*[\s\S]*?\*\//g, "").trim(); + +const attrFor = (brand: string) => (brand === DEFAULT_BRAND ? null : brand); +const MODES: Mode[] = ["light", "dark"]; + +// The sidebar and the More-sheet group dividers. Nothing else. +const CAPS_ALLOWED = [".more-group-label", ".nav-group-label"].sort(); + +function selectorsUppercasing(): string[] { + const found = new Set(); + root.walkDecls("text-transform", (d) => { + if (clean(d.value) !== "uppercase") return; + for (const sel of (d.parent as Rule).selectors) found.add(clean(sel)); + }); + return [...found].sort(); +} + +function declarationsFor(selector: string): Map { + const decls = new Map(); + root.walkRules((rule: Rule) => { + if (!rule.selectors.map(clean).includes(selector)) return; + rule.walkDecls((d) => { decls.set(d.prop, d.value); }); + }); + return decls; +} + +describe("#652 caps: only the nav group dividers shout", () => { + it("no rule outside the two dividers uppercases", () => { + expect(selectorsUppercasing()).toEqual(CAPS_ALLOWED); + }); + + it("table headers are readable ink at a readable size, not tracked caps", () => { + const th = declarationsFor("table.data th"); + expect(th.get("color")).toBe("var(--ink)"); + expect(th.get("letter-spacing")).toBe("0"); + expect(th.get("font-weight")).toBe("600"); + }); + + it("the badge and the step pill keep their pill and drop the tracking", () => { + expect(declarationsFor(".badge").get("letter-spacing")).toBe("0"); + expect(declarationsFor(".badge").get("border-radius")).toBe("var(--r-pill)"); + expect(declarationsFor(".step-n").get("letter-spacing")).toBe("0"); + expect(declarationsFor(".step-n").get("border-radius")).toBe("var(--r-pill)"); + }); + + it("the dead .eyebrow rules are gone", () => { + expect(css).not.toContain(".eyebrow"); + }); +}); + +// A header only becomes more legible if the colour it moves to is legible. +// Asserted with the repo's own contrast(), across every palette and both +// modes, rather than reasoned about from two hex values. +describe.each(BRANDS)("#652 contrast: table headers on %s", (brand) => { + it.each(MODES)("%s: --ink on --surface clears WCAG AA", (mode) => { + const t = resolveTokens(attrFor(brand), mode); + expect(contrast(t.get("--ink")!, t.get("--surface")!)).toBeGreaterThanOrEqual(4.5); + }); +}); diff --git a/web/src/styles.css b/web/src/styles.css index 06e1b9872..cac5801d2 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -731,10 +731,8 @@ input.cell { color: var(--stat-accent); font-size: 0.7rem; font-weight: 700; - /* The headings carry negative display tracking; this is a label, so it opens - up instead. */ - letter-spacing: 0.08em; - text-transform: uppercase; + /* Sentence case now — no tracking, no uppercase. */ + letter-spacing: 0; } /* Flock + date sit above the two panes as context, not as a step. */ @@ -1097,10 +1095,9 @@ input.cell { gap: 0.3rem; padding: 0.16rem 0.6rem; border-radius: var(--r-pill); - font-size: 0.72rem; + font-size: 0.78rem; font-weight: 700; - letter-spacing: 0.03em; - text-transform: uppercase; + letter-spacing: 0; white-space: nowrap; background: var(--tint-muted); color: var(--muted); @@ -1131,16 +1128,6 @@ input.cell { .meter.is-warn > span { background: var(--warn); } -/* Eyebrow caps pill (section labels) */ -.eyebrow { - display: inline-block; - font-size: 0.68rem; - font-weight: 700; - letter-spacing: 0.1em; - text-transform: uppercase; - color: var(--muted); -} - /* ------------------------------------------------------- Sidebar shell */ .shell { display: grid; @@ -1464,11 +1451,10 @@ table.data td { } table.data th { - color: var(--muted); - font-weight: 700; - font-size: 0.68rem; - text-transform: uppercase; - letter-spacing: 0.06em; + color: var(--ink); + font-weight: 600; + font-size: 0.8rem; + letter-spacing: 0; } table.data tbody tr:hover { @@ -2130,10 +2116,6 @@ dl.glossary { top: 1.5rem; } -.help-toc .eyebrow { - padding: 0 0.6rem; -} - .help-toc ul { list-style: none; padding: 0; From dccfcc6badd6cb86e35341e012e8b68be1ec9abc Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 2 Sep 2026 11:57:37 -0700 Subject: [PATCH 4/9] docs: escape a leading issue reference and language-tag a fence (#661 review) --- docs/designs/651-652-spa-elevation-and-caps.md | 4 ++-- .../651-652-spa-elevation-and-caps/01-implementer-runbook.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/designs/651-652-spa-elevation-and-caps.md b/docs/designs/651-652-spa-elevation-and-caps.md index 8f3291bc1..8c4944e55 100644 --- a/docs/designs/651-652-spa-elevation-and-caps.md +++ b/docs/designs/651-652-spa-elevation-and-caps.md @@ -202,5 +202,5 @@ Anything beyond this stops and reports rather than absorbing more design. ## 10. Out of scope -#653 (table layout), #655 (empty states), #656 (visual identity) — sibling slices with their own -surfaces. A finding against one of those is filed there. +Issues #653 (table layout), #655 (empty states) and #656 (visual identity) — sibling slices with +their own surfaces. A finding against one of those is filed there. diff --git a/docs/plans/651-652-spa-elevation-and-caps/01-implementer-runbook.md b/docs/plans/651-652-spa-elevation-and-caps/01-implementer-runbook.md index f5c72cd7c..fab1dc7f3 100644 --- a/docs/plans/651-652-spa-elevation-and-caps/01-implementer-runbook.md +++ b/docs/plans/651-652-spa-elevation-and-caps/01-implementer-runbook.md @@ -533,7 +533,7 @@ git push -u origin feat/651-652-spa-elevation-and-caps Open the PR with `gh pr create`. **The title is the release note and becomes the squashed commit subject**, so it must be a conventional-commit subject. Use exactly: -``` +```text feat(web): elevation hierarchy and sentence-case labels (#651, #652) ``` From 2ca6b854073d4b67c0f696e10e86b26ddebb12e4 Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 2 Sep 2026 12:09:58 -0700 Subject: [PATCH 5/9] test(web): catch a shadow painted through filter: drop-shadow() (#651) --- .../02-fix-increment-1.md | 123 ++++++++++++++++++ web/src/styles.elevation.test.ts | 28 +++- 2 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 docs/plans/651-652-spa-elevation-and-caps/02-fix-increment-1.md diff --git a/docs/plans/651-652-spa-elevation-and-caps/02-fix-increment-1.md b/docs/plans/651-652-spa-elevation-and-caps/02-fix-increment-1.md new file mode 100644 index 000000000..a44b4bc96 --- /dev/null +++ b/docs/plans/651-652-spa-elevation-and-caps/02-fix-increment-1.md @@ -0,0 +1,123 @@ +# Fix increment 1 — close the `filter: drop-shadow()` bypass in the elevation guard + +Review round 1, `false-green` seat. **Driver-verified before dispatch**, not taken on the reviewer's word. + +## The finding + +`selectorsCastingShadow()` walks `box-shadow` declarations only. A drop shadow can also be painted with +`filter: drop-shadow(...)`, which that walk never visits — so the guard's stated invariant, *only a float +casts a shadow*, does not hold. + +**Observed by the driver on head `dccfcc6b`:** adding +`filter: drop-shadow(0 8px 24px rgba(29, 21, 33, 0.2));` to the `.card, .panel, .order-panel` rule puts a +visible drop shadow on every screen that uses those three classes, and +`npx vitest run src/styles.elevation.test.ts src/styles.caps.test.ts` stays **26/26 green**. That is a real +regression shipping past a guard whose whole purpose is to stop it. + +**Why the fix is cheap:** `grep -n "^\s*filter:" web/src/styles.css` returns nothing and there is no +`backdrop-filter` either. The stylesheet paints no filters at all today, so nothing has to be +allow-listed. + +## The shape of the fix, and why this shape + +**One set, two mechanisms.** Rather than adding a second, separate assertion, `selectorsCastingShadow()` +learns about the other way a shadow can be painted. The existing `SHADOW_ALLOWED` equality then covers +both mechanisms in one place, so a float that legitimately needs `drop-shadow()` is handled by the same +allow-list as one that uses `box-shadow` — and there is still exactly one assertion to read. + +It targets `drop-shadow` specifically, not `filter` wholesale: `filter: blur()` and `filter: brightness()` +are not shadows and banning them would be a guard asserting something its invariant never claimed. +`-webkit-filter` is included because it paints identically in a browser that honours it. + +## Files + +- Modify `web/src/styles.elevation.test.ts` — **PROTECTED**, transcribe exactly. +- Nothing else. `web/src/styles.css` is NOT touched by this increment: the shipped stylesheet is correct, + it is the guard that was incomplete. + +## Step 1 — RED + +Replace the whole `selectorsCastingShadow` function with the block below, and add the one new constant +above it. **PROTECTED — transcribe or STOP.** + +```ts +// A shadow can be painted two ways. `box-shadow` is the obvious one; +// `filter: drop-shadow(...)` renders the same thing and lives in a completely +// different declaration, so a walk that visits only `box-shadow` reports a +// clean stylesheet while a card floats on every screen. Review round 1 proved +// that with a mutation the guard did not notice. +// +// Both mechanisms feed ONE set, so the SHADOW_ALLOWED equality below governs +// both and a float that legitimately needs drop-shadow() is allow-listed +// exactly like one that uses box-shadow. +// +// This targets drop-shadow, not `filter` wholesale: blur() and brightness() +// are not shadows, and banning them would assert something the invariant never +// claimed. The stylesheet declares no filter at all today. +const SHADOW_PROPS = ["box-shadow", "filter", "-webkit-filter"]; + +function selectorsCastingShadow(): string[] { + const found = new Set(); + for (const prop of SHADOW_PROPS) { + root.walkDecls(prop, (d) => { + const casts = prop === "box-shadow" + ? castsShadow(d.value) + : d.value.includes("drop-shadow"); + if (!casts) return; + for (const sel of (d.parent as Rule).selectors) found.add(clean(sel)); + }); + } + return [...found].sort(); +} +``` + +Then plant the mutation the fix exists to catch, run the guards, and RECORD THE RED: + +```bash +cd web +# add this line inside the `.card, .panel, .order-panel { }` rule in src/styles.css: +# filter: drop-shadow(0 8px 24px rgba(29, 21, 33, 0.2)); +npx vitest run src/styles.elevation.test.ts src/styles.caps.test.ts +``` + +| Named test | Assertion | Stable discriminator | Expected | +|---|---|---|---| +| `#651 elevation: only a float casts a shadow > no rule outside the float set casts a drop shadow` | `expect(selectorsCastingShadow()).toEqual(SHADOW_ALLOWED)` | the received array gains `".card"`, `".panel"` and `".order-panel"` | **RED, 1 failed** | + +**Before this increment's change that same mutation is GREEN — the driver observed it.** If it is green +after your change too, the fix did not work: STOP and report, do not adjust anything. + +Then `git checkout -- src/styles.css` to remove the planted shadow and confirm 26/26 green again. + +## Step 2 — the new mutation rows + +Run all four, restoring `src/styles.css` between each with `git checkout -- src/styles.css`, and confirm +`git status --porcelain` shows no change to it afterwards. + +| # | Mutation | Must go | On which assertion | +|---|---|---|---| +| M10 | `filter: drop-shadow(0 8px 24px rgba(29, 21, 33, 0.2));` on `.panel` | RED | the non-inset selector set — this is the finding itself | +| M11 | `-webkit-filter: drop-shadow(0 8px 24px rgba(29, 21, 33, 0.2));` on `.panel` | RED | same set — the prefixed property paints the same shadow | +| M12 | `filter: blur(2px);` on `.panel` | **GREEN** | proves the guard targets drop-shadow, not `filter` wholesale. If this reddens the guard overreaches and the fix is wrong. | +| M13 | `filter: drop-shadow(0 24px 64px rgba(0,0,0,.3));` on `.dialog` | **GREEN** | `.dialog` is an allow-listed float, so the one set governs both mechanisms uniformly. This is the row proving the allow-list is shared, not duplicated. | + +## Step 3 — gates and commit + +Run **G1** then **G2** from the runbook's gate table, both clean against the baseline +(`Test Files 109 passed (109)` / `Tests 2396 passed (2396)`; coverage unmoved). + +```bash +git add web/src/styles.elevation.test.ts docs/plans/651-652-spa-elevation-and-caps/02-fix-increment-1.md +git commit -m "test(web): catch a shadow painted through filter: drop-shadow() (#651)" +git push origin feat/651-652-spa-elevation-and-caps +``` + +## Step 4 — reply on the review thread + +Do NOT reply on GitHub. Report back to the driver instead with: the commit SHA, the M10-M13 table with +the results you actually OBSERVED, and G1/G2 output. The driver answers the reviewer. + +## Report back + +Commit SHA, `git status --short` verbatim, G1 and G2 results, the four mutation rows as observed, and +anything here that turned out to be wrong. diff --git a/web/src/styles.elevation.test.ts b/web/src/styles.elevation.test.ts index b0f0cfafe..f1f78c9a4 100644 --- a/web/src/styles.elevation.test.ts +++ b/web/src/styles.elevation.test.ts @@ -46,12 +46,32 @@ function castsShadow(value: string): boolean { return layers(value).some((layer) => !layer.startsWith("inset")); } +// A shadow can be painted two ways. `box-shadow` is the obvious one; +// `filter: drop-shadow(...)` renders the same thing and lives in a completely +// different declaration, so a walk that visits only `box-shadow` reports a +// clean stylesheet while a card floats on every screen. Review round 1 proved +// that with a mutation the guard did not notice. +// +// Both mechanisms feed ONE set, so the SHADOW_ALLOWED equality below governs +// both and a float that legitimately needs drop-shadow() is allow-listed +// exactly like one that uses box-shadow. +// +// This targets drop-shadow, not `filter` wholesale: blur() and brightness() +// are not shadows, and banning them would assert something the invariant never +// claimed. The stylesheet declares no filter at all today. +const SHADOW_PROPS = ["box-shadow", "filter", "-webkit-filter"]; + function selectorsCastingShadow(): string[] { const found = new Set(); - root.walkDecls("box-shadow", (d) => { - if (!castsShadow(d.value)) return; - for (const sel of (d.parent as Rule).selectors) found.add(clean(sel)); - }); + for (const prop of SHADOW_PROPS) { + root.walkDecls(prop, (d) => { + const casts = prop === "box-shadow" + ? castsShadow(d.value) + : d.value.includes("drop-shadow"); + if (!casts) return; + for (const sel of (d.parent as Rule).selectors) found.add(clean(sel)); + }); + } return [...found].sort(); } From 1ac69b3bf3cd8d2f46aa69d8e391671f85bae9c4 Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 2 Sep 2026 12:20:05 -0700 Subject: [PATCH 6/9] test(web): fold case and walk every declaration in the style guards (#651, #652) --- .../03-fix-increment-2.md | 151 ++++++++++++++++++ web/src/styles.caps.test.ts | 13 +- web/src/styles.elevation.test.ts | 41 +++-- 3 files changed, 191 insertions(+), 14 deletions(-) create mode 100644 docs/plans/651-652-spa-elevation-and-caps/03-fix-increment-2.md diff --git a/docs/plans/651-652-spa-elevation-and-caps/03-fix-increment-2.md b/docs/plans/651-652-spa-elevation-and-caps/03-fix-increment-2.md new file mode 100644 index 000000000..a56f34926 --- /dev/null +++ b/docs/plans/651-652-spa-elevation-and-caps/03-fix-increment-2.md @@ -0,0 +1,151 @@ +# Fix increment 2 — fold case, and stop enumerating properties + +Review round 2, `false-green` seat. **All three holes driver-reproduced before dispatch.** + +## The findings + +Round 1's fix taught the guard about `filter`. Round 2 walked straight past it three ways. Observed by +the driver on head `2ca6b854`, each injected into `.panel` — a selector that must never cast a shadow: + +| Injected | Renders a shadow? | Guard result | +|---|---|---| +| `Filter: drop-shadow(0 8px 24px rgba(0,0,0,.3));` | yes | **26/26 green** — missed | +| `filter: DROP-SHADOW(0 8px 24px rgba(0,0,0,.3));` | yes | **26/26 green** — missed | +| `filter: var(--leaky-shadow);` | yes | **26/26 green** — missed | +| `filter: drop-shadow(0 8px 24px rgba(0,0,0,.3));` (control) | yes | 1 failed — caught | + +CSS property names and function names are both case-insensitive; `postcss`'s `walkDecls(prop, cb)` does an +exact `child.prop === prop` string comparison, and `.includes("drop-shadow")` is a literal lowercase +match. So the round-1 fix held only for the exact spelling it was written against. + +## The shape of the fix, and why the shape changed + +**Two misses of the same shape mean the method is wrong** — that is AGENTS.md's own guard rule, and it now +applies to this guard twice over. Round 1 missed `filter` because the walk enumerated `box-shadow`. Round 2 +missed three more spellings because the fix enumerated `["box-shadow", "filter", "-webkit-filter"]`. The +answer is not a longer list. It is to stop listing. + +So the walk visits **every declaration in the stylesheet**, folds case on both the property and the value, +and asks one question: does this declaration paint a shadow? That covers `Filter`, `FILTER`, +`-webkit-filter`, any vendor prefix nobody has thought of, and a custom property that *defines* a +drop-shadow for a `var()` to pick up later — the third hole, caught at the point of definition, so a +drop-shadow cannot enter the stylesheet unnoticed at all. + +Walking everything means the walk now reaches declarations inside `@font-face`, `@property` and +`@keyframes`, whose parents are not rules and have no `.selectors`. That is a crash, not a miss, so the +parent type is checked explicitly. + +## Files + +- Modify `web/src/styles.elevation.test.ts` — **PROTECTED**. +- Modify `web/src/styles.caps.test.ts` — **PROTECTED**. It carries the identical case-sensitivity defect + (`clean(d.value) !== "uppercase"` against a case-insensitive property and value). It was not the + reported finding, but shipping a fix for one guard while knowingly leaving its twin is precisely the + "reads as safety" failure AGENTS.md warns about. +- `web/src/styles.css` is NOT modified. The shipped stylesheet has been correct and unchanged since + `5cd6c387`; every defect in rounds 1 and 2 has been in the guards. + +## Step 1 — the elevation guard + +Replace the `SHADOW_PROPS` constant and the whole `selectorsCastingShadow` function with this. +**PROTECTED — transcribe or STOP.** + +```ts +// A shadow can be painted more ways than a list can keep up with. Round 1 of +// review found `filter: drop-shadow()` escaping a walk that enumerated +// `box-shadow`. Round 2 found `Filter:`, `DROP-SHADOW(` and +// `filter: var(--token)` escaping a walk that enumerated three property names. +// Two misses of the same shape mean the METHOD is wrong, so this walks EVERY +// declaration and asks one question of each, rather than listing the +// properties someone thought of. +// +// Case is folded on both sides: CSS property names and function names are +// case-insensitive, and postcss compares them as exact strings. +// +// A value that merely MENTIONS drop-shadow counts, which is what catches a +// custom property defining one for a later var() to pick up. The shadow is +// caught where it enters the stylesheet, not where it is used. +function selectorsCastingShadow(): string[] { + const found = new Set(); + root.walkDecls((d) => { + // Walking everything reaches declarations inside @font-face, @property and + // @keyframes, whose parent is not a rule and has no selectors. That is a + // crash rather than a miss, so it is excluded explicitly. + const parent = d.parent; + if (parent === undefined || parent.type !== "rule") return; + + const prop = d.prop.toLowerCase(); + const value = d.value.toLowerCase(); + const casts = prop === "box-shadow" + ? castsShadow(value) + : value.includes("drop-shadow"); + if (!casts) return; + + for (const sel of (parent as Rule).selectors) found.add(clean(sel)); + }); + return [...found].sort(); +} +``` + +## Step 2 — the caps guard + +In `web/src/styles.caps.test.ts`, replace the whole `selectorsUppercasing` function with this. +**PROTECTED — transcribe or STOP.** + +```ts +// Same case-folding defect as the elevation guard, found in review round 2 and +// fixed here rather than left as the twin of a hole we had just closed: +// `TEXT-TRANSFORM: UPPERCASE` renders identically and postcss compares both +// the property and the value as exact, case-sensitive strings. +function selectorsUppercasing(): string[] { + const found = new Set(); + root.walkDecls((d) => { + const parent = d.parent; + if (parent === undefined || parent.type !== "rule") return; + if (d.prop.toLowerCase() !== "text-transform") return; + if (clean(d.value).toLowerCase() !== "uppercase") return; + for (const sel of (parent as Rule).selectors) found.add(clean(sel)); + }); + return [...found].sort(); +} +``` + +## Step 3 — mutation rows + +Each: inject into `web/src/styles.css`, run, record, then `git checkout -- web/src/styles.css` and +confirm the restore. **Anchor every injection on a multi-line, unique string and confirm with `grep` +that the text is actually in the file before trusting the run** — a mutation that did not apply must +produce no result at all, never a green one. + +Run with `cd web && npx vitest run src/styles.elevation.test.ts src/styles.caps.test.ts`. + +| # | Injection (into `.panel` unless stated) | Must go | On which assertion | +|---|---|---|---| +| M14 | `Filter: drop-shadow(0 8px 24px rgba(0,0,0,.3));` | RED | non-inset selector set — round 2 finding 1 | +| M15 | `filter: DROP-SHADOW(0 8px 24px rgba(0,0,0,.3));` | RED | same set — round 2 finding 2 | +| M16 | `--leaky-shadow: drop-shadow(0 8px 24px rgba(0,0,0,.3));` into the top-level `:root` block | RED | same set — round 2 finding 3, caught at the definition. Expect the received array to gain `":root"`. | +| M17 | `filter: blur(2px);` | **GREEN** | proves no overreach: blur is not a shadow | +| M18 | `transition: filter 0.2s ease;` | **GREEN** | proves the value check, not the property name, is what decides | +| M19 | `TEXT-TRANSFORM: UPPERCASE;` on `.badge` | RED | the uppercase selector set — the caps guard's twin defect | +| M20 | `text-transform: capitalize;` on `.badge` | **GREEN** | capitalize is title case, not shouting; the invariant never claimed it | + +Four of the seven must stay GREEN. **If any of those four reddens, the fix overreaches and is wrong: +STOP and report rather than adjusting anything.** + +## Step 4 — gates and commit + +**G1** then **G2**, both clean against the baseline (`Test Files 109 passed (109)` / +`Tests 2396 passed (2396)`, coverage unmoved). + +```bash +git add web/src/styles.elevation.test.ts web/src/styles.caps.test.ts docs/plans/651-652-spa-elevation-and-caps/03-fix-increment-2.md +git commit -m "test(web): fold case and walk every declaration in the style guards (#651, #652)" +git push origin feat/651-652-spa-elevation-and-caps +``` + +Do NOT reply on GitHub, do NOT edit the PR description, do NOT merge. + +## Report back + +Commit SHA, `git status --short` verbatim, G1 and G2 output, the M14-M20 table as OBSERVED, and anything +here that turned out to be wrong. diff --git a/web/src/styles.caps.test.ts b/web/src/styles.caps.test.ts index 281d78d15..fed24fd66 100644 --- a/web/src/styles.caps.test.ts +++ b/web/src/styles.caps.test.ts @@ -19,11 +19,18 @@ const MODES: Mode[] = ["light", "dark"]; // The sidebar and the More-sheet group dividers. Nothing else. const CAPS_ALLOWED = [".more-group-label", ".nav-group-label"].sort(); +// Same case-folding defect as the elevation guard, found in review round 2 and +// fixed here rather than left as the twin of a hole we had just closed: +// `TEXT-TRANSFORM: UPPERCASE` renders identically and postcss compares both +// the property and the value as exact, case-sensitive strings. function selectorsUppercasing(): string[] { const found = new Set(); - root.walkDecls("text-transform", (d) => { - if (clean(d.value) !== "uppercase") return; - for (const sel of (d.parent as Rule).selectors) found.add(clean(sel)); + root.walkDecls((d) => { + const parent = d.parent; + if (parent === undefined || parent.type !== "rule") return; + if (d.prop.toLowerCase() !== "text-transform") return; + if (clean(d.value).toLowerCase() !== "uppercase") return; + for (const sel of (parent as Rule).selectors) found.add(clean(sel)); }); return [...found].sort(); } diff --git a/web/src/styles.elevation.test.ts b/web/src/styles.elevation.test.ts index f1f78c9a4..b71d25ecf 100644 --- a/web/src/styles.elevation.test.ts +++ b/web/src/styles.elevation.test.ts @@ -59,19 +59,38 @@ function castsShadow(value: string): boolean { // This targets drop-shadow, not `filter` wholesale: blur() and brightness() // are not shadows, and banning them would assert something the invariant never // claimed. The stylesheet declares no filter at all today. -const SHADOW_PROPS = ["box-shadow", "filter", "-webkit-filter"]; - +// A shadow can be painted more ways than a list can keep up with. Round 1 of +// review found `filter: drop-shadow()` escaping a walk that enumerated +// `box-shadow`. Round 2 found `Filter:`, `DROP-SHADOW(` and +// `filter: var(--token)` escaping a walk that enumerated three property names. +// Two misses of the same shape mean the METHOD is wrong, so this walks EVERY +// declaration and asks one question of each, rather than listing the +// properties someone thought of. +// +// Case is folded on both sides: CSS property names and function names are +// case-insensitive, and postcss compares them as exact strings. +// +// A value that merely MENTIONS drop-shadow counts, which is what catches a +// custom property defining one for a later var() to pick up. The shadow is +// caught where it enters the stylesheet, not where it is used. function selectorsCastingShadow(): string[] { const found = new Set(); - for (const prop of SHADOW_PROPS) { - root.walkDecls(prop, (d) => { - const casts = prop === "box-shadow" - ? castsShadow(d.value) - : d.value.includes("drop-shadow"); - if (!casts) return; - for (const sel of (d.parent as Rule).selectors) found.add(clean(sel)); - }); - } + root.walkDecls((d) => { + // Walking everything reaches declarations inside @font-face, @property and + // @keyframes, whose parent is not a rule and has no selectors. That is a + // crash rather than a miss, so it is excluded explicitly. + const parent = d.parent; + if (parent === undefined || parent.type !== "rule") return; + + const prop = d.prop.toLowerCase(); + const value = d.value.toLowerCase(); + const casts = prop === "box-shadow" + ? castsShadow(value) + : value.includes("drop-shadow"); + if (!casts) return; + + for (const sel of (parent as Rule).selectors) found.add(clean(sel)); + }); return [...found].sort(); } From 21f5f05627b84abf273285dcabcd7ead9efa84a0 Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 2 Sep 2026 13:04:22 -0700 Subject: [PATCH 7/9] test(web): a keyframe is not a surface that floats (#651, #652) --- .../03-fix-increment-2.md | 8 +- .../04-fix-increment-3.md | 132 ++++++++++++++++++ web/src/styles.caps.test.ts | 25 +++- web/src/styles.elevation.test.ts | 25 +++- 4 files changed, 186 insertions(+), 4 deletions(-) create mode 100644 docs/plans/651-652-spa-elevation-and-caps/04-fix-increment-3.md diff --git a/docs/plans/651-652-spa-elevation-and-caps/03-fix-increment-2.md b/docs/plans/651-652-spa-elevation-and-caps/03-fix-increment-2.md index a56f34926..459783ed8 100644 --- a/docs/plans/651-652-spa-elevation-and-caps/03-fix-increment-2.md +++ b/docs/plans/651-652-spa-elevation-and-caps/03-fix-increment-2.md @@ -11,9 +11,13 @@ the driver on head `2ca6b854`, each injected into `.panel` — a selector that m |---|---|---| | `Filter: drop-shadow(0 8px 24px rgba(0,0,0,.3));` | yes | **26/26 green** — missed | | `filter: DROP-SHADOW(0 8px 24px rgba(0,0,0,.3));` | yes | **26/26 green** — missed | -| `filter: var(--leaky-shadow);` | yes | **26/26 green** — missed | +| `filter: var(--leaky-shadow);` | no — the token is undefined, so it resolves to nothing | **26/26 green** — missed | | `filter: drop-shadow(0 8px 24px rgba(0,0,0,.3));` (control) | yes | 1 failed — caught | +The var() row is the weakest of the three: it shows the guard does not read through a custom property, +but the injected value rendered nothing because the token was never defined. M16 is the form that both +renders and is caught. + CSS property names and function names are both case-insensitive; `postcss`'s `walkDecls(prop, cb)` does an exact `child.prop === prop` string comparison, and `.includes("drop-shadow")` is a literal lowercase match. So the round-1 fix held only for the exact spelling it was written against. @@ -129,7 +133,7 @@ Run with `cd web && npx vitest run src/styles.elevation.test.ts src/styles.caps. | M19 | `TEXT-TRANSFORM: UPPERCASE;` on `.badge` | RED | the uppercase selector set — the caps guard's twin defect | | M20 | `text-transform: capitalize;` on `.badge` | **GREEN** | capitalize is title case, not shouting; the invariant never claimed it | -Four of the seven must stay GREEN. **If any of those four reddens, the fix overreaches and is wrong: +Three of the seven must stay GREEN. **If any of those three reddens, the fix overreaches and is wrong: STOP and report rather than adjusting anything.** ## Step 4 — gates and commit diff --git a/docs/plans/651-652-spa-elevation-and-caps/04-fix-increment-3.md b/docs/plans/651-652-spa-elevation-and-caps/04-fix-increment-3.md new file mode 100644 index 000000000..4eafb4c63 --- /dev/null +++ b/docs/plans/651-652-spa-elevation-and-caps/04-fix-increment-3.md @@ -0,0 +1,132 @@ +# Fix increment 3 — a keyframe is not a surface + +Review round 3, CodeRabbit on head `1ac69b3b`. **Driver-reproduced before dispatch.** + +## The finding + +Fix increment 2 stopped enumerating property names and walked every declaration instead. Walking +everything reaches `@keyframes`, and a keyframe frame **is** a postcss rule — its selector is `from`, `to` +or a percentage — so the `parent.type !== "rule"` guard added in that increment does not exclude it. + +**Observed by the driver on head `1ac69b3b`:** adding +`from { box-shadow: 0 8px 24px rgba(0,0,0,0.2); }` to `@keyframes dialog-rise` — a perfectly legitimate +animation of a shadow the dialog is already allowed to cast — turns the guard **RED**, because `from` +lands in the casting-shadow set and is not in `SHADOW_ALLOWED`. + +That is a false alarm the fix introduced, and it is the more damaging kind: it does not let a regression +through, it blocks correct work. Nobody hits it today because no keyframe animates a shadow, so it would +have sat latent until the first person tried. + +**The guard's invariant is about which SURFACES float.** A keyframe is a point in time, not a surface. + +## Also corrected here, from the same review round + +1. `03-fix-increment-2.md` says "Four of the seven must stay GREEN". Its own table marks three — M17, M18 + and M20. The implementer noticed and followed the table. Prose corrected to match. +2. `03-fix-increment-2.md`'s findings table claims `filter: var(--leaky-shadow)` renders a shadow. It does + not: `--leaky-shadow` is defined nowhere, so the value resolves to nothing. The `var()` indirection gap + is real, but that row overstated the demonstration. Corrected to say what was actually observed — + the guard did not flag it, and the reason it renders nothing is that the token is undefined. **M16 + remains valid and unchanged**: it injects the custom property into `:root`, which is the form that both + renders and is caught. + +**Refuted, with the reason, and NOT changed:** the same round asked for `G1` to be redefined as +`cd web && npm test`. G1 is the build gate and G2 the test gate, fixed by +`01-implementer-runbook.md`'s gate table and copied from `.github/workflows/ci.yml`'s `web` job +(`npm run build`, `npm run test:coverage`). Renumbering them here would put two different definitions of +G1 in one plan directory. The suggestion to avoid `git checkout -- web/src/styles.css` for mutation +cleanup is also declined: the runbook already requires a clean tree before mutating, and a targeted +restore is exactly what makes "no net diff" checkable afterwards. + +## Files + +- Modify `web/src/styles.elevation.test.ts` — **PROTECTED**. +- Modify `web/src/styles.caps.test.ts` — **PROTECTED**. +- Modify `docs/plans/651-652-spa-elevation-and-caps/03-fix-increment-2.md` — the two corrections above. +- `web/src/styles.css` is NOT modified. + +## Step 1 — the shared exclusion + +In **both** guard files, add this helper immediately above the `selectorsCastingShadow` / +`selectorsUppercasing` function, and extend the import on line 5 of each file to +`import type { AtRule, Node, Rule } from "postcss";`. **PROTECTED — transcribe or STOP.** + +```ts +// A @keyframes frame IS a rule — its selector is `from`, `to` or a percentage — +// so the rule-parent check below does not exclude it, and walking every +// declaration reaches inside animations. An animation that legitimately tweens +// a shadow would otherwise put `from` into the set and fail a guard that is +// about which SURFACES float. A keyframe is a point in time, not a surface. +// +// Found by review round 3 against increment 2's own fix: it does not let a +// regression through, it blocks correct work, which is why it was latent — +// no keyframe animates a shadow today. +// +// The name test is suffix-based so it covers `-webkit-keyframes` too, and it +// walks the whole ancestor chain because a keyframes block can itself be +// nested inside a @media. +function insideKeyframes(node: Node | undefined): boolean { + let current: Node | undefined = node; + while (current !== undefined) { + if (current.type === "atrule" && /(^|-)keyframes$/i.test((current as AtRule).name)) return true; + current = current.parent as Node | undefined; + } + return false; +} +``` + +## Step 2 — use it in both walkers + +In `web/src/styles.elevation.test.ts`, inside `selectorsCastingShadow`'s callback, immediately after the +existing `if (parent === undefined || parent.type !== "rule") return;` line, add: + +```ts + if (insideKeyframes(parent)) return; +``` + +In `web/src/styles.caps.test.ts`, inside `selectorsUppercasing`'s callback, in the same position, add the +identical line. + +## Step 3 — mutation rows + +Same discipline: unique multi-line anchor, `grep` to confirm the injection is in the file, no result line +for a row that did not apply, `git checkout -- web/src/styles.css` between rows. + +| # | Injection | Must go | Why | +|---|---|---|---| +| M21 | `from { box-shadow: 0 8px 24px rgba(0,0,0,0.2); }` inside `@keyframes dialog-rise` | **GREEN** | the finding itself. **Before this increment it is RED — the driver observed that on `1ac69b3b`.** | +| M22 | `from { filter: drop-shadow(0 8px 24px rgba(0,0,0,0.2)); }` inside `@keyframes dialog-rise` | **GREEN** | the same exclusion via the other mechanism | +| M23 | `from { text-transform: uppercase; }` inside `@keyframes dialog-rise` | **GREEN** | the caps guard's half of the same exclusion | +| M24 | `box-shadow: var(--shadow-dialog);` on `.panel` | RED | **the regression control.** Proves the keyframes exclusion did not blind the guard to an ordinary rule. If this goes green the fix has broken the guard entirely. | +| M25 | `filter: drop-shadow(0 8px 24px rgba(0,0,0,.3));` on `.panel` | RED | the same control through the filter mechanism | + +**M24 and M25 are the rows that matter.** An exclusion that silences the guard everywhere would make +M21-M23 pass for the wrong reason; those two are what tell the difference. + +## Step 4 — the two documentation corrections + +In `docs/plans/651-652-spa-elevation-and-caps/03-fix-increment-2.md`: + +1. Change `**Four of the seven** must stay GREEN` to `**Three of the seven** must stay GREEN`, and + `If any of those four reddens` to `If any of those three reddens`. +2. In the findings table at the top, change the `filter: var(--leaky-shadow);` row's second column from + `yes` to `no — the token is undefined, so it resolves to nothing`, and leave its third column + (`26/26 green — missed`) as it is, adding after the table: + `The var() row is the weakest of the three: it shows the guard does not read through a custom property, but the injected value rendered nothing because the token was never defined. M16 is the form that both renders and is caught.` + +## Step 5 — gates and commit + +**G1** then **G2** as defined in `01-implementer-runbook.md`'s gate table — unchanged, do not redefine them. + +```bash +git add web/src/styles.elevation.test.ts web/src/styles.caps.test.ts docs/plans/651-652-spa-elevation-and-caps/ +git commit -m "test(web): a keyframe is not a surface that floats (#651, #652)" +git push origin feat/651-652-spa-elevation-and-caps +``` + +Do NOT reply on GitHub, do NOT edit the PR description, do NOT merge. + +## Report back + +Commit SHA, `git status --short`, G1 and G2 output, the M21-M25 table as OBSERVED, and anything here that +turned out to be wrong. diff --git a/web/src/styles.caps.test.ts b/web/src/styles.caps.test.ts index fed24fd66..a7949472b 100644 --- a/web/src/styles.caps.test.ts +++ b/web/src/styles.caps.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import { readFileSync } from "node:fs"; import { resolve } from "node:path"; import postcss from "postcss"; -import type { Rule } from "postcss"; +import type { AtRule, Node, Rule } from "postcss"; import { contrast, resolveTokens, type Mode } from "./test/cssTokens"; import { BRANDS, DEFAULT_BRAND } from "./lib/brand"; @@ -23,11 +23,34 @@ const CAPS_ALLOWED = [".more-group-label", ".nav-group-label"].sort(); // fixed here rather than left as the twin of a hole we had just closed: // `TEXT-TRANSFORM: UPPERCASE` renders identically and postcss compares both // the property and the value as exact, case-sensitive strings. +// A @keyframes frame IS a rule — its selector is `from`, `to` or a percentage — +// so the rule-parent check below does not exclude it, and walking every +// declaration reaches inside animations. An animation that legitimately tweens +// a shadow would otherwise put `from` into the set and fail a guard that is +// about which SURFACES float. A keyframe is a point in time, not a surface. +// +// Found by review round 3 against increment 2's own fix: it does not let a +// regression through, it blocks correct work, which is why it was latent — +// no keyframe animates a shadow today. +// +// The name test is suffix-based so it covers `-webkit-keyframes` too, and it +// walks the whole ancestor chain because a keyframes block can itself be +// nested inside a @media. +function insideKeyframes(node: Node | undefined): boolean { + let current: Node | undefined = node; + while (current !== undefined) { + if (current.type === "atrule" && /(^|-)keyframes$/i.test((current as AtRule).name)) return true; + current = current.parent as Node | undefined; + } + return false; +} + function selectorsUppercasing(): string[] { const found = new Set(); root.walkDecls((d) => { const parent = d.parent; if (parent === undefined || parent.type !== "rule") return; + if (insideKeyframes(parent)) return; if (d.prop.toLowerCase() !== "text-transform") return; if (clean(d.value).toLowerCase() !== "uppercase") return; for (const sel of (parent as Rule).selectors) found.add(clean(sel)); diff --git a/web/src/styles.elevation.test.ts b/web/src/styles.elevation.test.ts index b71d25ecf..0f56f716a 100644 --- a/web/src/styles.elevation.test.ts +++ b/web/src/styles.elevation.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import { readFileSync } from "node:fs"; import { resolve } from "node:path"; import postcss from "postcss"; -import type { Rule } from "postcss"; +import type { AtRule, Node, Rule } from "postcss"; // #651 — elevation encodes what floats; radius encodes nesting depth. // @@ -73,6 +73,28 @@ function castsShadow(value: string): boolean { // A value that merely MENTIONS drop-shadow counts, which is what catches a // custom property defining one for a later var() to pick up. The shadow is // caught where it enters the stylesheet, not where it is used. +// A @keyframes frame IS a rule — its selector is `from`, `to` or a percentage — +// so the rule-parent check below does not exclude it, and walking every +// declaration reaches inside animations. An animation that legitimately tweens +// a shadow would otherwise put `from` into the set and fail a guard that is +// about which SURFACES float. A keyframe is a point in time, not a surface. +// +// Found by review round 3 against increment 2's own fix: it does not let a +// regression through, it blocks correct work, which is why it was latent — +// no keyframe animates a shadow today. +// +// The name test is suffix-based so it covers `-webkit-keyframes` too, and it +// walks the whole ancestor chain because a keyframes block can itself be +// nested inside a @media. +function insideKeyframes(node: Node | undefined): boolean { + let current: Node | undefined = node; + while (current !== undefined) { + if (current.type === "atrule" && /(^|-)keyframes$/i.test((current as AtRule).name)) return true; + current = current.parent as Node | undefined; + } + return false; +} + function selectorsCastingShadow(): string[] { const found = new Set(); root.walkDecls((d) => { @@ -81,6 +103,7 @@ function selectorsCastingShadow(): string[] { // crash rather than a miss, so it is excluded explicitly. const parent = d.parent; if (parent === undefined || parent.type !== "rule") return; + if (insideKeyframes(parent)) return; const prop = d.prop.toLowerCase(); const value = d.value.toLowerCase(); From e09f02d79213cf18927a8a518ab553af27299e7c Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 2 Sep 2026 14:01:19 -0700 Subject: [PATCH 8/9] fix(web): capitalise the two badge labels that relied on CSS to shout (#652) --- .../05-fix-increment-4.md | 172 ++++++++++++++++++ web/src/i18n/badgeCase.test.ts | 37 ++++ web/src/i18n/en.ts | 8 +- web/src/i18n/es.ts | 8 +- web/src/i18n/tl.ts | 8 +- web/src/routes/Dashboard.test.tsx | 2 +- web/src/routes/GradesPage.test.tsx | 4 +- web/src/routes/HelpPage.test.tsx | 2 +- 8 files changed, 225 insertions(+), 16 deletions(-) create mode 100644 docs/plans/651-652-spa-elevation-and-caps/05-fix-increment-4.md create mode 100644 web/src/i18n/badgeCase.test.ts diff --git a/docs/plans/651-652-spa-elevation-and-caps/05-fix-increment-4.md b/docs/plans/651-652-spa-elevation-and-caps/05-fix-increment-4.md new file mode 100644 index 000000000..b575e22a1 --- /dev/null +++ b/docs/plans/651-652-spa-elevation-and-caps/05-fix-increment-4.md @@ -0,0 +1,172 @@ +# Fix increment 4 — two badge strings that relied on CSS to capitalise them + +**The first genuine PRODUCT defect of this slice.** Every earlier round found defects in the guards or +the documentation; this one is visible to a user on the dashboard. + +Found by the driver comparing before/after screenshots of the real built SPA — not by any review seat, +because it is not visible in a selector sweep or a stylesheet walk. #652's own acceptance criterion +named this exact risk: *"verify nothing relied on CSS to capitalise"*. Two strings did. + +## The finding + +`.badge` used to apply `text-transform: uppercase`. Two badge strings were therefore authored in lower +case at source, because the stylesheet made them read as capitals. With the transform gone they render +exactly as written, which reads as a typo rather than as sentence case. + +| Key | Rendered by | en | es | tl | Was | Now | +|---|---|---|---|---|---|---| +| `noEntryBadge` | `web/src/routes/Dashboard.tsx:150`, `` | `no entry` | `sin registro` | `walang tala` | `NO ENTRY` | `no entry` | +| `saleableYesBadge` | `web/src/routes/GradesPage.tsx:270`, `` | `yes` | `sí` | `oo` | `YES` | `yes` | + +**The set is complete, not a sample.** Every `*Badge` key in every locale was enumerated: four per +locale, and exactly these two start lower case in each. `editingDraftBadge` and `disabledBadge` are +already capitalised and render correctly — the daily-entry screenshot confirms `Editing draft`. + +Eight other lower-case-initial strings were checked and are NOT affected: they render through `.muted`, +`.sr-only`, `.sidebar-version` or plain labels, none of which ever uppercased. + +## Files + +- `web/src/i18n/en.ts`, `web/src/i18n/es.ts`, `web/src/i18n/tl.ts` +- `web/src/i18n/badgeCase.test.ts` — new guard, **PROTECTED** +- `web/src/routes/Dashboard.test.tsx` and `web/src/routes/HelpPage.test.tsx` — two assertions pin the old + text and must move with it +- `web/src/styles.css` is NOT touched. + +## Step 1 — the strings + +**`noEntryBadge`** — and, because the Help page and glossary quote this badge by name, its three quoted +copies move with it. AGENTS.md treats a missing doc update like a missing test, and here the doc quotes +the exact string. + +| File | Line | From | To | +|---|---|---|---| +| `en.ts` | 626 | `noEntryBadge: "no entry",` | `noEntryBadge: "No entry",` | +| `en.ts` | ~2404 | `A tile marked no entry is` | `A tile marked No entry is` | +| `en.ts` | ~3186 | `"no entry" is the alarm state` | `"No entry" is the alarm state` | +| `es.ts` | 517 | `noEntryBadge: "sin registro",` | `noEntryBadge: "Sin registro",` | +| `es.ts` | ~1878 | `marcada sin registro` | `marcada Sin registro` | +| `es.ts` | ~2692 | `"sin registro" es el estado de alarma` | `"Sin registro" es el estado de alarma` | +| `tl.ts` | 539 | `noEntryBadge: "walang tala",` | `noEntryBadge: "Walang tala",` | +| `tl.ts` | ~1951 | `may walang tala` | `may Walang tala` | +| `tl.ts` | ~2789 | `ang "walang tala" ang alarm state` | `ang "Walang tala" ang alarm state` | + +**Do NOT change `tileLinkLabelMissing`** (`en.ts:635` and its siblings). That is a screen-reader +sentence — `"{{flock}}: no entry yet, open today's entry"` — where the phrase is prose mid-sentence, not +the badge label. Capitalising it would be wrong. + +**`saleableYesBadge`** — no Help or glossary copy quotes it (the `saleable` in the Help +text quotes the column label `saleableLabel`, which is unchanged). + +| File | From | To | +|---|---|---| +| `en.ts` | `saleableYesBadge: "yes",` | `saleableYesBadge: "Yes",` | +| `es.ts` | `saleableYesBadge: "sí",` | `saleableYesBadge: "Sí",` | +| `tl.ts` | `saleableYesBadge: "oo",` | `saleableYesBadge: "Oo",` | + +Mind the accent: the Spanish value is `Sí`, not `Si`. + +## Step 2 — the guard + +Create `web/src/i18n/badgeCase.test.ts`. **PROTECTED — transcribe or STOP.** + +```ts +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; + +// #652 removed `text-transform: uppercase` from `.badge`. Two badge strings had +// been authored in lower case BECAUSE the stylesheet capitalised them, so they +// began rendering as "no entry" and "yes" — which reads as a typo, not as +// sentence case. A user saw it on the dashboard; no selector sweep or +// stylesheet walk can, because the defect is in the text, not the CSS. +// +// The convention this pins: a key ending in `Badge` is rendered inside a +// `.badge` pill, and a pill's label starts with a capital. Enumerated from the +// locale files rather than listed here, so a new badge string is covered the +// day it is added rather than the day someone remembers this test. +const LOCALES = ["en", "es", "tl"] as const; + +function badgeStrings(locale: string): [string, string][] { + const src = readFileSync(resolve(process.cwd(), `src/i18n/${locale}.ts`), "utf8"); + const out: [string, string][] = []; + const re = /^\s*([A-Za-z0-9_]*Badge)\s*:\s*"([^"]*)"/gm; + for (let m = re.exec(src); m !== null; m = re.exec(src)) out.push([m[1], m[2]]); + return out; +} + +describe.each(LOCALES)("%s: badge labels are capitalised", (locale) => { + const entries = badgeStrings(locale); + + it("finds badge strings at all — a regex that matches nothing would pass every assertion below", () => { + expect(entries.length).toBeGreaterThanOrEqual(4); + }); + + it.each(entries)("%s starts with a capital", (_key, value) => { + // Interpolation placeholders and digits are legitimate openers; a lower-case + // letter is the defect this guard exists for. + expect(value[0]).not.toMatch(/\p{Ll}/u); + }); +}); +``` + +The first test is deliberate: a regex that stopped matching would leave `it.each([])` asserting nothing +and the file would pass while guarding nothing. That is the shape of false green this slice has already +hit twice. + +## Step 3 — the two pinned assertions + +- `web/src/routes/Dashboard.test.tsx:115` — `getByText("no entry")` becomes `getByText("No entry")`. +- `web/src/routes/HelpPage.test.tsx:1101` — `getByText("no entry", { selector: "strong" })` becomes + `getByText("No entry", { selector: "strong" })`. +- `web/src/routes/GradesPage.test.tsx:64` — `getByText("yes")` becomes `getByText("Yes")`. +- `web/src/routes/GradesPage.test.tsx:459` — `queryByText("yes")` becomes `queryByText("Yes")`. + +**Amendment, added after the first dispatch stopped on it.** The first two were all the driver's caller +ledger listed, and the ledger was wrong: it was built by grepping for the i18n KEY names plus the single +literal `"no entry"`, and never grepped the literal `"yes"`. The implementer hit the miss at G2, stopped +rather than fixing an unlisted file, and reported it — which is exactly the behaviour the runbook asks +for, and the reason the defect surfaced as a question instead of as a silent scope creep. + +The corrected method, which is what found the fourth site: **grep every VALUE of every changed key, in +every locale, across every test and spec file** — not the key names, and not one representative literal. +A key name appears where the string is produced; the literal appears where it is asserted, and those are +different files. + +Line 459 is the subtler of the two. It sits inside a `withOverride("grades", "saleableYesBadge", +"YES-MARKER", ...)` block and asserts the DEFAULT label is absent while the override is active. It does +not fail after the rename — `queryByText` takes an exact, case-sensitive string, so `"yes"` simply stops +matching anything — but it stops testing what it claims: it would pass even if the override did nothing. +A test that passes for the wrong reason is the `false-green` class this slice has already paid for +twice, so it moves with the value rather than being left green. + +Change only the expected string. **If either test then fails for any other reason, STOP and report** — +that means something else depended on the old text. + +## Step 4 — mutation rows + +| # | Mutation | Must go | On which assertion | +|---|---|---|---| +| M27 | revert `en.ts` `noEntryBadge` to `"no entry"` | RED | `en: badge labels are capitalised > noEntryBadge starts with a capital` | +| M28 | revert `tl.ts` `saleableYesBadge` to `"oo"` | RED | the `tl` case of the same assertion | +| M29 | change `es.ts` `saleableLabel` to lower case (it already is) — i.e. confirm a NON-`Badge` lower-case string does not trip the guard | **GREEN** | proves the guard is scoped to the badge convention and is not just "no lower-case strings anywhere" | +| M30 | break the regex: change `Badge` to `Badgez` in the guard's own pattern | RED | `finds badge strings at all` — proves the empty-match false green is closed | + +M30 mutates the test file rather than the product; restore it exactly. + +## Step 5 — gates and commit + +**G1**, then **G2**. The suite count rises by the new guard's cases. + +```bash +git add web/src/i18n/ web/src/routes/Dashboard.test.tsx web/src/routes/HelpPage.test.tsx web/src/i18n/badgeCase.test.ts docs/plans/651-652-spa-elevation-and-caps/ +git commit -m "fix(web): capitalise the two badge labels that relied on CSS to shout (#652)" +git push origin feat/651-652-spa-elevation-and-caps +``` + +Do NOT reply on GitHub, do NOT edit the PR description, do NOT merge. + +## Report back + +Commit SHA, `git status --short`, G1 and G2 output, the M27-M30 table as OBSERVED, and the new suite +totals. diff --git a/web/src/i18n/badgeCase.test.ts b/web/src/i18n/badgeCase.test.ts new file mode 100644 index 000000000..4d8187a52 --- /dev/null +++ b/web/src/i18n/badgeCase.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; + +// #652 removed `text-transform: uppercase` from `.badge`. Two badge strings had +// been authored in lower case BECAUSE the stylesheet capitalised them, so they +// began rendering as "no entry" and "yes" — which reads as a typo, not as +// sentence case. A user saw it on the dashboard; no selector sweep or +// stylesheet walk can, because the defect is in the text, not the CSS. +// +// The convention this pins: a key ending in `Badge` is rendered inside a +// `.badge` pill, and a pill's label starts with a capital. Enumerated from the +// locale files rather than listed here, so a new badge string is covered the +// day it is added rather than the day someone remembers this test. +const LOCALES = ["en", "es", "tl"] as const; + +function badgeStrings(locale: string): [string, string][] { + const src = readFileSync(resolve(process.cwd(), `src/i18n/${locale}.ts`), "utf8"); + const out: [string, string][] = []; + const re = /^\s*([A-Za-z0-9_]*Badge)\s*:\s*"([^"]*)"/gm; + for (let m = re.exec(src); m !== null; m = re.exec(src)) out.push([m[1], m[2]]); + return out; +} + +describe.each(LOCALES)("%s: badge labels are capitalised", (locale) => { + const entries = badgeStrings(locale); + + it("finds badge strings at all — a regex that matches nothing would pass every assertion below", () => { + expect(entries.length).toBeGreaterThanOrEqual(4); + }); + + it.each(entries)("%s starts with a capital", (_key, value) => { + // Interpolation placeholders and digits are legitimate openers; a lower-case + // letter is the defect this guard exists for. + expect(value[0]).not.toMatch(/\p{Ll}/u); + }); +}); diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 6384cd33a..a32359e78 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -623,7 +623,7 @@ export const en = { // screen and links straight to Daily entry for that flock and day. todayPanelTitle: "Today", noFlocksMessage: "No flocks yet — create one on the Daily entry page.", - noEntryBadge: "no entry", + noEntryBadge: "No entry", // {{total}} is the farm-locale formatted figure (#650). todayEggsTotal: "{{total}} eggs today", // aria-label of a tile link; the visible tile shows name, eggs and status. @@ -879,7 +879,7 @@ export const en = { // Saleable column's "yes" badge — lowercase, distinct from common.yes // ("Yes"); same case-sensitive-variant treatment as sales:close. - saleableYesBadge: "yes", + saleableYesBadge: "Yes", }, // Feed & inventory catalog + receiving/usage/correction screen (Task 16, // #182, batch B3 — the biggest B3 screen). `inventory` is in @@ -2401,7 +2401,7 @@ export const en = { dashboardHeading: "Dashboard", dashboardTiles: "Today: one tile per active flock with today's eggs and status, the ones without " - + "an entry first. A tile marked no entry is the house nobody has recorded yet — tap " + + "an entry first. A tile marked No entry is the house nobody has recorded yet — tap " + "it to open Daily entry for that flock and day. Past twelve tiles, a link carries the rest.", dashboardTrend: "Last 14 days: eggs per day from the production report, with hen-day %" @@ -3183,7 +3183,7 @@ export const en = { glossaryCaptureStatusTerm: "Capture status", glossaryCaptureStatusDef: "Whether each active flock has a daily entry for today. The dashboard shows it as one tile per " - + "flock, missing ones first; \"no entry\" is the alarm state and links to Daily entry for that flock and day.", + + "flock, missing ones first; \"No entry\" is the alarm state and links to Daily entry for that flock and day.", glossaryEggLotTerm: "Egg lot", glossaryEggLotDef: diff --git a/web/src/i18n/es.ts b/web/src/i18n/es.ts index 240cac71c..0dffe650a 100644 --- a/web/src/i18n/es.ts +++ b/web/src/i18n/es.ts @@ -514,7 +514,7 @@ export const es = { // Estado de captura (#654) todayPanelTitle: "Hoy", noFlocksMessage: "Aún no hay lotes — cree uno en la página de Registro diario.", - noEntryBadge: "sin registro", + noEntryBadge: "Sin registro", todayEggsTotal: "{{total}} huevos hoy", tileLinkLabel: "{{flock}}: abrir el registro de hoy", tileLinkLabelMissing: "{{flock}}: sin registro todavía, abrir el registro de hoy", @@ -697,7 +697,7 @@ export const es = { statusHeader: "Estado", // Saleable column's "yes" badge - saleableYesBadge: "sí", + saleableYesBadge: "Sí", }, // machine-drafted (#182) — pending native review. Task CT3 (B3 catch-up): @@ -1875,7 +1875,7 @@ export const es = { dashboardHeading: "Panel", dashboardTiles: "Hoy: una tarjeta por lote activo con los huevos y el estado de hoy, primero los que " - + "no tienen registro. Una tarjeta marcada sin registro es la nave que nadie ha " + + "no tienen registro. Una tarjeta marcada Sin registro es la nave que nadie ha " + "registrado todavía — tóquela para abrir el Registro diario de ese lote y ese día. Pasadas doce " + "tarjetas, un enlace lleva al resto.", dashboardTrend: @@ -2689,7 +2689,7 @@ export const es = { glossaryCaptureStatusTerm: "Estado de captura", glossaryCaptureStatusDef: "Si cada lote activo tiene un registro diario de hoy. El panel lo muestra como una tarjeta por " - + "lote, primero los que faltan; \"sin registro\" es el estado de alarma y enlaza al Registro diario " + + "lote, primero los que faltan; \"Sin registro\" es el estado de alarma y enlaza al Registro diario " + "de ese lote y ese día.", glossaryEggLotTerm: "Lote de huevos", diff --git a/web/src/i18n/tl.ts b/web/src/i18n/tl.ts index ecce075f4..284eacc83 100644 --- a/web/src/i18n/tl.ts +++ b/web/src/i18n/tl.ts @@ -536,7 +536,7 @@ export const tl = { // Katayuan ng pagtatala (#654) todayPanelTitle: "Ngayon", noFlocksMessage: "Wala pang kawan — gumawa ng isa sa page na Araw-araw na Tala.", - noEntryBadge: "walang tala", + noEntryBadge: "Walang tala", todayEggsTotal: "{{total}} itlog ngayon", tileLinkLabel: "{{flock}}: buksan ang tala ngayong araw", tileLinkLabelMissing: "{{flock}}: wala pang tala, buksan ang tala ngayong araw", @@ -724,7 +724,7 @@ export const tl = { statusHeader: "Katayuan", // Saleable column's "yes" badge - saleableYesBadge: "oo", + saleableYesBadge: "Oo", }, // machine-drafted (#182) — pending native review. Task CT3 (B3 catch-up): @@ -1948,7 +1948,7 @@ export const tl = { dashboardHeading: "Dashboard", dashboardTiles: "Ngayon: isang tile bawat aktibong kawan na may itlog at katayuan ngayong araw, " - + "una ang mga walang tala. Ang tile na may walang tala ang bahay na wala pang " + + "una ang mga walang tala. Ang tile na may Walang tala ang bahay na wala pang " + "nagtatala — pindutin ito para buksan ang Araw-araw na Tala para sa kawan at araw na iyon. Lampas " + "sa labindalawang tile, isang link ang magdadala sa iba.", dashboardTrend: @@ -2786,7 +2786,7 @@ export const tl = { glossaryCaptureStatusTerm: "Katayuan ng pagtatala", glossaryCaptureStatusDef: "Kung may araw-araw na tala ngayong araw ang bawat aktibong kawan. Ipinapakita ito ng dashboard " - + "bilang isang tile bawat kawan, una ang mga kulang; ang \"walang tala\" ang alarm state at " + + "bilang isang tile bawat kawan, una ang mga kulang; ang \"Walang tala\" ang alarm state at " + "nakalink sa Araw-araw na Tala para sa kawan at araw na iyon.", glossaryEggLotTerm: "Lote ng itlog", diff --git a/web/src/routes/Dashboard.test.tsx b/web/src/routes/Dashboard.test.tsx index 55225b4c6..9466b59df 100644 --- a/web/src/routes/Dashboard.test.tsx +++ b/web/src/routes/Dashboard.test.tsx @@ -112,7 +112,7 @@ describe("Dashboard capture status (#654)", () => { const f3 = screen.getByRole("link", { name: "Flock f3: no entry yet, open today's entry" }); for (const tile of [f2, f3]) { expect(tile).toHaveClass("is-missing"); - expect(within(tile).getByText("no entry")).toBeInTheDocument(); + expect(within(tile).getByText("No entry")).toBeInTheDocument(); expect(within(tile).getByText("—")).toBeInTheDocument(); expect(within(tile).queryByText("999")).not.toBeInTheDocument(); } diff --git a/web/src/routes/GradesPage.test.tsx b/web/src/routes/GradesPage.test.tsx index 57c0c9219..5d01218f1 100644 --- a/web/src/routes/GradesPage.test.tsx +++ b/web/src/routes/GradesPage.test.tsx @@ -61,7 +61,7 @@ describe("GradesPage display", () => { it("renders each grade with saleable + status columns", async () => { await renderReady(ADMIN); const rowA = screen.getByRole("row", { name: /Grade A/ }); - expect(within(rowA).getByText("yes")).toBeInTheDocument(); // saleable + expect(within(rowA).getByText("Yes")).toBeInTheDocument(); // saleable expect(within(rowA).getByText("Active")).toBeInTheDocument(); const rowOld = screen.getByRole("row", { name: /Legacy/ }); // By cell, not by text: the record-history column (#494) also renders "—" @@ -456,7 +456,7 @@ describe("GradesPage i18n wiring (#182, Task 14)", () => { await renderReady(ADMIN); const rowA = screen.getByRole("row", { name: /Grade A/ }); expect(within(rowA).getByText("YES-MARKER")).toBeInTheDocument(); - expect(within(rowA).queryByText("yes")).not.toBeInTheDocument(); + expect(within(rowA).queryByText("Yes")).not.toBeInTheDocument(); }); }); }); diff --git a/web/src/routes/HelpPage.test.tsx b/web/src/routes/HelpPage.test.tsx index 9b785c1ae..e1bd8da3a 100644 --- a/web/src/routes/HelpPage.test.tsx +++ b/web/src/routes/HelpPage.test.tsx @@ -1098,7 +1098,7 @@ describe("HelpPage visual pass (#657)", () => { // The section's Open link: ROUTE_FOR["dashboard"] = "/" resolved against the Worker's own nav. const section = screen.getByRole("heading", { name: "Dashboard", level: 3 }).closest("section")!; expect(within(section).getByRole("link", { name: "Open Dashboard" })).toHaveAttribute("href", "/"); - expect(within(section).getByText("no entry", { selector: "strong" })).toBeInTheDocument(); + expect(within(section).getByText("No entry", { selector: "strong" })).toBeInTheDocument(); expect(within(section).getByText(/submitted days only/i)).toBeInTheDocument(); expect(within(section).getByText("stacked bar", { selector: "strong" })).toBeInTheDocument(); expect(screen.getByText("Capture status", { selector: "dt a" })).toBeInTheDocument(); From c0c93ec488a228a3ebf1976ea7bb221e21734770 Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 2 Sep 2026 14:10:55 -0700 Subject: [PATCH 9/9] docs(web): record that .toolbar is styled ahead of #653, not dead (#651) --- .../06-fix-increment-5.md | 75 +++++++++++++++++++ web/src/styles.css | 17 ++++- 2 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 docs/plans/651-652-spa-elevation-and-caps/06-fix-increment-5.md diff --git a/docs/plans/651-652-spa-elevation-and-caps/06-fix-increment-5.md b/docs/plans/651-652-spa-elevation-and-caps/06-fix-increment-5.md new file mode 100644 index 000000000..377069a14 --- /dev/null +++ b/docs/plans/651-652-spa-elevation-and-caps/06-fix-increment-5.md @@ -0,0 +1,75 @@ +# Fix increment 5 — say in the stylesheet that `.toolbar` is waiting for #653 + +Owner decision, 2026-09-02, after reviewing before/after screenshots: keep `.toolbar` and `--r-panel`, +and record why they exist so the next reader does not delete them as dead code. + +## The finding + +`.toolbar` has **zero consumers**. `grep -rn "toolbar" web/src --include='*.tsx' --include='*.ts'` returns +only this slice's own guard file; the built JS contains no such class name. `--r-panel` has exactly one +consumer, that same unrendered rule. + +This is the third selector in this slice whose issue text described a stylesheet that had moved on — +after `.stat`, which no longer exists, and `.eyebrow`, which was dead and got deleted. The difference is +that `.toolbar` is **not** dead by mistake: the delivery contract's ownership map records #653 ("table +layout — date-range filters into a bounded toolbar") as the slice that introduces its consumers. So it +is groundwork, and the owner chose to keep it. + +Nothing about that is inferable from the stylesheet, which is why it goes in the stylesheet. + +## Files + +- `web/src/styles.css` — a comment only. **No declaration changes.** The rule keeps every property it has. + +## Step 1 + +The comment block above `.toolbar` currently reads: + +```css +/* -------------------------------------------- Toolbar (inset filter bar) */ +/* --canvas and --surface-2 sit at 1.05-1.21:1 in every palette, so a + borderless fill would be invisible — the hairline border stays for that + reason, not because this is a floating card. */ +``` + +Replace those two comment blocks with this one. **PROTECTED — transcribe or STOP.** Do not touch the +`.toolbar { ... }` declarations beneath it. + +```css +/* -------------------------------------------- Toolbar (inset filter bar) */ +/* NOT DEAD CODE, and not rendered yet either: nothing in web/src applies + `.toolbar` today. It is styled here ahead of #653, which moves the + date-range filters into a bounded toolbar and is the slice that adds the + consumers. Deleting it would make #653 re-derive the two decisions below. + Check `grep -rn toolbar web/src --include='*.tsx'` before assuming it is + still unused — the day #653 lands, this note is stale. + --r-panel likewise has this rule as its only consumer today. + + Why it is inset rather than carded (#651): elevation encodes what floats, + and a filter bar does not. Why it keeps a hairline border despite being + inset: --canvas and --surface-2 sit between 1.05:1 and 1.21:1 in every + palette and both modes — measured with the repo's own contrast() helper — + so a borderless fill is invisible on every one of them, not just the light + ones. */ +``` + +## Step 2 — gates and commit + +**G1** then **G2**, both clean. A comment change moves nothing, so the counts stay +`Test Files 110 passed (110)` / `Tests 2411 passed (2411)`. + +Confirm with `git diff --stat` that exactly one file changed and that +`git diff web/src/styles.css | grep -c '^[+-][^+-]'` counts only comment lines — **if any declaration +line appears in the diff, STOP and report**, because this increment must not change rendering. + +```bash +git add web/src/styles.css docs/plans/651-652-spa-elevation-and-caps/06-fix-increment-5.md +git commit -m "docs(web): record that .toolbar is styled ahead of #653, not dead (#651)" +git push origin feat/651-652-spa-elevation-and-caps +``` + +Do NOT reply on GitHub, do NOT edit the PR description, do NOT merge. + +## Report back + +Commit SHA, `git status --short`, G1 and G2 output, and the output of the declaration-line check above. diff --git a/web/src/styles.css b/web/src/styles.css index cac5801d2..635fcfea7 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -1046,9 +1046,20 @@ input.cell { } /* -------------------------------------------- Toolbar (inset filter bar) */ -/* --canvas and --surface-2 sit at 1.05-1.21:1 in every palette, so a - borderless fill would be invisible — the hairline border stays for that - reason, not because this is a floating card. */ +/* NOT DEAD CODE, and not rendered yet either: nothing in web/src applies + `.toolbar` today. It is styled here ahead of #653, which moves the + date-range filters into a bounded toolbar and is the slice that adds the + consumers. Deleting it would make #653 re-derive the two decisions below. + Check `grep -rn toolbar web/src --include='*.tsx'` before assuming it is + still unused — the day #653 lands, this note is stale. + --r-panel likewise has this rule as its only consumer today. + + Why it is inset rather than carded (#651): elevation encodes what floats, + and a filter bar does not. Why it keeps a hairline border despite being + inset: --canvas and --surface-2 sit between 1.05:1 and 1.21:1 in every + palette and both modes — measured with the repo's own contrast() helper — + so a borderless fill is invisible on every one of them, not just the light + ones. */ .toolbar { display: flex; flex-wrap: wrap;