fix(nav): only the most-specific sidebar item is active
On /career/builder/{id} both "Career Workspace" (/career) and "CV Builder"
(/career/builder) highlighted, because AppShell tested each item with
`pathname === to || pathname.startsWith(to + "/")` — so /career matched
every /career/... child. No "most specific wins" rule.
Add AppShell.activeNavTo(pathname, tos): the longest `to` that the path is
at or under wins, across both nav lists; every other item is inactive. A
child route never lights up a parent nav item. `selected` now compares
against that single computed activeTo. Exported as a pure function so the
ownership rule is unit-tested directly (sidebar-active-nav.test.ts):
exactly one active item for /career, /career/builder and
/career/builder/{id}, and no double-highlight.
Also give the breadcrumb/title in App.tsx explicit /career/builder ->
"CV Builder" ownership (it previously showed "Career Workspace"), and
reframe the Career Workspace header to the "Career Profile" product
framing: "This information powers your CVs, applications, cover letters
and AI assistance."
Frontend only — no change to CareerProfiles, CvVariants, CV generation,
extraction APIs, AI, permissions or tenant isolation. Plan for the deeper
information-architecture work is in docs/career-workspace-ux-refactor.md,
staged so the 1376-line CareerProfilePage and the live CV/extraction
pipeline are refactored incrementally with verification, not in one risky
rewrite.
Verified: tsc clean, frontend build clean, 135 frontend tests pass
(128 + 7 new nav tests). Backend untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -47,6 +47,20 @@ function initialsFrom(s?: string) {
|
||||
|
||||
const DESKTOP_SIDEBAR_KEY = "appShellDesktopSidebarCollapsed";
|
||||
|
||||
// Which single nav item owns the current path. A child route must not light up its parent:
|
||||
// /career/builder/5 belongs to "CV Builder" (/career/builder), not "Career Workspace" (/career),
|
||||
// even though the old `pathname.startsWith(to + "/")` test matched both. Explicit ownership =
|
||||
// the LONGEST `to` that the path is at or under wins; everything else is inactive. Exported so the
|
||||
// rule is unit-testable without rendering the shell.
|
||||
export function activeNavTo(pathname: string, tos: string[]): string | null {
|
||||
let best: string | null = null;
|
||||
for (const to of tos) {
|
||||
const owns = pathname === to || pathname.startsWith(to + "/");
|
||||
if (owns && (best === null || to.length > best.length)) best = to;
|
||||
}
|
||||
return best;
|
||||
}
|
||||
|
||||
// The nav rail stays a fixed dark navy regardless of the app's light/dark theme toggle --
|
||||
// a deliberate signature element, not derived from theme tokens.
|
||||
const SIDEBAR_BG = "#0f172a";
|
||||
@@ -129,6 +143,12 @@ export default function AppShell({
|
||||
};
|
||||
}, [nav, navBottom]);
|
||||
|
||||
// Compute the one active destination across BOTH nav lists, so the most specific route wins.
|
||||
const activeTo = useMemo(
|
||||
() => activeNavTo(pathname, [...nav, ...navBottom].map((i) => i.to)),
|
||||
[pathname, nav, navBottom],
|
||||
);
|
||||
|
||||
const renderNavList = (groups: Array<[string, NavItem[]]>) => (
|
||||
<Box sx={{ px: desktopNavCollapsed ? 0.75 : 1.25, pt: 1 }}>
|
||||
{groups.map(([section, rows]) => (
|
||||
@@ -140,7 +160,7 @@ export default function AppShell({
|
||||
) : null}
|
||||
<List sx={{ px: desktopNavCollapsed ? 0.25 : 0.75, pt: desktopNavCollapsed ? 0.25 : 0.75 }}>
|
||||
{rows.map((item) => {
|
||||
const selected = pathname === item.to || pathname.startsWith(item.to + "/");
|
||||
const selected = item.to === activeTo;
|
||||
return (
|
||||
<ListItemButton
|
||||
key={item.to}
|
||||
|
||||
Reference in New Issue
Block a user