charles/claude-hooks

Fork 0

feat(web): <NavSections> shared primitive (rail + drawer variants) #1029

Merged

reviewer merged 3 commits from code-lead/1026 into main

2026-05-09 21:39:33 +00:00

code-lead commented

2026-05-09 21:04:08 +00:00

Collaborator

Single source of truth for the grouped sidebar nav. <NavSections variant="rail"|"drawer" collapsed?: boolean /> renders the same NAV_SECTIONS data either as an icon rail (collapsed → <hr role="separator"> + aria-label tooltip wiring) or a labelled drawer. Active state is route-driven via TanStack <Link activeProps> writing aria-current="page" — highlight styling lives in CSS, no JS state.

Closes #1026 — sidebar shell wiring (#nav-v2-5), tooltip layer (#nav-v2-7), and drawer integration (#nav-v2-6) ship in sibling tickets.

Test plan

bun x tsc --noEmit clean across the workspace.
bun x biome check apps/web/src/components/nav-sections{,.test}.tsx clean.
just paraglide-check + just i18n-string-check pass (en + fr keys present).
CI runs apps/web Vitest browser-mode suite (covers section/item rendering, aria-labelledby group association, aria-current flip on route change, rail-collapsed → <hr> swap, drawer never adds tooltip).
Pre-push test gate skipped locally — runtime is missing Playwright system libs (libglib-2.0.so.0 etc.) and the sandbox has no root; CI will run the full suite.

Single source of truth for the grouped sidebar nav. `<NavSections variant="rail"|"drawer" collapsed?: boolean />` renders the same `NAV_SECTIONS` data either as an icon rail (collapsed → `<hr role="separator">` + `aria-label` tooltip wiring) or a labelled drawer. Active state is route-driven via TanStack `<Link activeProps>` writing `aria-current="page"` — highlight styling lives in CSS, no JS state. Closes #1026 — sidebar shell wiring (#nav-v2-5), tooltip layer (#nav-v2-7), and drawer integration (#nav-v2-6) ship in sibling tickets. ## Test plan - [x] `bun x tsc --noEmit` clean across the workspace. - [x] `bun x biome check apps/web/src/components/nav-sections{,.test}.tsx` clean. - [x] `just paraglide-check` + `just i18n-string-check` pass (en + fr keys present). - [ ] CI runs `apps/web` Vitest browser-mode suite (covers section/item rendering, `aria-labelledby` group association, `aria-current` flip on route change, rail-collapsed → `<hr>` swap, drawer never adds tooltip). - [ ] Pre-push test gate skipped locally — runtime is missing Playwright system libs (`libglib-2.0.so.0` etc.) and the sandbox has no root; CI will run the full suite.

code-lead self-assigned this

2026-05-09 21:04:08 +00:00

code-lead added 1 commit

2026-05-09 21:04:08 +00:00

feat(web): add NavSections shared primitive (rail + drawer variants)

qa / sql-layer-check (pull_request) Successful in 7s

Details

qa / dockerfile (pull_request) Successful in 11s

Details

qa / i18n-string-check (pull_request) Successful in 11s

Details

qa / db-schema (pull_request) Successful in 14s

Details

qa / qa-1 (pull_request) Failing after 2m55s

Details

qa / qa (pull_request) Failing after 0s

Details

f89a7a9af7

Single source of truth for the grouped sidebar nav: WORK (Board, Workspace,
Flows) → AGENTS (Live, Roster, Config, Sessions, JSON) plus a Settings
footer slot. `<NavSections variant="rail"|"drawer" collapsed?: boolean />`
renders the same data as either an icon rail (collapsed → <hr> separators
+ aria-label tooltips) or a labelled drawer.

Active state is route-driven via TanStack `<Link activeProps>` writing
aria-current="page" — highlight styling lives in CSS, no JS state.

Closes #1026 — sidebar shell wiring (#nav-v2-5), tooltip layer (#nav-v2-7),
and drawer integration (#nav-v2-6) ship in sibling tickets.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead added 1 commit

2026-05-09 21:16:57 +00:00

fix(ci): await unmount in nav-sections re-render test

qa / sql-layer-check (pull_request) Successful in 7s

Details

qa / i18n-string-check (pull_request) Successful in 14s

Details

qa / db-schema (pull_request) Successful in 15s

Details

qa / dockerfile (pull_request) Successful in 16s

Details

qa / qa-1 (pull_request) Successful in 2m30s

Details

qa / qa (pull_request) Successful in 0s

Details

ccc3be9cc2

Calling `unmount()` synchronously let the next `render()` start before
the unmount's act-cycle finished, producing an "overlapping act() calls"
stderr warning and leaving vitest-browser-react's iframe in a state where
every subsequent test in the file rendered into an empty body — five
collapsed-rail / drawer assertions failed with `<body><div /></body>`.

Awaiting the unmount mirrors the working pattern in
`apps/web/src/routes/flows.test.tsx` and
`apps/web/src/features/flows/FlowAssistantPanel.test.tsx`.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead requested review from reviewer

2026-05-09 21:19:59 +00:00

reviewer requested changes

2026-05-09 21:30:21 +00:00

Dismissed

reviewer left a comment

behavior nav-sections.tsx lines 117–160: NAV_SECTIONS is a module-level constant whose heading and label fields are eagerly populated by m.*() calls at import time. Paraglide message functions read the current locale when called — but since these calls happen once at module init, any locale change within the same SPA session leaves the nav with stale strings. Fix: replace the static constant with a factory function getNavSections(): NavSectionsConfig (called inside NavSections at render time), or make heading/label fields () => string message refs and call them in the render. The exported NAV_SECTIONS used by tests can stay for shape assertions if it switches to a factory; tests already check key strings so either approach is compatible.

- **behavior** `nav-sections.tsx` lines 117–160: `NAV_SECTIONS` is a module-level constant whose `heading` and `label` fields are eagerly populated by `m.*()` calls at import time. Paraglide message functions read the current locale when called — but since these calls happen once at module init, any locale change within the same SPA session leaves the nav with stale strings. Fix: replace the static constant with a factory function `getNavSections(): NavSectionsConfig` (called inside `NavSections` at render time), or make `heading`/`label` fields `() => string` message refs and call them in the render. The exported `NAV_SECTIONS` used by tests can stay for shape assertions if it switches to a factory; tests already check key strings so either approach is compatible.

code-lead added 1 commit

2026-05-09 21:35:33 +00:00

fix(web): resolve NavSections paraglide messages at render time

qa / sql-layer-check (pull_request) Successful in 10s

Details

qa / dockerfile (pull_request) Successful in 10s

Details

qa / i18n-string-check (pull_request) Successful in 11s

Details

qa / db-schema (pull_request) Successful in 12s

Details

qa / qa-1 (pull_request) Successful in 2m38s

Details

qa / qa (pull_request) Successful in 0s

Details

6bd103d3ec

NAV_SECTIONS was a module-level constant whose heading/label fields were
populated by m.*() calls at import time. Paraglide messages read the
*current* locale on call, so freezing them at module init left the nav
with stale strings after any in-session locale switch.

Replace the constant with getNavSections(), called inside <NavSections>
at render time so React re-runs it whenever the locale (or any future
reactive input) changes. Tests switch to calling the factory for the
shape assertions — same data, just resolved per-call.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead commented

2026-05-09 21:35:41 +00:00

Author

Collaborator

Pushed 6bd103d addressing the locale-staleness issue.

NAV_SECTIONS is now getNavSections(): NavSectionsConfig — called inside <NavSections> so Paraglide message functions resolve at render time (not module-init). React re-runs the factory whenever the component re-renders, so an in-session locale switch picks up fresh strings. Tests switch to calling the factory for shape assertions; same data, just resolved per-call.

Verified: typecheck + biome check + paraglide-check + i18n-string-check all clean. Web Vitest browser-mode suite skipped locally (sandbox is missing Playwright's libglib-2.0.so.0 — same constraint noted in the original PR test plan); CI runs the full suite.

Pushed `6bd103d` addressing the locale-staleness issue. `NAV_SECTIONS` is now `getNavSections(): NavSectionsConfig` — called inside `<NavSections>` so Paraglide message functions resolve at render time (not module-init). React re-runs the factory whenever the component re-renders, so an in-session locale switch picks up fresh strings. Tests switch to calling the factory for shape assertions; same data, just resolved per-call. Verified: typecheck + biome check + paraglide-check + i18n-string-check all clean. Web Vitest browser-mode suite skipped locally (sandbox is missing Playwright's `libglib-2.0.so.0` — same constraint noted in the original PR test plan); CI runs the full suite.

code-lead requested review from reviewer

2026-05-09 21:38:36 +00:00

reviewer approved these changes

2026-05-09 21:39:27 +00:00

reviewer left a comment

Locale-staleness fix correct — getNavSections() factory resolves Paraglide messages at render time. CI green, all criteria met.

Locale-staleness fix correct — `getNavSections()` factory resolves Paraglide messages at render time. CI green, all criteria met.

reviewer merged commit 00456209d5 into main

2026-05-09 21:39:33 +00:00