fix(ui): give nav links distinct focus identities - #24
Merged
Merged
Conversation
web's Playwright keyboard-nav gate went red on main: Error: /products: 2 links have no accessible name (only 1 may fall back to the shared href) The same destination is linked from three places — the desktop nav, the mobile nav (Header.tsx) and the footer link groups (Footer.tsx) — and none carried a data-testid or aria-label. The test identifies a link by data-testid ?? aria-label ?? href, so at desktop width the visible desktop nav link and the visible footer link both fall back to the bare href, collapse into one identity, and read as a focus trap. That is the same failure mode the test's own comment records from 2026-09-11. Adds a test-only disambiguator to all three renderings: nav-desktop-<label>, nav-mobile-<label>, footer-<group>-<label>. Deliberately NOT an aria-label: that would satisfy the gate but get announced to screen readers, making two identical links read differently for no user benefit. data-testid is invisible to assistive tech and is the identity the test prefers first. Nothing user-visible changes. tsup build clean, all three expressions present in dist/index.js.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The failure
nself-org/web'sAccessibility (WCAG AA)gate went red on main(run 34751962108):
Why it is this repo's bug, not web's
The commit that went red changed exactly one file — a docs page
(
licensing/offline.mdx) that links to nothing on/products. The two offendinglinks come from the shared header and footer in this package.
/productsis linked from three renderings, none of which carried adata-testidor
aria-label:Header.tsx(navItems.map,hidden lg:flex)Header.tsx(navItems.map, inside the collapsible panel)Footer.tsx(group.links.map)tests/keyboard/keyboard-nav.spec.tsidentifies a link bydata-testid ?? aria-label ?? hrefand skips anything withoffsetParent === null. At desktop width the mobile panel is hidden, but the desktopnav link and the footer link are both visible and both fall back to the bare href —
two "unnamed" links for one destination, which the test reports as a collapsed focus
identity. Its own comment records this exact failure mode from 2026-09-11, where three
links collapsed into one identity and tripped a false focus-trap.
That also explains why it looked intermittent: it is viewport- and
render-visibility-dependent, not random.
The fix
A test-only disambiguator on all three renderings:
nav-desktop-<label>,nav-mobile-<label>,footer-<group>-<label>.Deliberately not an
aria-label. That would satisfy the gate while making twootherwise-identical links announce differently to screen readers, for no user benefit
— worse accessibility in exchange for a green check.
data-testidis invisible toassistive tech and is the identity the test prefers first, so it fixes the real
collapsed-identity problem without touching what users hear.
Nothing user-visible changes.
Verification
pnpm buildin@nself-web/uiis clean, and all three expressions are present in thebuilt output:
The authoritative check is web's a11y gate against a build that consumes this — web
clones this repo fresh in CI, so it picks the change up once merged.