fix(nav): the current-page highlight was wired end to end, then dropped - #236
Merged
Conversation
GlobalNavigation computes `currentView` from the path with mapPathToView() and passes it into TopNavigation as a prop — but TopNavigation declared the prop and never read it anywhere in its render. The "highlight the page you're on" feature existed on both ends and did nothing in between: neither a sighted user nor a screen reader could tell which top-level page was current. That is also why the nav-contract audit's grep flagged this file — the string `currentView` is present, computing an active state is the declared intent, and zero aria-current existed in the repo. Fixed with `usePathname()` directly in TopNavigation rather than finally consuming `currentView`: that mapping is coarser than the nav items it would need to match against (`ROUTES.forms` maps to the view key `'saved-forms'`, `Blog` isn't mapped at all and silently falls through to `'builder'`), so comparing against the live path is more correct, not just more available. Only the mobile menu's `navigation.map()` render was touched — it's the only place that array is rendered; the desktop nav renders its top links as separate inline JSX, a different item the fleet audit did not flag here. This does change what a user sees, not just what assistive tech hears: the active mobile nav item is now visibly highlighted, because a broken feature was actually restored rather than an attribute bolted onto nothing. The `currentView` prop is left unused where it already was — retiring the dead mapPathToView plumbing is a separate cleanup, not folded in here. Typecheck 0 errors (once contentlayer types are generated, per this repo's own CLAUDE.md — pre-existing prerequisite, unrelated to this change). Lint 0 errors, 2 pre-existing warnings (identical before/after). 46 tests pass. Found by the fleet nav-contract audit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeELB8b3N4JrT2asYL9WvE
catomean
force-pushed
the
fix/nav-contract
branch
from
August 31, 2026 12:35
0cd25c6 to
a827831
Compare
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
1 participant
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.
GlobalNavigationcomputescurrentViewfrom the path withmapPathToView()and passes it intoTopNavigationas a prop — butTopNavigationdeclared the prop and never read it anywhere in its render. The "highlight the page you're on" feature existed on both ends and did nothing in between: neither a sighted user nor a screen reader could tell which top-level page was current.That's also why the nav-contract audit flagged this file — the string
currentViewis present, computing an active state is the declared intent, and zeroaria-currentexisted in the repo.The fix
usePathname()directly inTopNavigation, rather than finally consumingcurrentView. That mapping is coarser than the nav items it would need to match against —ROUTES.formsmaps to the view key'saved-forms',Blogisn't mapped at all and silently falls through to'builder'— so comparing against the live path is more correct, not just more available.Only the mobile menu's
navigation.map()render was touched — it's the only place that array renders; the desktop nav renders its top links as separate inline JSX, a different (already-fixed-elsewhere) concern.Scope note, stated plainly
This changes what a user sees, not just what assistive tech hears — the active mobile nav item is now visibly highlighted, because a broken feature was actually restored rather than an attribute bolted onto nothing that computes anything.
currentViewis left unused where it already was; retiring the deadmapPathToViewplumbing is a separate cleanup, not folded in here.Verification
Typecheck: 0 errors (once contentlayer types are generated, per this repo's own
CLAUDE.md— pre-existing prerequisite, unrelated to this change). Lint: 0 errors, 2 pre-existing warnings, identical count before/after this diff. 46 tests pass.Found by the fleet nav-contract audit.
🤖 Generated with Claude Code
https://claude.ai/code/session_01XeELB8b3N4JrT2asYL9WvE