diff --git a/SHARED.md b/SHARED.md index 30d7016..70a4dac 100644 --- a/SHARED.md +++ b/SHARED.md @@ -180,6 +180,55 @@ Stated explicitly, because "share everything" is its own failure: lists paid model ids (BYOK — the user's key, the user's choice) while the same id in kivvi's fallback was a bug. Centralize the **rule**; assert it **locally**, where the app knows which is which. +- **Navigation chrome.** Measured 2026-08-29: ~13,300 lines of nav across 20 + repos, and the experiment has already been run. `sitekit` is the one shared + renderer, it serves 2 of 20, and it shipped two defects — ~28px targets and + no focus style — to **both** consumers, unfixable downstream because a + consumer cannot patch markup it does not own. Its nav model (a flat + `{path, label}` list, no groups, no icons, no footer links, no mobile menu) + also cannot express what solon's mega-menu or reparaturbonus-zh's drawer + need, which is why every repo with real nav complexity reinvented its own + rather than adopt it. Centralizing the markup centralized the bug. + The navigation **contract** below is the shareable part. + +--- + +## The navigation contract + +Six rules, each one a defect found in the 2026-08-29 audit, each mechanically +checkable. This is the nav answer to "centralize the rule, assert it locally". + +1. The active link of every nav surface carries `aria-current`. +2. Every toggle controlling a panel carries `aria-expanded`. +3. Every interactive nav element is at least 44×44px. +4. The label lives **inside** the control, never beside it. +5. Persisted UI state distinguishes `null` from empty. +6. Every internal href comes from a routes constant. + +**Enforced in two places, because they cover disjoint surfaces.** +`scripts/ci/ui-defect-audit.mjs` checks 1, 3 and 4 by **rendering** each live +site — which is the only thing that spans Next apps, CSS modules, Tailwind and +wild-spirit's no-framework generator alike. But it renders **public entry pages +only**, so it structurally cannot see a sidebar behind a login. Authed surfaces +need a source-level check the repo runs in its own `verify`: orangecat's +`check:dead-labels` (rule 4) and fleetcrown's `check_paired` in +`check-design-system.sh` (rule 1) are the two working examples. + +**Deliberately NOT on the ratchet.** The ratchet counts concerns that should +converge on ONE implementation, and nav is the opposite: every repo is supposed +to have its own nav config, so counting those files would score the correct +outcome as duplication and the number would have no meaning. The contract is +enforced by the gates above instead. (This revises the audit's own first +recommendation — writing the "do not centralize" section above is what showed +the two could not both be right.) + +**Why a gate and not a convention.** The audit's sharpest finding was not that +teams do not know these rules — it is that aoz-housing, fleetcrown, vitareba +and evig each applied `aria-current` correctly on every nav surface **but one**. +Four teams, four stragglers. Hand-application always fails at the margin, and +the margin is invisible until someone renders it. Design tokens are the control +group: near-spotless fleet-wide, because they got a convention **plus a gate** +(`check:accent-ink`) rather than a shared component library. --- diff --git a/scripts/ci/test-ui-defect-audit.mjs b/scripts/ci/test-ui-defect-audit.mjs index 723fd64..e02c8a7 100644 --- a/scripts/ci/test-ui-defect-audit.mjs +++ b/scripts/ci/test-ui-defect-audit.mjs @@ -18,6 +18,71 @@ import { loadPlaywright, MEASURE } from "./ui-defect-audit.mjs"; const FIXTURES = { + // ── navigation fixtures ─────────────────────────────────────────────────── + + // The orangecat sidebar as it shipped: the section title is an

in a + //
, and the ONLY thing carrying the click is the chevron beside it. + // Visitors aimed at the word "Fund" and nothing happened. + navDeadLabel: ` +
+ +
`, + + // The same row done correctly: the label lives INSIDE the control, so the + // whole row is the target. This must stay silent. + navLabelInsideControl: ` +
+ +
`, + + // A nav containing a link to the page we are on, with nothing announcing it. + navUnmarkedCurrent: ` +
+ +
`, + + // The same nav, announcing correctly. + navMarkedCurrent: ` +
+ +
`, + + // A nav of outbound links only — it legitimately contains no link to the + // current page, so "no aria-current" is CORRECT here and must stay silent. + navNoSelfLink: ` +
+ +
`, + + // Targets below the fleet's 44px floor. + navSmallTargets: ` +
+ +
`, // The original fleetcrown fleet card: an icon INLINE at the head of two of // the four rows, shoving only those lines sideways by its own width, and a // wrapped hint whose second line falls back to the container edge. @@ -154,6 +219,26 @@ async function main() { return page.evaluate(MEASURE); }; + // The nav detectors compare each link's pathname against location.pathname, + // which setContent alone cannot exercise: an about:blank page has no path to + // match. Serve the fixture from a routed URL so "the link to the page you are + // on" is a real condition rather than one the test can never reach. + const measureAt = async (html, path) => { + const url = `https://fixture.test${path}`; + await page.route("https://fixture.test/**", (route) => + route.fulfill({ + status: 200, + contentType: "text/html", + body: `${html}`, + }), + ); + await page.goto(url, { waitUntil: "load" }); + await page.waitForTimeout(120); + const r = await page.evaluate(MEASURE); + await page.unroute("https://fixture.test/**"); + return r; + }; + let passed = 0; const check = async (label, fn) => { await fn(); @@ -250,6 +335,69 @@ async function main() { ); }); + + await check("catches a nav label that is not the control (the orangecat sidebar bug)", async () => { + const r = await measure(FIXTURES.navDeadLabel); + // innerText is the RENDERED text, so `text-transform: uppercase` reports + // "FUND" and not the "Fund" in the source. Compare case-insensitively — + // the detector is right and a case-sensitive assertion would be the bug. + assert( + r.navDeadLabels.length === 1 && + r.navDeadLabels[0].label.toLowerCase() === "fund", + `the stranded "Fund" heading must be reported, got ${JSON.stringify(r.navDeadLabels)}`, + ); + }); + + await check("stays silent when the label lives inside the control", async () => { + const r = await measure(FIXTURES.navLabelInsideControl); + assert( + r.navDeadLabels.length === 0, + `a full-row button is correct markup, got ${JSON.stringify(r.navDeadLabels)}`, + ); + }); + + await check("catches a current page that no link announces", async () => { + const r = await measureAt(FIXTURES.navUnmarkedCurrent, "/here"); + assert( + r.navMissingCurrent.length === 1 && r.navMissingCurrent[0].href === "/here", + `the unmarked self-link must be reported, got ${JSON.stringify(r.navMissingCurrent)}`, + ); + }); + + await check("stays silent when the current page IS announced", async () => { + const r = await measureAt(FIXTURES.navMarkedCurrent, "/here"); + assert( + r.navMissingCurrent.length === 0, + `aria-current is present, got ${JSON.stringify(r.navMissingCurrent)}`, + ); + }); + + await check("does NOT demand aria-current from a nav with no link to this page", async () => { + // A footer of outbound links has no current page to mark. Flagging it would + // make the detector fire on every site that has one. + const r = await measureAt(FIXTURES.navNoSelfLink, "/somewhere-else"); + assert( + r.navMissingCurrent.length === 0, + `no self-link means nothing to announce, got ${JSON.stringify(r.navMissingCurrent)}`, + ); + }); + + await check("catches nav targets below the 44px floor", async () => { + const r = await measureAt(FIXTURES.navSmallTargets, "/x"); + assert( + r.navSmallTargets.length === 2, + `both 32px targets must be reported, got ${JSON.stringify(r.navSmallTargets)}`, + ); + }); + + await check("stays silent on nav targets that meet the floor", async () => { + const r = await measureAt(FIXTURES.navMarkedCurrent, "/here"); + assert( + r.navSmallTargets.length === 0, + `44px targets are fine, got ${JSON.stringify(r.navSmallTargets)}`, + ); + }); + await browser.close(); console.log(`\n${passed}/${passed} ui-defect-audit self-tests passed`); } diff --git a/scripts/ci/ui-defect-audit.mjs b/scripts/ci/ui-defect-audit.mjs index 8d9befe..1d33290 100755 --- a/scripts/ci/ui-defect-audit.mjs +++ b/scripts/ci/ui-defect-audit.mjs @@ -332,7 +332,103 @@ export const MEASURE = String.raw`(() => { } } - return { contrast: contrast, ragged: ragged, wrapped: wrapped, stacksSeen: seen }; + // ── navigation ──────────────────────────────────────────────────────────── + // Three defects a fleet-wide nav audit found in 20 repos, all invisible to + // every check that only looks for forbidden strings, because each is + // something someone FORGOT rather than something someone wrote. + // + // Rendered rather than grepped on purpose: this fleet holds Next apps, CSS + // modules, Tailwind, and one hand-rolled static generator with no framework + // at all. No source-level lint spans that. The DOM does. + var navMissingCurrent = []; + var navSmallTargets = []; + var navDeadLabels = []; + + var navRoots = document.querySelectorAll('nav, [role="navigation"]'); + var here = location.pathname.replace(/\/+$/, "") || "/"; + + for (var n = 0; n < navRoots.length; n++) { + var root = navRoots[n]; + if (!root.getBoundingClientRect().width) continue; + + // 1. AN UNMARKED CURRENT PAGE. Only claimed when a link in this nav really + // does point at the page we are on — otherwise a nav that legitimately + // contains no self-link (a footer of outbound links) reads as a defect. + var links = root.querySelectorAll("a[href]"); + var selfLink = null, announced = false; + for (var l = 0; l < links.length; l++) { + var a = links[l]; + if (a.getAttribute("aria-current")) announced = true; + var path; + try { path = new URL(a.href, location.origin).pathname.replace(/\/+$/, "") || "/"; } + catch (e) { continue; } + if (path === here && a.getBoundingClientRect().width > 0) selfLink = a; + } + if (selfLink && !announced) { + navMissingCurrent.push({ + label: (selfLink.innerText || "").trim().slice(0, 40).replace(/\s+/g, " "), + href: selfLink.getAttribute("href") || "", + navLabel: root.getAttribute("aria-label") || root.className.slice(0, 40) || "nav" + }); + } + + // 2. TARGETS BELOW THE FLOOR. 44px is this fleet's own standard, not the + // WCAG 2.2 AA minimum of 24px — fleetcrown enforces it centrally and + // kivvi and wild-spirit state it explicitly, so a nav under it is out of + // step with the fleet rather than out of compliance. Say which. + var controls = root.querySelectorAll('a[href], button, [role="button"], [role="tab"]'); + for (var c = 0; c < controls.length; c++) { + var ctl = controls[c]; + var cr = ctl.getBoundingClientRect(); + if (cr.width === 0 || cr.height === 0) continue; + var ccs = getComputedStyle(ctl); + if (ccs.visibility === "hidden" || ccs.opacity === "0") continue; + // A control nested inside another is measured by its parent; skip it so + // one small icon inside a large row is not reported as the row. + if (ctl.parentElement && ctl.parentElement.closest('a[href], button')) continue; + if (cr.height < 44 || cr.width < 44) { + navSmallTargets.push({ + w: Math.round(cr.width), h: Math.round(cr.height), + tag: ctl.tagName.toLowerCase(), + text: (ctl.innerText || ctl.getAttribute("aria-label") || "").trim().slice(0, 30).replace(/\s+/g, " ") + }); + } + } + + // 3. A LABEL THAT LOOKS LIKE A CONTROL BUT IS NOT. The orangecat sidebar + // shipped this: an

section title beside a chevron-only