From 4f69d395be217fe90761f0f7938ed0ee72ac05ca Mon Sep 17 00:00:00 2001 From: cesnimda Date: Mon, 20 Jul 2026 02:29:50 +0200 Subject: [PATCH] fix(nav): only the most-specific sidebar item is active MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/career-workspace-ux-refactor.md | 111 ++++++++++++++++++ job-tracker-ui/src/App.tsx | 2 + job-tracker-ui/src/layout/AppShell.tsx | 22 +++- job-tracker-ui/src/sidebar-active-nav.test.ts | 43 +++++++ .../src/views/CareerWorkspacePage.tsx | 6 +- 5 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 docs/career-workspace-ux-refactor.md create mode 100644 job-tracker-ui/src/sidebar-active-nav.test.ts diff --git a/docs/career-workspace-ux-refactor.md b/docs/career-workspace-ux-refactor.md new file mode 100644 index 0000000..b636ff3 --- /dev/null +++ b/docs/career-workspace-ux-refactor.md @@ -0,0 +1,111 @@ +# Career Workspace + CV Builder UX refactor + +> 2026-07-20. UX/product restructuring against v1.0.0. **Frontend-only** — no change to +> CareerProfiles, CvVariants, the CV generation pipeline, extraction APIs, AI services, permissions, +> or tenant isolation. +> +> This document is the plan. It is delivered **staged**: Phase 0 (the sidebar bug and the framing +> copy) is implemented and tested now; Phases 1–4 are scoped for incremental delivery because they +> require surgery on the 1376-line `CareerProfilePage` and touch the production CV/extraction flow, +> where a single large rewrite would risk the "do not break" list. Each later phase is independently +> shippable and verifiable. + +## Current problems + +1. **Sidebar double-highlight.** On `/career/builder/{id}`, both "Career Workspace" and "CV Builder" + light up. Root cause: `AppShell` used `pathname === to || pathname.startsWith(to + "/")` per item, + so `/career` matched every `/career/...` child. No "most specific wins" rule. *(Fixed — Phase 0.)* +2. **Too many competing concepts on one page.** `CareerWorkspacePage` is a thin shell around + `CareerProfilePage` (1376 lines), which bundles: a structured-CV editor, extraction-run history, a + "CV structure overview", rewrite **templates** with a PDF carousel (a second, template-driven CV + builder), and field-level review metadata. A user cannot tell which artifact is "their CV". +3. **Internal vocabulary leaks to users** — "structured CV", "JSON structure", "extraction schema", + "structure overview". These are implementation concepts. +4. **Extraction is opaque.** Upload runs, the profile changes, but the user never sees *what* changed + and cannot approve it. History is shown as a primary section instead of the *result* of an import. +5. **Two CV builders.** The template-driven rewrite/PDF flow inside the profile page overlaps the real + CV Builder (`/career/builder`), which already owns templates, layout, styling, variants and PDF. + +## New information architecture + +**Product rule:** *Career Profile* holds your information; *CV Builder* creates documents from it. + +| Surface | Owns | Does NOT own | +|---|---|---| +| **Career Profile** (`/career`) | Personal info, professional summary, work experience, education, skills, projects, certifications, languages. The facts. | Templates, layout, styling, PDF, variants | +| **CV Builder** (`/career/builder`, `/career/builder/{id}`) | Templates, layout, styling, section order/visibility, variants, PDF generation | Career facts (it *reads* the profile) | + +Career Profile page structure (target): + +- **Header** — "Career Profile", subtitle *"This information powers your CVs, applications, cover + letters and AI assistance."*, profile completeness %, last updated, quick action → CV Builder. +- **Sections** (user-facing labels only): Personal information · Professional summary · Work + experience · Education · Skills · Projects · Certifications · Languages. +- **Import CV** — current source (filename, date, status) + `[Upload new CV]`; after extraction a + review screen (below). History moves to Settings → Advanced → Import history. + +## Removed / relocated concepts + +| Concept | Disposition | +|---|---| +| "Structured CV Editor" | Renamed and reframed to **Career Profile editor** (same fields, user vocabulary) | +| "CV Structure Overview" | Removed from the user surface; if needed for debugging, move under admin/developer tools | +| "Template-driven CV Builder" (rewrite templates + PDF carousel inside the profile page) | **Removed** — the CV Builder already provides templates, layouts, styling, sections, customization and PDF. One CV Builder only | +| Extraction run history as a primary section | **Relocated** to Settings → Advanced → Import history | +| Internal terms ("structured CV", "JSON", "schema") in labels/help text | Replaced with plain language | + +## Route ownership (authoritative) + +``` +/career → Career Profile (Career Workspace nav item) +/career/builder → CV Builder (CV Builder nav item) +/career/builder/{id} → CV Builder (child of CV Builder, NOT Career Profile) +``` + +Rule: the sidebar item whose `to` is the **longest prefix** the current path is at or under wins; +all others are inactive. A child route never activates a parent nav item. + +## Implementation plan + +### Phase 0 — Sidebar bug + framing (DONE, this change) +- `AppShell.activeNavTo(pathname, tos)` — exported pure function; longest-owning `to` wins. `selected` + now compares against the single computed `activeTo` across both nav lists. +- Breadcrumb/title in `App.tsx`: explicit `/career/builder` → "CV Builder" ownership before the + `/career` fallback (previously `/career/builder` showed "Career Workspace"). +- Career Workspace header reframed to the "Career Profile" product framing. +- Tests: `sidebar-active-nav.test.ts` — asserts exactly one active item for `/career`, + `/career/builder`, `/career/builder/{id}`, and that no item double-highlights. + +### Phase 1 — Career Profile editor sections (frontend) +Reorganise `CareerProfilePage` presentation into the eight named sections with user-facing labels; +strip internal vocabulary from headings/help. No data-model or API change — the same +`StructuredCvProfile` shape is read and saved. Ship behind the existing page; verify save/load of each +section against the existing profile API. + +### Phase 2 — Import CV review screen +Add a post-upload "New information found" review (Experience / Skills / Languages / Education) with +`[Accept all] · [Review individually] · [Discard]`, diffing the extracted profile against the current +one **client-side** (no backend change — the extraction API already returns the structured result and +field metadata). Nothing is written until the user accepts. Relocate run history to Settings → +Advanced → Import history. + +### Phase 3 — Remove the second CV builder +Delete the rewrite-template + PDF-carousel flow from `CareerProfilePage`. Confirm the CV Builder +(`/career/builder`) covers templates/layout/styling/variants/PDF first (it does). Verify public CV and +PDF generation still work end to end. + +### Phase 4 — WYSIWYG for long-form fields +Rich editor (bold, lists, links, undo/redo) for professional summary, work descriptions, achievements, +projects, cover letters. **Store clean HTML or Markdown only** — no editor-specific state — and the +renderer consumes the same format. Choose a small dependency already compatible with the stack, or a +minimal contentEditable wrapper; decide at Phase 4 to avoid a premature dependency. + +## Preserved (verified not touched) +CareerProfiles, CvVariants, CV generation, public CV pages, extraction APIs, AI services, permissions, +tenant isolation. Phases 1–4 are frontend-only; any that appears to need a backend change is a signal +to re-scope, not to change the model. + +## Why staged +The removals and the review/WYSIWYG surfaces all require editing the 1376-line `CareerProfilePage` and +the live extraction/generation path. Delivering them as one change would put the v1.0.0 CV pipeline at +risk with no incremental verification. Each phase above is small enough to ship and verify on its own. diff --git a/job-tracker-ui/src/App.tsx b/job-tracker-ui/src/App.tsx index 8940f19..60836b3 100644 --- a/job-tracker-ui/src/App.tsx +++ b/job-tracker-ui/src/App.tsx @@ -85,6 +85,7 @@ function breadcrumbsFor(path: string, t: (k: any) => string): string[] { if (path.startsWith("/trash")) return [t("home"), t("trash")]; if (path.startsWith("/settings")) return [t("home"), t("settings")]; if (path.startsWith("/profile")) return [t("home"), t("account"), t("profile")]; + if (path.startsWith("/career/builder")) return [t("home"), "Career Workspace", "CV Builder"]; if (path.startsWith("/career")) return [t("home"), "Career Workspace"]; if (path.startsWith("/settings/connected-accounts")) return [t("home"), t("settings"), "Connected accounts"]; if (path.startsWith("/admin/audit")) return [t("home"), t("admin"), t("auditLog")]; @@ -104,6 +105,7 @@ function titleFor(path: string, t: (k: any) => string): string { if (path.startsWith("/trash")) return t("trash"); if (path.startsWith("/settings")) return t("settings"); if (path.startsWith("/profile")) return t("profile"); + if (path.startsWith("/career/builder")) return "CV Builder"; if (path.startsWith("/career")) return "Career Workspace"; if (path.startsWith("/settings/connected-accounts")) return "Connected accounts"; if (path.startsWith("/admin/audit")) return t("auditLog"); diff --git a/job-tracker-ui/src/layout/AppShell.tsx b/job-tracker-ui/src/layout/AppShell.tsx index 24ed3c4..fd076a5 100644 --- a/job-tracker-ui/src/layout/AppShell.tsx +++ b/job-tracker-ui/src/layout/AppShell.tsx @@ -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[]]>) => ( {groups.map(([section, rows]) => ( @@ -140,7 +160,7 @@ export default function AppShell({ ) : null} {rows.map((item) => { - const selected = pathname === item.to || pathname.startsWith(item.to + "/"); + const selected = item.to === activeTo; return ( Career Workspace +// /career/builder -> CV Builder +// A child route must activate exactly one nav item — the most specific owner. +const TOS = ["/dashboard", "/jobs", "/career", "/career/builder", "/settings"]; + +describe("sidebar active nav ownership", () => { + test("/career activates Career Workspace only", () => { + expect(activeNavTo("/career", TOS)).toBe("/career"); + }); + + test("/career/builder activates CV Builder only, not Career Workspace", () => { + expect(activeNavTo("/career/builder", TOS)).toBe("/career/builder"); + }); + + test("/career/builder/{id} activates CV Builder only (the reported bug)", () => { + // Previously /career matched via startsWith('/career/') AND /career/builder matched — both lit up. + expect(activeNavTo("/career/builder/42", TOS)).toBe("/career/builder"); + }); + + test("a plain child of /career (not /builder) still belongs to Career Workspace", () => { + expect(activeNavTo("/career/anything-else", TOS)).toBe("/career"); + }); + + test("unrelated routes are unaffected and exact matches win", () => { + expect(activeNavTo("/jobs", TOS)).toBe("/jobs"); + expect(activeNavTo("/settings/connected-accounts", TOS)).toBe("/settings"); + }); + + test("a path owned by no nav item activates nothing", () => { + expect(activeNavTo("/admin/system", TOS)).toBeNull(); + }); + + test("exactly one item is ever active (no double highlight)", () => { + for (const path of ["/career", "/career/builder", "/career/builder/7", "/jobs"]) { + const active = activeNavTo(path, TOS); + const matches = TOS.filter((t) => t === active); + expect(matches).toHaveLength(1); + } + }); +}); diff --git a/job-tracker-ui/src/views/CareerWorkspacePage.tsx b/job-tracker-ui/src/views/CareerWorkspacePage.tsx index de69577..b3ebfeb 100644 --- a/job-tracker-ui/src/views/CareerWorkspacePage.tsx +++ b/job-tracker-ui/src/views/CareerWorkspacePage.tsx @@ -15,15 +15,15 @@ export default function CareerWorkspacePage() { - Career Workspace + Career Profile - Maintain the master career profile that powers your CVs, tailored application material, and future portfolio outputs. + This information powers your CVs, applications, cover letters and AI assistance. - Your master profile is the source of truth. Job-specific CV drafts remain separate and never overwrite it. + Your career profile holds your information. The CV Builder creates documents from it — job-specific CVs stay separate and never overwrite your profile.