From 0198cefb6e9a32e8e7baca25637068b2293c2957 Mon Sep 17 00:00:00 2001
From: Mao Nakamoto <41178744+maonakamoto@users.noreply.github.com>
Date: Mon, 31 Aug 2026 09:37:07 +0200
Subject: [PATCH] fix(nav): the admin sidebar styled the current page without
announcing it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
All three of AdminSidebar's link sites passed an `active` flag into
`navLinkClass('sidebar', active)` and none set aria-current, so the highlight
existed only for people who can see it. Every other nav surface in this app —
NavItem, BottomNav, MobileMenuNav, Breadcrumbs, DashboardMobileNav — already
announced it correctly.
Fixed at the SSOT rather than at the three call sites. `navLinkClass` returns
a className and nothing else, which is what made "styled but silent" easy to
write: the styling decision and the announcement lived in different places, so
they could drift. `navLinkProps` returns both, and spreading it means they
cannot:
Deliberately NOT applied to every navLinkClass caller. The chip, tab and
segmented shapes are buttons and tab controls where `aria-selected` is the
correct attribute, not `aria-current` — a blanket sweep would have been wrong
in more places than it fixed. navLinkClass stays for those.
Five tests on the helper, including one asserting its className is byte-identical
to what navLinkClass returns for every shape and both states, so this cannot
quietly restyle anything.
Found by a fleet-wide navigation audit across 20 repos, which found the same
one-straggler shape in aoz-housing, fleetcrown and vitareba.
Co-Authored-By: Claude Opus 5 (1M context)
Claude-Session: https://claude.ai/code/session_01XeELB8b3N4JrT2asYL9WvE
---
src/app/admin/AdminSidebar.tsx | 8 ++--
.../design/__tests__/nav-link-props.test.ts | 46 +++++++++++++++++++
src/lib/design/nav.ts | 25 ++++++++++
3 files changed, 75 insertions(+), 4 deletions(-)
create mode 100644 src/lib/design/__tests__/nav-link-props.test.ts
diff --git a/src/app/admin/AdminSidebar.tsx b/src/app/admin/AdminSidebar.tsx
index ae452a13d..030b95b22 100644
--- a/src/app/admin/AdminSidebar.tsx
+++ b/src/app/admin/AdminSidebar.tsx
@@ -2,7 +2,7 @@
import { useState, useMemo } from 'react'
import { adminInteractive } from '@/lib/admin-ui'
-import { navLinkClass } from '@/lib/design/nav'
+import { navLinkProps } from '@/lib/design/nav'
import { useTranslations } from 'next-intl'
import Link from 'next/link'
import { Link as PublicLink } from '@/i18n/navigation'
@@ -213,7 +213,7 @@ export function AdminSidebar({
key={section.path}
href={section.path}
onClick={() => { setMobileMenuOpen(false); setFilterQuery('') }}
- className={navLinkClass('sidebar', isActive(section.path))}
+ {...navLinkProps('sidebar', isActive(section.path))}
>
@@ -247,7 +247,7 @@ export function AdminSidebar({
setMobileMenuOpen(false)}
- className={navLinkClass('sidebar', onlyActive, sidebarCollapsed ? 'justify-center' : '')}
+ {...navLinkProps('sidebar', onlyActive, sidebarCollapsed ? 'justify-center' : '')}
title={sidebarCollapsed ? sectionLabel(tSections, only) : undefined}
>
@@ -290,7 +290,7 @@ export function AdminSidebar({
key={section.path}
href={section.path}
onClick={() => setMobileMenuOpen(false)}
- className={navLinkClass('sidebar', active, sidebarCollapsed ? 'justify-center' : '')}
+ {...navLinkProps('sidebar', active, sidebarCollapsed ? 'justify-center' : '')}
title={sidebarCollapsed ? `${sectionLabel(tSections, section)}${sensitive ? ` (${t('sensitiveLabel')})` : ''}` : sensitivityReason}
>
{/* Larger icon when collapsed so it's easier to tap and recognise at a glance */}
diff --git a/src/lib/design/__tests__/nav-link-props.test.ts b/src/lib/design/__tests__/nav-link-props.test.ts
new file mode 100644
index 000000000..66813aa67
--- /dev/null
+++ b/src/lib/design/__tests__/nav-link-props.test.ts
@@ -0,0 +1,46 @@
+/**
+ * A nav link's styling and its announcement must come from the same call.
+ *
+ * `navLinkClass` returns only a className, so a caller could style the active
+ * item and forget to say which one it is — the highlight then exists only for
+ * people who can see it. AdminSidebar did exactly that on all three of its
+ * link sites while every other nav surface in this app set `aria-current`
+ * correctly. Four surfaces right and one wrong is what applying a rule by hand
+ * looks like at the margin, so `navLinkProps` returns both together and the two
+ * decisions cannot drift apart.
+ */
+import { navLinkClass, navLinkProps, NAV_STATE } from '@/lib/design/nav'
+
+describe('navLinkProps', () => {
+ it('announces the active link as the current page', () => {
+ expect(navLinkProps('sidebar', true)['aria-current']).toBe('page')
+ })
+
+ it('leaves aria-current off inactive links rather than setting it false', () => {
+ // `aria-current="false"` is a valid token meaning "not current", but it is
+ // noise in the accessibility tree — omit the attribute instead.
+ expect(navLinkProps('sidebar', false)['aria-current']).toBeUndefined()
+ })
+
+ it('returns exactly the className navLinkClass would have', () => {
+ for (const shape of Object.keys(NAV_STATE) as (keyof typeof NAV_STATE)[]) {
+ for (const active of [true, false]) {
+ expect(navLinkProps(shape, active).className).toBe(navLinkClass(shape, active))
+ }
+ }
+ })
+
+ it('passes an extra className through unchanged', () => {
+ expect(navLinkProps('sidebar', true, 'justify-center').className).toBe(
+ navLinkClass('sidebar', true, 'justify-center'),
+ )
+ })
+
+ it('marks the active state for every nav shape, not just the sidebar', () => {
+ // The bug was one surface forgetting. A helper that only worked for the
+ // shape that happened to be broken would not close the class.
+ for (const shape of Object.keys(NAV_STATE) as (keyof typeof NAV_STATE)[]) {
+ expect(navLinkProps(shape, true)['aria-current']).toBe('page')
+ }
+ })
+})
diff --git a/src/lib/design/nav.ts b/src/lib/design/nav.ts
index 8ef9bc90c..7d1a06d63 100644
--- a/src/lib/design/nav.ts
+++ b/src/lib/design/nav.ts
@@ -85,3 +85,28 @@ export function navLinkClass(shape: NavShape, active: boolean, className?: strin
const s = NAV_STATE[shape]
return cn(s.base, active ? s.active : s.inactive, className)
}
+
+/**
+ * The class AND the announcement, from one call.
+ *
+ * `navLinkClass` on its own lets a caller style the active item and forget to
+ * say which one it is — the highlight then exists only for people who can see
+ * it. AdminSidebar did exactly that on all three of its link sites while every
+ * other nav surface here set `aria-current` correctly, which is what applying
+ * a rule by hand looks like at the margin.
+ *
+ * Spread this instead of passing `className` by itself, and the two decisions
+ * cannot drift apart:
+ *
+ *
+ */
+export function navLinkProps(
+ shape: NavShape,
+ active: boolean,
+ className?: string,
+): { className: string; 'aria-current': 'page' | undefined } {
+ return {
+ className: navLinkClass(shape, active, className),
+ 'aria-current': active ? 'page' : undefined,
+ }
+}