From c55129f349a055cff0789faee3798c9ba420ce97 Mon Sep 17 00:00:00 2001 From: Mao Nakamoto <41178744+maonakamoto@users.noreply.github.com> Date: Sat, 29 Aug 2026 12:36:54 +0200 Subject: [PATCH] fix(nav): the shared chrome shipped two defects no consumer could fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A shared renderer is the one place where a mistake is not one bug in one repo but the same bug on every site that installs it — and unfixable downstream, because a consumer cannot patch markup it does not own. Two shipped: 1. Nav links were `px-2 py-1` on `text-xs`, roughly a 28px target. That is a fiddly thing to hit on a phone and below the 44px floor the rest of the fleet holds (fleetcrown enforces it centrally; kivvi and wild-spirit meet it explicitly). Now `inline-flex min-h-11 items-center`. 2. The package defined no focus style at all. A consumer's CSS reset can remove the user-agent ring, and nothing here replaced it, so keyboard users could lose their visible position in the nav on every site that installs this. Both the nav links and the wordmark now carry `focus-visible:outline-2 outline-offset-2 outline-accent` — a semantic class, so it stays the consumer's colour, per this package's no-tokens rule. The masthead does NOT get taller. Its own comment warns that wrapping the nav "makes a sticky masthead eat a third of a phone screen", so the row pays for the bigger targets out of its own padding: py-4 becomes py-2, and 44px items land the header at the height it already had. Five rendered tests, asserting markup rather than source and importing by package name so a broken exports map fails here rather than in a consumer. Four fail against the previous renderers; the fifth covers `aria-current`, which already worked and is guarded so a refactor of these classes cannot quietly drop it. Both live consumers inherit this: substrata (v0.1.1) and camille-boulangerie (v0.2.0) are also version-drifted from each other, so both move to v0.3.0. Found by a fleet-wide navigation audit across 20 repos, where this was the only defect that multiplies. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XeELB8b3N4JrT2asYL9WvE --- README.md | 2 +- package.json | 2 +- src/react/SiteChrome.tsx | 10 ++++-- src/react/SiteNav.tsx | 11 ++++++- test/nav-a11y.test.js | 71 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 test/nav-a11y.test.js diff --git a/README.md b/README.md index 96daf22..491b5fc 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ every consumer gets it on the next version bump. ## Install ```bash -npm i github:bitbaum/sitekit#v0.2.0 +npm i github:bitbaum/sitekit#v0.3.0 ``` ESM-only. `react >= 18` is a peer dependency of `sitekit/react`; the root diff --git a/package.json b/package.json index e81b03e..0f50ca6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sitekit", - "version": "0.2.0", + "version": "0.3.0", "description": "A website as data: a closed union of section shapes validated at runtime, one set of React renderers, and per-field provenance so a generated site can prove it fabricated nothing.", "license": "MIT", "author": "Mao Nakamoto", diff --git a/src/react/SiteChrome.tsx b/src/react/SiteChrome.tsx index c26cc7e..ac53869 100644 --- a/src/react/SiteChrome.tsx +++ b/src/react/SiteChrome.tsx @@ -30,8 +30,14 @@ export function SiteMasthead({ chrome, navItems, currentPath, Link = DefaultLink {/* One row at every width. Wrapping the nav onto a second line makes a sticky masthead eat a third of a phone screen, so on narrow viewports the nav scrolls sideways instead. */} -
- + {/* py-2, not py-4: the nav's items are now 44px tall (the touch floor), + and the row pays for that out of its own padding so the masthead + keeps the same height it always had. */} +
+ {chrome.name} diff --git a/src/react/SiteNav.tsx b/src/react/SiteNav.tsx index 9636d9e..ba717de 100644 --- a/src/react/SiteNav.tsx +++ b/src/react/SiteNav.tsx @@ -38,7 +38,16 @@ export function SiteNav({ items, currentPath, Link = DefaultLink }: Props) { href={href(item.path)} aria-current={isCurrent ? 'page' : undefined} className={[ - 'shrink-0 rounded px-2 py-1 font-mono text-xs uppercase tracking-caps transition-colors', + // min-h-11 is the 44px touch floor. `px-2 py-1` on text-xs came out + // around 28px, which is a fiddly target on a phone and below the + // floor the rest of the fleet holds. The masthead row drops from + // py-4 to py-2 to pay for it, so the sticky header does NOT get + // taller — see the comment in SiteChrome. + 'inline-flex min-h-11 shrink-0 items-center rounded px-2 font-mono text-xs uppercase tracking-caps transition-colors', + // A consumer's CSS reset can remove the UA focus ring, and this + // package shipped nothing to replace it — so keyboard users had no + // visible position in the nav on every site that installs it. + 'focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-accent', isCurrent ? 'text-fg-primary underline decoration-accent decoration-2 underline-offset-8' : 'text-fg-tertiary hover:text-fg-primary', diff --git a/test/nav-a11y.test.js b/test/nav-a11y.test.js new file mode 100644 index 0000000..b769c1b --- /dev/null +++ b/test/nav-a11y.test.js @@ -0,0 +1,71 @@ +/** + * The chrome must not ship defects its consumers cannot fix. + * + * A shared renderer is the one place where a mistake is not one bug in one repo + * but the same bug on every site that installs it — and unfixable downstream, + * because a consumer cannot patch markup it does not own. Two such defects + * shipped: nav links were `px-2 py-1` on `text-xs` (roughly a 28px target, below + * the 44px floor the rest of the fleet holds), and nothing in the package + * defined a focus style, so a consumer's CSS reset could remove the UA ring and + * leave keyboard users with no visible position in the nav. + * + * These assert the rendered markup, not the source, and import by package name + * so a broken `exports`/`files` map fails here rather than in a consumer. + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { createElement as h } from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; + +import { SiteMasthead } from 'sitekit/react'; + +const CHROME = { name: 'Café Beispiel', tagline: 'Kaffee', footerNote: 'Run by its owners.' }; +const NAV = [ + { path: '', label: 'Home' }, + { path: 'menu', label: 'Menu' }, +]; + +const masthead = (currentPath = 'menu') => + renderToStaticMarkup(h(SiteMasthead, { chrome: CHROME, navItems: NAV, currentPath })); + +test('nav links meet the 44px touch floor', () => { + const html = masthead(); + assert.ok(html.includes('min-h-11'), 'nav links must carry the 44px minimum height'); + assert.ok( + !/class="[^"]*\bpx-2 py-1\b[^"]*font-mono text-xs uppercase/.test(html), + 'the old sub-floor px-2 py-1 target must be gone', + ); +}); + +test('nav links carry a visible focus style of their own', () => { + const html = masthead(); + assert.ok( + html.includes('focus-visible:outline-2'), + 'a consumer reset can remove the UA ring, so the package must ship one', + ); + assert.ok(html.includes('focus-visible:outline-offset-2')); +}); + +test('the wordmark is focusable with a visible ring and a real target', () => { + const html = masthead(); + // The home link sits before the nav; check the first anchor specifically. + const firstAnchor = html.slice(html.indexOf('')); + assert.ok(firstAnchor.includes('focus-visible:outline-2'), 'wordmark needs a focus ring'); + assert.ok(firstAnchor.includes('min-h-11'), 'wordmark needs a 44px target'); +}); + +test('the masthead does not grow to pay for the bigger targets', () => { + // The sticky masthead's own comment warns against eating a phone screen. The + // row gives up padding (py-4 -> py-2) so 44px items cost no extra height. + const html = masthead(); + assert.ok(html.includes('py-2'), 'row padding should be py-2'); + assert.ok( + !/justify-between gap-6 py-4/.test(html), + 'the row must not still be py-4, or the header got taller', + ); +}); + +test('the active item is still announced', () => { + // Guard against a refactor of these classes dropping what already worked. + assert.ok(masthead('menu').includes('aria-current="page"')); +});