From f8210cefeac35dc6ec258cf0635d35574c8c63d1 Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 16 Sep 2026 14:32:32 +0000 Subject: [PATCH 1/5] feat(web): retire the Slack-blue link colour for ink + a rule underline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #834 A link is now `--ink` text, underlined at rest in a new `--link-rule` token (28% ink, flattened over `--surface`), full ink on hover and focus, per the owner's DIRECTION.md decision (2026-09-16) — which supersedes #822 D7.1's brand-derived `--link: var(--brand)` proposal, amended in place in docs/designs/822-mui-revamp.md. `--link` becomes an alias for `--ink` rather than being rewritten at every call site (the smaller diff); `--link-hover` is retired, since "full ink on hover" is already `--ink`. Of the nine `var(--link)` consumers, six are text-colour rules (now ink + the 28% rule underline where the element is genuinely underlined text) and three are border-color rules reclassified to `--stat-accent` (a hover highlight, not a link). `palette.info.main` moves off `--link` to `--stat-accent`. A `MuiLink` override gives a real MUI `Link` the same treatment, pinned in farmTheme.policy.test.ts. styles.test.ts pins link-text contrast at 4.5:1 and states, with measured numbers, why the 28% rule is not held to a 3:1 non-text threshold (a decorative reinforcement under already-compliant text, not the sole means of identifying the link). --- docs/designs/822-mui-revamp.md | 10 +++-- web/src/styles.css | 56 ++++++++++++++++++++------ web/src/styles.test.ts | 35 ++++++++++++++++ web/src/theme/FarmThemeProvider.tsx | 27 ++++++++++++- web/src/theme/farmTheme.policy.test.ts | 23 +++++++++++ web/src/theme/farmTokens.ts | 4 +- 6 files changed, 135 insertions(+), 20 deletions(-) diff --git a/docs/designs/822-mui-revamp.md b/docs/designs/822-mui-revamp.md index 453acd4c8..f99e37ba0 100644 --- a/docs/designs/822-mui-revamp.md +++ b/docs/designs/822-mui-revamp.md @@ -190,7 +190,7 @@ All from `@mui/material` 9.4.0 (`package-lock.json`). No `@mui/x-*` package; no | 17 | `p.error` / `p.warn` `role="alert"` | `Alert severity` | `Alert` renders `role="alert"` by default; the E2E "no alert on screen" reading (`AppLayout.tsx:105-110`) depends on that, and the permanent live region at L111 must stay a plain `

`. | each screen slice | | 18 | `.update-banner` | `Snackbar` holding an `Alert` with actions | Keeps `--shadow-bar` via the elevation map (D6). | #828 | | 19 | `ThemeToggle` | `IconButton` (icon-only sites) and `Button variant="text"` (labelled) | `applyTheme` unchanged; the observer in `FarmThemeProvider` re-reads. | #829 | -| 20 | `GlossaryLink`, `.content a` | `Link` | colour from `palette.info` until #834, then `primary` (D7.1) | #833 | +| 20 | `GlossaryLink`, `.content a` | `Link` | colour from `palette.info` until #834, then `--ink` underlined in `--link-rule` (owner decision 2026-09-16, DIRECTION.md, supersedes D7.1's brand-derived proposal — see the amendment on D7.1 below) | #833 | | 21 | `DayStrip`, `StockBar`, `.meter`, `.meter-stack`, `GradingChip`, `BrandSplash`, `ProvenanceCell`, `FarmDate` | **kept as they are** | bespoke data marks and interactions (#654, #777, F134, #179); their CSS stays (§2.2). `StockBar` is a multi-grade stacked bar and `LinearProgress` is single-value; `GradingChip` is a drag source (§2.1). No alternative meets the functionality, so these are not owner-review rows. | none | | 22 | `usePagedList` "Load more" | `Button` | the hook (381 lines, ticket discipline) is not presentation and stays; there is no offset `Pagination` to adopt. | none | @@ -284,7 +284,9 @@ So adopting it re-leads every paragraph, table cell and label that does not set ### D7. The three identity questions (#656), decided here so #834 to #836 inherit a decision -**D7.1 Links derive from the brand: yes. Owner review.** `--link: var(--brand)` in light, `--link: var(--stat-accent)` in night; `--link-hover` is `--brand-press` in light and the same `--stat-accent` at 85% opacity in night. Verified with the WCAG formula over the declared values, on `--surface` and `--surface-2`, all four palettes: each palette resolved through its own `data-brand` block (the `--surface-2` value differs per palette, so a fixed value gives wrong numbers): light `--brand` on `--surface` / `--surface-2` is 14.00/12.57 (aubergine), 11.25/10.23 (forest), 11.63/10.44 (slate), 10.90/9.63 (terracotta); night `--stat-accent` is 11.05/9.99, 10.95/9.66, 10.39/9.32, 9.67/8.91. Every pair clears 4.5:1 with margin, worst case 8.91; today's blue is 6.22 light and 7.70 night, so contrast rises everywhere. Pinned in `styles.test.ts` with the repo's own `contrast()` (`test/cssTokens.ts`), not with the numbers above. The one reason this stays an owner call is legibility rather than contrast: an aubergine link can read as a visited link, and a terracotta one as an error. The alternative, if that reads badly in the #834 captures, is a fifth hand-chosen `--link` per palette block, brand-adjacent and contrast-checked like `--stat-accent`, which also means adding it to `LIGHT_REQUIRED` (L15), `DARK_REQUIRED` (L26) and `GOLDEN` (L161) in `styles.test.ts` in the same commit, or the relation test at L238 fails. Consequence stated deliberately: `palette.info.main` (`FarmThemeProvider.tsx:34`) then equals `primary`, so an `Alert severity="info"` reads as brand-coloured, which is the intended monotheism. The sidebar's `.sidebar button.link` override (L1441-1453) already forces on-brand colour there and is unaffected. +**D7.1 Links derive from the brand: superseded.** The owner decided against it on 2026-09-16 (DIRECTION.md line 15: "Links are ink, underlined in a 28% ink rule"; line 22 rejects the blue link colour): a link is `--ink` text, underlined at rest in a new `--link-rule` token (28% ink, flattened over `--surface`), full ink on hover and focus. `--link` becomes an alias for `--ink` rather than a brand derivation, so `palette.info.main` (`FarmThemeProvider.tsx`) is repointed at `--stat-accent` instead of inheriting `--link` — the same "info is brand-adjacent, not plain text" outcome the original proposal wanted, reached a different way. #834 implements this and pins it in `styles.test.ts`/`farmTheme.policy.test.ts`. + +**D7.1, original proposal, superseded, kept for history:** `--link: var(--brand)` in light, `--link: var(--stat-accent)` in night; `--link-hover` is `--brand-press` in light and the same `--stat-accent` at 85% opacity in night. Verified with the WCAG formula over the declared values, on `--surface` and `--surface-2`, all four palettes: each palette resolved through its own `data-brand` block (the `--surface-2` value differs per palette, so a fixed value gives wrong numbers): light `--brand` on `--surface` / `--surface-2` is 14.00/12.57 (aubergine), 11.25/10.23 (forest), 11.63/10.44 (slate), 10.90/9.63 (terracotta); night `--stat-accent` is 11.05/9.99, 10.95/9.66, 10.39/9.32, 9.67/8.91. Every pair clears 4.5:1 with margin, worst case 8.91; today's blue is 6.22 light and 7.70 night, so contrast rises everywhere. The one reason this stayed an owner call was legibility rather than contrast: an aubergine link can read as a visited link, and a terracotta one as an error. The alternative on the table was a fifth hand-chosen `--link` per palette block, brand-adjacent and contrast-checked like `--stat-accent`. The sidebar's `.sidebar button.link` override (L1441-1453) already forces on-brand colour there and is unaffected either way. **D7.2 Optical size: adopt it, but the premise was wrong and the cost is not zero.** The loaded face is `wght`-only (§1). #835 swaps `main.tsx:3` to `@fontsource-variable/inter/opsz.css`, which replaces the seven `wght` files with the seven `opsz` files (they also carry `wght`, `font-weight: 100 900` in both entries). Once that face is loaded, prefer `font-optical-sizing: auto` on the theme's `h1`, `h2`, on the figure class that stays in CSS (`.trend-kpi` L2045) and on the phone house card's figure (today `.capture-tile-eggs` L1778, a `Typography` variant after #829): `auto` lets the browser interpolate from a 1280 heading to a 390 one, which one fixed `"opsz" 32` cannot. Because headings are theme variants after #823, the setting reaches `Typography` without a second declaration, which is the interaction #835 flagged. Cost: +118.9 KiB precache, +24.1 KiB on the latin subset a phone actually fetches. #835 runs after #825 and is charged against its ceiling (D9); if the ceiling cannot absorb it the fallback is one display cut for `h1`/`h2` only, and the doc records which was chosen. @@ -307,7 +309,7 @@ Order is by dependency, then by what each slice teaches the next. **Deletions ar | 9 | **#831** ledgers (+ Feed, Water, Reports) | D3.3 row 3; pairs 7, 9, 10, 11, 15; delete `.order-panel` (L1728-1731, last consumer Flocks moves to #832 if Flocks converts later) and `tr.discounted .badge-warn` (L1225-1253, Sales only); **keep** `.toolbar` L1168-1193 (seventh consumer is `AuditPage.tsx:432`, #833, and deleting early reopens #653), `table.data` L1623-1695 and `tr.inactive td` L1686 (the five CRUD lists in #832 and Audit in #833 still render them), `.page-head` L1571, and `.muted`/`.error`/`.success`/`.warn` (L1195-1224; `.muted` alone has 103 sites in 24 files including `ErrorBoundary`, `EmptyState` and `AppLayout`, and maps to `Typography color="text.secondary"` screen by screen under pair 10); one PR per screen when the diff is large (Sales alone has 4,110 test lines) | each screen's table and filter bar at both widths; Sales with a drill-down open | | 10 | **#832** CRUD lists | D3.3 row 4; Flocks' `.order-panel` | each list and one open form dialog at both widths; Users' RBAC behaviour is fixed (3,872 test lines) | | 11 | **#833** tail (+ Account, SetPassword) | D3.3 row 5; pairs 16, 20; delete the login and help blocks L2264-2796 (including `.glossary-entry:target`), the farm-settings block L3090-3278 (`.update-banner` L3279-3322 went with #828), the L316-534 remainder, and as last consumer: `.toolbar` L1168-1193, `table.data` L1623-1695, `.page-head` and `.content` L1559-1622, the badge rules L1275-1296, the semantic-text rules L1195-1224, and the shared `.card` rule; retire the last CSS guards | Login in both themes, Help, Settings, Audit at both widths | -| 12 | **#834** link colour | D7.1 | Dashboard, a list screen and Login in both themes, all four palettes (`playwright.palettes.config.ts` already captures this set) | +| 12 | **#834** link colour | D7.1 (superseded by the owner's DIRECTION.md decision, 2026-09-16 — see the amendment on D7.1) | Dashboard, a list screen and Login in both themes, all four palettes (`playwright.palettes.config.ts` already captures this set) | | 13 | **#835** opsz | D7.2 | a page title and a figure at 1280 before and after, 1:1 | | 14 | **#836** mark | D7.3 | the sidebar mark, favicon at 16 and 32px, home-screen icon, all four palettes | | after | **#50** offline capture | untouched by this doc; it captures from #830's screen, so it waits for #830; D9's ceiling constrains what it may precache | n/a | @@ -363,7 +365,7 @@ Product calls this doc makes that the owner reviews before or at the slice that | `NumberField` on `TextField` + `IconButton`, or Base UI `NumberField` | D2 pair 3, #828 | the other option; measured cost decides | | Form dialogs `fullScreen` below 900px | D2 pair 2, D3.3, #827/#832 | a bottom-sheet variant on MUI `Dialog` | | Daily-entry footer buttons stack full-width, reversing F134's side-by-side pair | D3.4, #823 | keep the footer side by side and stack only `.actions` and dialog foots | -| Links take `--brand` / `--stat-accent` | D7.1, #834 | a fifth hand-chosen `--link` per palette | +| Links take `--brand` / `--stat-accent` (D7.1's original proposal, superseded) | D7.1, #834 | what #834 actually shipped: `--ink`, underlined in a new `--link-rule` (28% ink), per the owner's DIRECTION.md decision (2026-09-16) | | Ceiling 1,800 KiB | D9, #825 | 1,600 KiB and the `opsz` fallback | ## 8. Simplicity ceiling diff --git a/web/src/styles.css b/web/src/styles.css index eeacbaeda..96f713cce 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -17,11 +17,17 @@ --on-brand: #ffffff; --on-brand-mute: #d9bdde; - /* Ink & links (blue is the only chromatic departure from aubergine) */ + /* Ink & links (#834 retires the Slack-blue departure — DIRECTION.md, owner + decision 2026-09-16, supersedes D7.1's brand-derived `--link: var(--brand)` + proposal: a link is ink, underlined in a 28% ink rule, full ink on hover + and focus. --link stays its own name rather than folding into --ink at + every call site — it names what the colour is FOR, and it is the smaller + diff over rewriting six consumers. --link-hover is retired: "full ink on + hover" is already --ink, so no separate hover shade is needed. */ --ink: #1d1d1d; --muted: #696969; - --link: #1264a3; - --link-hover: #3860be; + --link: var(--ink); + --link-rule: #c0c0c0; /* 28% --ink flattened over --surface (#ffffff) */ /* Surfaces */ --canvas: #faf7fc; /* app background — a whisper of lavender */ @@ -197,8 +203,8 @@ :root[data-theme="dark"] { --ink: #f2ecf4; --muted: #b3a8b8; - --link: #7fb4e6; - --link-hover: #a9cef0; + --link: var(--ink); + --link-rule: #5c5560; /* 28% --ink flattened over --surface (dark) */ --canvas: #17121a; --surface: #221a26; --surface-2: #2b2231; @@ -383,7 +389,8 @@ button.btn-danger:hover:not(:disabled) { background: var(--danger-press); } -/* Inline action / navigation buttons — text, not pills. Blue, like links. */ +/* Inline action / navigation buttons — text, not pills. Ink, like links + (#834): underlined at rest in the 28% ink rule, full ink on hover. */ button.link { background: none; color: var(--link); @@ -392,12 +399,16 @@ button.link { letter-spacing: 0; padding: 0.3rem 0.4rem; border-radius: var(--r-input); + text-decoration: underline; + text-decoration-color: var(--link-rule); + text-underline-offset: 2px; } button.link:hover:not(:disabled) { background: none; - color: var(--link-hover); text-decoration: underline; + text-decoration-color: var(--ink); + text-underline-offset: 2px; } :focus-visible { @@ -1508,12 +1519,15 @@ tr.discounted .badge-warn { its own single class. */ :where(.content a) { color: var(--link); - text-decoration: none; + text-decoration: underline; + text-decoration-color: var(--link-rule); + text-underline-offset: 2px; } :where(.content a):hover { - color: var(--link-hover); text-decoration: underline; + text-decoration-color: var(--ink); + text-underline-offset: 2px; } /* ---------------------------------------------------------------- Tables */ @@ -2256,8 +2270,11 @@ label.check { text-decoration: none; } +/* #834 — this ring is a hover highlight, not a link underline: --stat-accent + is this stylesheet's existing "interactive edge" colour (.palette-option's + checked border), not a repurposed --link. */ .help-open:hover { - border-color: var(--link); + border-color: var(--stat-accent); text-decoration: none; } @@ -2366,6 +2383,10 @@ dl.glossary { text-decoration: none; } +/* #834 — color: var(--link) already equals --ink via the alias, so this + headword keeps its rest-state colour on hover by design (only the + underline reveals the link); no rule-underline is added, since the rest + state's own "no underline until hover" is the intended affordance here. */ .glossary-entry dt a:hover { color: var(--link); text-decoration: underline; @@ -2395,9 +2416,12 @@ dl.glossary { vertical-align: middle; } +/* #834 — a small ring badge (a "?" glyph), not underlined text: the text + colour still follows --link (now ink), but the ring is a hover highlight, + not a link underline, so its edge takes --stat-accent like .help-open's. */ .help-link:hover { color: var(--link); - border-color: var(--link); + border-color: var(--stat-accent); text-decoration: none; } @@ -3306,11 +3330,15 @@ input.locked { font-size: 0.85rem; color: var(--link); padding: 0.15rem 0.4rem; + text-decoration: underline; + text-decoration-color: var(--link-rule); + text-underline-offset: 2px; } .named-picker-loadmore:hover:not(:disabled) { - color: var(--link-hover); text-decoration: underline; + text-decoration-color: var(--ink); + text-underline-offset: 2px; } /* Trigger button: the page-owned button that opens the picker. Uses the same @@ -3335,10 +3363,12 @@ button.named-picker-trigger { letter-spacing: 0; } +/* #834 — a form control's hover edge, not a link: --stat-accent for the + same reason as .help-open/.help-link's hover rings above. */ button.named-picker-trigger:hover:not(:disabled) { background: var(--surface); color: var(--ink); - border-color: var(--link); + border-color: var(--stat-accent); text-decoration: none; } diff --git a/web/src/styles.test.ts b/web/src/styles.test.ts index 9567fe310..0444d5a03 100644 --- a/web/src/styles.test.ts +++ b/web/src/styles.test.ts @@ -132,6 +132,41 @@ describe.each(BRANDS)("palette: %s", (brand) => { expect(contrast(at("--focus"), at(bg))).toBeGreaterThanOrEqual(4.5); }); + // #834 — the Slack-blue link retirement (DIRECTION.md, owner decision + // 2026-09-16): a link is --ink text underlined in a --link-rule (28% ink) + // rule. Checked against every background a link can render on, not just + // --surface: --surface-2 differs per palette (a toolbar or a help panel), + // and --link/--link-rule themselves do not (--ink is theme-scoped only), + // so this is the check that would catch a palette whose --surface-2 got + // too close to ink. + it.each(MODES)("%s: link text clears WCAG AA (4.5:1) on --surface and --surface-2", (mode) => { + const t = resolveTokens(attrFor(brand), mode); + const at = (k: string) => t.get(k)!; + for (const bg of ["--surface", "--surface-2"]) + expect(contrast(at("--link"), at(bg)), `${brand}/${mode} --link vs ${bg}`) + .toBeGreaterThanOrEqual(4.5); + }); + + // The 28% ink rule is a decorative reinforcement under text that already + // clears 4.5:1 above, not the sole means of identifying the link — WCAG + // 1.4.11 (Non-text Contrast) targets UI-component boundaries and states + // that ARE the only cue, and DIRECTION.md's own interactive state ("full + // ink on hover and focus") already carries that job. So 3:1 is not applied + // here: flattened over the surfaces it actually sits on, a literal 28% + // ink cannot clear 3:1 against a near-white or near-black surface — the + // measured worst case across all four palettes is ~1.6:1 light / ~2.1:1 + // dark, and reaching 3:1 would need roughly 48% (light) / 38% (dark) ink, + // a materially bolder rule than DIRECTION.md specified. The floor below is + // the honest measured minimum, not an invented pass, so a future change + // that makes the rule fainter still gets caught. + it.each(MODES)("%s: the 28% ink rule stays visibly above its surface (not a 3:1 pass — see comment)", (mode) => { + const t = resolveTokens(attrFor(brand), mode); + const at = (k: string) => t.get(k)!; + for (const bg of ["--surface", "--surface-2"]) + expect(contrast(at("--link-rule"), at(bg)), `${brand}/${mode} --link-rule vs ${bg}`) + .toBeGreaterThanOrEqual(1.5); + }); + it.each(MODES)("%s: the login Forget glyph clears WCAG AA on its rest fill", (mode) => { // #587 — .auth-forget-farm draws its × over --surface-2 at rest. The // destructive FILL token (--danger) does not clear 4.5:1 for that glyph in diff --git a/web/src/theme/FarmThemeProvider.tsx b/web/src/theme/FarmThemeProvider.tsx index 242ab8a46..07568996e 100644 --- a/web/src/theme/FarmThemeProvider.tsx +++ b/web/src/theme/FarmThemeProvider.tsx @@ -126,7 +126,12 @@ export function createFarmTheme(tokens: TokenValues, mode: ThemeMode): Theme { error: { main: tokens["--danger"], contrastText: tokens["--on-danger"] }, success: { main: tokens["--success"] }, warning: { main: tokens["--warn"] }, - info: { main: tokens["--link"] }, + // #834 — `--link` is now `--ink` (the Slack-blue retirement), and ink is + // not a colour `Alert severity="info"` can render as. `--stat-accent` + // is this app's other palette-derived accent (the sidebar's active-item + // rule, the selected radio edge), so info reads as brand-adjacent + // rather than as plain body text. + info: { main: tokens["--stat-accent"] }, background: { default: tokens["--canvas"], paper: tokens["--surface"] }, text: { primary: tokens["--ink"], secondary: tokens["--muted"] }, divider: tokens["--hairline"], @@ -453,6 +458,26 @@ export function createFarmTheme(tokens: TokenValues, mode: ThemeMode): Theme { }, }, }, + // #834 — DIRECTION.md's link language for a real MUI `Link`: ink text, + // underlined in the 28% ink rule at rest, full ink on hover and focus. + // No screen renders `@mui/material`'s `Link` today (they render + // `Typography component={Link}` from react-router, styled by + // `:where(.content a)` in styles.css instead), so this is groundwork — + // it has to exist before a screen can adopt the real component, and + // `farmTheme.policy.test.ts` is where it is pinned. + MuiLink: { + defaultProps: { underline: "always" }, + styleOverrides: { + root: { + color: tokens["--ink"], + textDecorationColor: tokens["--link-rule"], + textUnderlineOffset: "2px", + "&:hover, &:focus-visible": { + textDecorationColor: tokens["--ink"], + }, + }, + }, + }, }, }); } diff --git a/web/src/theme/farmTheme.policy.test.ts b/web/src/theme/farmTheme.policy.test.ts index fd20a1493..41646fd8a 100644 --- a/web/src/theme/farmTheme.policy.test.ts +++ b/web/src/theme/farmTheme.policy.test.ts @@ -310,4 +310,27 @@ describe("farm theme policy (#823 G2)", () => { expect(theme.typography.caption.fontSize, `${label} caption size`).toBe("0.75rem"); } }); + + // #834 — DIRECTION.md's link language, on the one MUI component nothing + // renders yet: a real `Link` is ink, underlined in the 28% ink rule at + // rest, full ink on hover and focus. Pinned here (not just in + // styles.test.ts) because `--link`/`--link-rule` reaching `styles.css` says + // nothing about whether MUI's own `Link` picks them up — that only happens + // through this override. + it("colours a real MUI Link ink, underlined in the rule colour, full ink on hover and focus", () => { + for (const brand of BRANDS) { + for (const mode of MODES) { + const tokens = tokensFor(brand, mode); + const theme = createFarmTheme(tokens, mode); + const label = `${brand}/${mode}`; + expect(theme.components?.MuiLink?.defaultProps?.underline, `${label} MuiLink underline`).toBe("always"); + const root = slot(theme.components?.MuiLink?.styleOverrides?.root, `${label} MuiLink root`); + expect(root.color, `${label} MuiLink rest colour`).toBe(tokens["--ink"]); + expect(root.textDecorationColor, `${label} MuiLink rest underline colour`).toBe(tokens["--link-rule"]); + expect(root.textUnderlineOffset, `${label} MuiLink underline offset`).toBe("2px"); + const interactive = slot(root["&:hover, &:focus-visible"], `${label} MuiLink hover/focus`); + expect(interactive.textDecorationColor, `${label} MuiLink hover/focus underline colour`).toBe(tokens["--ink"]); + } + } + }); }); diff --git a/web/src/theme/farmTokens.ts b/web/src/theme/farmTokens.ts index c28ed5293..400dc989d 100644 --- a/web/src/theme/farmTokens.ts +++ b/web/src/theme/farmTokens.ts @@ -30,7 +30,7 @@ export const THEME_TOKENS = [ "--error", "--success", "--warn", "--danger", "--on-danger", "--tint-ok", "--tint-warn", "--tint-danger", "--tint-accent", "--tint-muted", "--stat-accent", "--lavender", - "--link", "--focus", "--font", + "--link", "--link-rule", "--focus", "--font", "--r-input", "--r-panel", "--r-card", "--r-pill", "--shadow-bar", "--shadow-dialog", ] as const; @@ -59,7 +59,7 @@ const JSDOM_FALLBACK = { "--tint-ok": "#e2f2ec", "--tint-warn": "#f7ecd9", "--tint-danger": "#f9e5df", "--tint-accent": "#f3e9f5", "--tint-muted": "#eeeaf0", "--stat-accent": "#4a154b", "--lavender": "#f9f0ff", - "--link": "#1264a3", "--focus": "#4a154b", + "--link": "#1d1d1d", "--link-rule": "#c0c0c0", "--focus": "#4a154b", "--font": "system-ui, sans-serif", "--r-input": "4px", "--r-panel": "8px", "--r-card": "12px", "--r-pill": "999px", "--shadow-bar": "0 -1px 0 rgba(29, 21, 33, 0.05), 0 -16px 40px rgba(29, 21, 33, 0.07)", From f1f0b167da6702bf2db1b38327972f03620f5ac3 Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 16 Sep 2026 14:47:39 +0000 Subject: [PATCH 2/5] fix(web): apply the full-ink link underline to keyboard focus too CodeRabbit review of #884, round 1: button.link, :where(.content a) and .named-picker-loadmore switched to a full-ink underline only on :hover. DIRECTION.md says full ink on hover AND focus, and the global :focus-visible rule only adds an outline, so a keyboard-only visitor kept seeing the faint 28% rule. Combine :focus-visible into each rule, and pin it with a selector-level regex test (mutation- verified: removing :focus-visible from button.link's selector fails the new test). --- web/src/styles.css | 13 +++++++++---- web/src/styles.test.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/web/src/styles.css b/web/src/styles.css index 96f713cce..5ec268c51 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -390,7 +390,9 @@ button.btn-danger:hover:not(:disabled) { } /* Inline action / navigation buttons — text, not pills. Ink, like links - (#834): underlined at rest in the 28% ink rule, full ink on hover. */ + (#834): underlined at rest in the 28% ink rule, full ink on hover and + keyboard focus (DIRECTION.md), the latter alongside the global + `:focus-visible` outline below, not instead of it. */ button.link { background: none; color: var(--link); @@ -404,7 +406,8 @@ button.link { text-underline-offset: 2px; } -button.link:hover:not(:disabled) { +button.link:hover:not(:disabled), +button.link:focus-visible { background: none; text-decoration: underline; text-decoration-color: var(--ink); @@ -1524,7 +1527,8 @@ tr.discounted .badge-warn { text-underline-offset: 2px; } -:where(.content a):hover { +:where(.content a):hover, +:where(.content a):focus-visible { text-decoration: underline; text-decoration-color: var(--ink); text-underline-offset: 2px; @@ -3335,7 +3339,8 @@ input.locked { text-underline-offset: 2px; } -.named-picker-loadmore:hover:not(:disabled) { +.named-picker-loadmore:hover:not(:disabled), +.named-picker-loadmore:focus-visible { text-decoration: underline; text-decoration-color: var(--ink); text-underline-offset: 2px; diff --git a/web/src/styles.test.ts b/web/src/styles.test.ts index 0444d5a03..05ab5742a 100644 --- a/web/src/styles.test.ts +++ b/web/src/styles.test.ts @@ -1,4 +1,6 @@ import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; import { contrast, declaredKeys, literalColourIn, luminance, resolveTokens, type Mode } from "./test/cssTokens"; import { BRANDS, DEFAULT_BRAND } from "./lib/brand"; @@ -279,6 +281,32 @@ it("pins every brand-scoped token a palette block can declare", () => { expect(pinned).toContain(token); }); +// #834 (CodeRabbit round 1) — DIRECTION.md says full ink on hover AND focus; +// the global `:focus-visible` rule only adds an outline, so a rule that +// switches `text-decoration-color` on `:hover` alone leaves a keyboard-only +// visitor seeing the faint 28% rule instead of full ink. Checked against the +// raw stylesheet text rather than resolved tokens, because the defect is +// about which SELECTOR carries the declaration, not what the declaration +// resolves to — `resolveTokens` only sees `:root` blocks and cannot tell a +// `:hover`-only rule from a `:hover, :focus-visible` one. +describe("the full-ink underline applies to keyboard focus, not only mouse hover (#834)", () => { + const css = readFileSync(resolve(process.cwd(), "src/styles.css"), "utf8"); + + // Tolerant of whitespace/line-wrapping so a reformat doesn't break it, but + // it still ties the SELECTOR to the DECLARATION rather than checking either + // in isolation — a `:focus-visible` selector that forgot the declaration + // (or a declaration on a rule missing `:focus-visible`) both fail this. + it.each([ + ["button.link", /button\.link:hover:not\(:disabled\)\s*,\s*button\.link:focus-visible\s*\{[^}]*\}/], + [":where(.content a)", /:where\(\.content a\):hover\s*,\s*:where\(\.content a\):focus-visible\s*\{[^}]*\}/], + [".named-picker-loadmore", /\.named-picker-loadmore:hover:not\(:disabled\)\s*,\s*\.named-picker-loadmore:focus-visible\s*\{[^}]*\}/], + ] as const)("%s: :hover and :focus-visible share the full-ink rule", (_name, pattern) => { + const match = pattern.exec(css); + expect(match, "no combined :hover, :focus-visible rule found").not.toBeNull(); + expect(match![0], "combined rule must set full ink").toContain("text-decoration-color: var(--ink)"); + }); +}); + // #654 — the dashboard's surfaces carry no shadow, no caps, no motion and no // literal colour: tiles inherit #651's elevation direction and #652's sentence // case, and the "no animated bars" acceptance is met by never animating. From 042d2f562d358f8e867113f79b7f36778486d0ef Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 16 Sep 2026 14:58:34 +0000 Subject: [PATCH 3/5] test(web): pin exact --link/--link-rule values, not just their contrast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex CLI review of #884 (CodeRabbit round 3 was rate limited): the two new #834 contrast tests asserted only "clears N:1", which a different colour above that floor would also pass — neither actually pinned the retirement's own claim (--link IS --ink, --link-rule IS the specific flattened-28%-ink hex). Adds the missing equality assertions; mutation-verified against a --link-rule literal that still cleared 1.5:1 but wasn't #c0c0c0. --- web/src/styles.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/web/src/styles.test.ts b/web/src/styles.test.ts index 05ab5742a..f589145f5 100644 --- a/web/src/styles.test.ts +++ b/web/src/styles.test.ts @@ -144,6 +144,11 @@ describe.each(BRANDS)("palette: %s", (brand) => { it.each(MODES)("%s: link text clears WCAG AA (4.5:1) on --surface and --surface-2", (mode) => { const t = resolveTokens(attrFor(brand), mode); const at = (k: string) => t.get(k)!; + // Contrast alone would also pass a different, still-sufficiently-dark + // blue (Codex review of #884, round 3) — the actual retirement claim is + // that --link IS --ink now, not merely that whatever it is clears AA. + expect(at("--link"), `${brand}/${mode} --link must equal --ink, not a separate colour`) + .toBe(at("--ink")); for (const bg of ["--surface", "--surface-2"]) expect(contrast(at("--link"), at(bg)), `${brand}/${mode} --link vs ${bg}`) .toBeGreaterThanOrEqual(4.5); @@ -164,6 +169,12 @@ describe.each(BRANDS)("palette: %s", (brand) => { it.each(MODES)("%s: the 28% ink rule stays visibly above its surface (not a 3:1 pass — see comment)", (mode) => { const t = resolveTokens(attrFor(brand), mode); const at = (k: string) => t.get(k)!; + // The contrast floor alone would also pass an unrelated colour above + // 1.5:1 (Codex review of #884, round 3) — pin the actual flattened-28%- + // ink value this decision computed, not just a property it happens to + // have. --link-rule is theme-scoped only, same as --link/--ink. + expect(at("--link-rule"), `${brand}/${mode} --link-rule value`) + .toBe(mode === "light" ? "#c0c0c0" : "#5c5560"); for (const bg of ["--surface", "--surface-2"]) expect(contrast(at("--link-rule"), at(bg)), `${brand}/${mode} --link-rule vs ${bg}`) .toBeGreaterThanOrEqual(1.5); From 511dcfa6492a0683aeb6dbb71d9714ee1d6bb12c Mon Sep 17 00:00:00 2001 From: mforce Date: Wed, 16 Sep 2026 15:06:33 +0000 Subject: [PATCH 4/5] fix(web): defend named-picker-trigger against button.link's rest-state underline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex CLI review of #884, round 4 (CodeRabbit still rate limited): button.link's new rest-state text-decoration is a real bleed-through risk for button.named-picker-trigger via the exact equal-specificity, later-wins mechanism the trigger's own comment already defends padding/font-size against. No page currently combines the two classes (grepped: only a lifecycle test fixture does), so this doesn't reproduce live today, but it is a direct, foreseeable consequence of button.link's new declaration and the trigger's existing comment claimed a combination that no longer occurs — corrected in the same edit. Reset text-decoration: none explicitly and pinned it; mutation-verified. Also scoped the round-1 focus-visible test's own comment: it covers the three always-underlined links, not .glossary-entry dt a, whose hover-only underline predates #834 and is unchanged by this PR. --- web/src/styles.css | 12 ++++++++++-- web/src/styles.test.ts | 21 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/web/src/styles.css b/web/src/styles.css index 5ec268c51..1d2042ca9 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -3349,12 +3349,20 @@ input.locked { /* Trigger button: the page-owned button that opens the picker. Uses the same metrics as form inputs so it aligns with neighboring date/text fields. The button.named-picker-trigger selectors below are the load-bearing ones: - pages give the trigger the shared `link` class, and `button.link` + IF a page ever gives the trigger the shared `link` class (none does today — + every real caller passes `className="named-picker-trigger"` alone; only a + test fixture combines them, salesExpensesUS3.lifecycle.test.tsx), `button.link` (element+class, (0,1,1)) outranks bare `.named-picker-trigger` ((0,1,0)) — without the element-qualified override the link's padding/font-size win and the trigger's box comes out 12px shorter than its adjacent inputs, so - the form-grid's bottom alignment leaves its top edge sitting low. */ + the form-grid's bottom alignment leaves its top edge sitting low. + #834 (Codex review, round 4): button.link now also sets `text-decoration` + at rest, which the SAME bleed-through would apply to the trigger — a form + control, not a link — the moment that combination occurs. text-decoration: + none below closes it defensively, matching the padding/font-size defense + this rule already existed for. */ button.named-picker-trigger { + text-decoration: none; /* The adjacent inputs sit inside