From c90459a7bd3c48876afe9279c7eb3de4d985ba0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 15 Jun 2026 00:04:13 +0200 Subject: [PATCH] refactor(nav): drive desktop and mobile from one destination model The desktop sidebar and the mobile bottom-nav each hand-curated their own destination list, and the two had drifted: the sidebar listed Measurements and Mood inline but hid Workouts; the bottom bar buried Measurements and Mood in More but promoted Workouts. A user who learned the product on one platform re-learned it on the other. Extract one ordered destination model both bars consume. The sidebar renders every entry in order; the bottom bar keeps its 5-slot shape and derives its More hub from the same list, so the two surfaces tell one story. Workouts is now first-class on both. A shared active-resolver prefers the most-specific sibling so a sub-route no longer lights up its parent. --- .../layout/__tests__/nav-model.test.ts | 75 ++++++++++ .../layout/__tests__/sidebar-nav.test.tsx | 22 +++ src/components/layout/bottom-nav.tsx | 98 ++++++------- src/components/layout/nav-model.ts | 133 ++++++++++++++++++ src/components/layout/sidebar-nav.tsx | 88 +++--------- 5 files changed, 290 insertions(+), 126 deletions(-) create mode 100644 src/components/layout/__tests__/nav-model.test.ts create mode 100644 src/components/layout/nav-model.ts diff --git a/src/components/layout/__tests__/nav-model.test.ts b/src/components/layout/__tests__/nav-model.test.ts new file mode 100644 index 000000000..b522a43b8 --- /dev/null +++ b/src/components/layout/__tests__/nav-model.test.ts @@ -0,0 +1,75 @@ +/** + * v1.17.1 (F-1 / F-3) — the one navigation information-model. + * + * Pins the coherence contract the desktop sidebar and the mobile + * bottom-nav both consume: one ordered destination list (so the two bars + * tell one story), Workouts AND Coach as first-class destinations on both + * surfaces, cycle gated by the account flag, and an active-resolver that + * prefers the most-specific sibling under `/insights`. + */ +import { describe, expect, it } from "vitest"; + +import { + NAV_DESTINATIONS, + isNavDestinationActive, + visibleNavDestinations, +} from "../nav-model"; + +describe("nav-model destination list", () => { + it("lists Workouts as a first-class destination on both surfaces", () => { + const hrefs = NAV_DESTINATIONS.map((d) => d.href); + expect(hrefs).toContain("/insights/workouts"); + }); + + it("orders the core spine: dashboard → measurements → mood → medications", () => { + const hrefs = NAV_DESTINATIONS.map((d) => d.href); + expect(hrefs.indexOf("/")).toBeLessThan(hrefs.indexOf("/measurements")); + expect(hrefs.indexOf("/measurements")).toBeLessThan( + hrefs.indexOf("/mood"), + ); + expect(hrefs.indexOf("/mood")).toBeLessThan( + hrefs.indexOf("/medications"), + ); + }); + + it("every destination carries an i18n key under the nav namespace", () => { + for (const d of NAV_DESTINATIONS) { + expect(d.tKey.startsWith("nav.")).toBe(true); + } + }); + +}); + +describe("visibleNavDestinations cycle gate", () => { + it("includes Cycle (between Medications and Insights) only when enabled", () => { + const off = visibleNavDestinations(false).map((d) => d.href); + const on = visibleNavDestinations(true).map((d) => d.href); + expect(off).not.toContain("/cycle"); + expect(on).toContain("/cycle"); + expect(on.indexOf("/medications")).toBeLessThan(on.indexOf("/cycle")); + expect(on.indexOf("/cycle")).toBeLessThan(on.indexOf("/insights")); + }); +}); + +describe("isNavDestinationActive most-specific resolution", () => { + it("matches the dashboard only on an exact path", () => { + expect(isNavDestinationActive("/", "/")).toBe(true); + expect(isNavDestinationActive("/", "/measurements")).toBe(false); + }); + + it("does not light up Insights when on its Workouts sibling", () => { + expect( + isNavDestinationActive("/insights/workouts", "/insights/workouts"), + ).toBe(true); + expect(isNavDestinationActive("/insights", "/insights/workouts")).toBe( + false, + ); + }); + + it("still lights up Insights on its own sub-routes", () => { + expect(isNavDestinationActive("/insights", "/insights")).toBe(true); + expect(isNavDestinationActive("/insights", "/insights/values/WEIGHT")).toBe( + true, + ); + }); +}); diff --git a/src/components/layout/__tests__/sidebar-nav.test.tsx b/src/components/layout/__tests__/sidebar-nav.test.tsx index 5d070b66a..a0db7aa88 100644 --- a/src/components/layout/__tests__/sidebar-nav.test.tsx +++ b/src/components/layout/__tests__/sidebar-nav.test.tsx @@ -131,6 +131,28 @@ describe(" targets deprecation (v1.8.6)", () => { }); }); +describe(" unified destination model (v1.17.1 F-1)", () => { + it("surfaces Workouts as a first-class sidebar destination", () => { + // Pre-unify the sidebar hid Workouts entirely while the mobile bar + // promoted it. Both now render the one shared model, so both carry it. + const html = render(); + expect(html).toContain('href="/insights/workouts"'); + expect(html).toContain("Workouts"); + }); + + it("marks Workouts active without also marking Insights active", () => { + const html = render({ pathname: "/insights/workouts" }); + // The Workouts link carries aria-current="page"; the Insights link, + // its less-specific sibling, must not (most-specific resolution). + const workouts = html.match( + /]*href="\/insights\/workouts"[^>]*>/, + ); + const insights = html.match(/]*href="\/insights"[^>]*>/); + expect(workouts?.[0]).toMatch(/aria-current="page"/); + expect(insights?.[0]).not.toMatch(/aria-current="page"/); + }); +}); + describe(" admin entry mirrors Settings (no sub-item expansion)", () => { // v1.4.16 A1: the maintainer reported the global sidebar expanding admin // sub-items on `/admin/*` was unwanted UX — the in-shell `` diff --git a/src/components/layout/bottom-nav.tsx b/src/components/layout/bottom-nav.tsx index 3dd18a6e1..cb0e510a3 100644 --- a/src/components/layout/bottom-nav.tsx +++ b/src/components/layout/bottom-nav.tsx @@ -1,26 +1,25 @@ "use client"; import { - Activity, Bell, Bug, - Droplets, - Dumbbell, - FlaskConical, Home, Lightbulb, MoreHorizontal, Pill, Plus, Settings, - Trophy, - Waves, + type LucideIcon, } from "lucide-react"; import Link from "next/link"; import { usePathname } from "next/navigation"; import { useQueryClient } from "@tanstack/react-query"; import { medicationsPrefetchIntentProps } from "@/lib/queries/prefetch-medications"; -import { useState } from "react"; +import { + isNavDestinationActive, + visibleNavDestinations, +} from "@/components/layout/nav-model"; +import { useMemo, useState } from "react"; import { useAppSettings } from "@/components/app-settings-provider"; import { useAuth } from "@/hooks/use-auth"; import { @@ -37,7 +36,7 @@ import { useTranslations } from "@/lib/i18n/context"; interface NavLink { href: string; tKey: string; - icon: typeof Home; + icon: LucideIcon; } // v1.12.x — iOS-parity bottom bar (additive middle-path). @@ -48,12 +47,14 @@ interface NavLink { // the existing measurement / medication / mood quick-entry surfaces // (see ``). // -// PRIMARY_LEFT / PRIMARY_RIGHT are the two anchor pairs that flank the -// center action. The previous strip (Home · Measurements · Mood · -// Medications · Insights) carried Measurements and Mood inline; those -// stay fully reachable — through the center capture picker AND the More -// hub below — so nothing is orphaned. On desktop the sidebar still -// lists every destination. +// v1.17.1 (F-1) — the two always-visible flanking anchors and the "More" +// hub are now BOTH derived from the one shared destination model +// (`nav-model.ts`), the same ordered list the desktop sidebar renders. +// The bar keeps its ergonomic 5-slot shape, but every destination that +// isn't a primary slot falls into the hub in the model's order — so the +// two surfaces tell one story instead of two hand-curated ones that drift. +const PRIMARY_SLOT_HREFS = ["/", "/medications", "/insights"] as const; + const PRIMARY_LEFT: ReadonlyArray = [ { href: "/", tKey: "nav.dashboard", icon: Home }, { href: "/medications", tKey: "nav.medications", icon: Pill }, @@ -63,33 +64,23 @@ const PRIMARY_RIGHT: ReadonlyArray = [ { href: "/insights", tKey: "nav.insights", icon: Lightbulb }, ]; -// The "More" hub — a real hub of the remaining top-level destinations, -// not just an overflow bucket. Measurements and Mood live here (they -// left the always-visible strip when the center capture action took the -// middle slot), alongside Workouts, Achievements, Notifications and -// Settings. Every entry is an existing top-level route. -const MORE_HUB: ReadonlyArray = [ - { href: "/measurements", tKey: "nav.measurements", icon: Activity }, - { href: "/mood", tKey: "nav.mood", icon: Waves }, - { href: "/labs", tKey: "nav.labs", icon: FlaskConical }, - { href: "/insights/workouts", tKey: "nav.workouts", icon: Dumbbell }, - { href: "/achievements", tKey: "nav.achievements", icon: Trophy }, - { href: "/notifications", tKey: "nav.notifications", icon: Bell }, - { href: "/settings/account", tKey: "nav.settings", icon: Settings }, -]; - -// v1.15.0 — the cycle entry, spliced into the More hub only when the -// account's `cycleTrackingEnabled` gate is true. Hidden for everyone else. -const CYCLE_HUB_ITEM: NavLink = { - href: "/cycle", - tKey: "nav.cycle", - icon: Droplets, +// Mobile-only hub conveniences that have no main-list home on desktop +// (the sidebar reaches them through its footer + avatar menu). They sit +// at the tail of the hub, after every shared destination. +const NOTIFICATIONS_HUB_ITEM: NavLink = { + href: "/notifications", + tKey: "nav.notifications", + icon: Bell, +}; +const SETTINGS_HUB_ITEM: NavLink = { + href: "/settings/account", + tKey: "nav.settings", + icon: Settings, }; -// The bug-report entry, spliced into the More hub under the same -// `bugReportEnabled` operator flag that gates the desktop sidebar -// entry. Pre-splice the route was desktop-only — a phone user had no -// path to `/bugreport` at all. +// The bug-report entry, appended under the same `bugReportEnabled` +// operator flag that gates the desktop sidebar entry. Pre-splice the +// route was desktop-only — a phone user had no path to `/bugreport`. const BUGREPORT_HUB_ITEM: NavLink = { href: "/bugreport", tKey: "nav.bugreport", @@ -108,23 +99,22 @@ export function BottomNav() { const [moreOpen, setMoreOpen] = useState(false); const [captureOpen, setCaptureOpen] = useState(false); - // v1.15.0 — splice the cycle entry into the More hub after Mood when the - // account's gate is on, so it never appears for accounts without it. - const cycleAwareHub: ReadonlyArray = user?.cycleTrackingEnabled - ? [MORE_HUB[0], MORE_HUB[1], CYCLE_HUB_ITEM, ...MORE_HUB.slice(2)] - : MORE_HUB; - // Bug report sits directly before Settings (the hub's last entry) - // when the operator flag is on — same gate as the desktop sidebar. - const moreHub: ReadonlyArray = bugReportEnabled - ? [ - ...cycleAwareHub.slice(0, -1), - BUGREPORT_HUB_ITEM, - cycleAwareHub[cycleAwareHub.length - 1], - ] - : cycleAwareHub; + // v1.17.1 (F-1) — the More hub is every shared destination that isn't an + // always-visible primary slot, in the model's order, plus the mobile-only + // Notifications / Settings conveniences (and Bug Report behind its flag). + // Cycle is filtered by the same account gate the sidebar uses, so the two + // surfaces gate it identically. + const moreHub = useMemo>(() => { + const shared = visibleNavDestinations(user?.cycleTrackingEnabled) + .filter((d) => !PRIMARY_SLOT_HREFS.includes(d.href as never)) + .map((d) => ({ href: d.href, tKey: d.tKey, icon: d.icon })); + const tail: NavLink[] = [NOTIFICATIONS_HUB_ITEM, SETTINGS_HUB_ITEM]; + if (bugReportEnabled) tail.unshift(BUGREPORT_HUB_ITEM); + return [...shared, ...tail]; + }, [user?.cycleTrackingEnabled, bugReportEnabled]); function isActiveLink(href: string) { - return href === "/" ? pathname === "/" : pathname.startsWith(href); + return isNavDestinationActive(href, pathname); } // The "More" entry is treated as active when any of its children diff --git a/src/components/layout/nav-model.ts b/src/components/layout/nav-model.ts new file mode 100644 index 000000000..da7109d54 --- /dev/null +++ b/src/components/layout/nav-model.ts @@ -0,0 +1,133 @@ +import { + Activity, + Droplets, + Dumbbell, + FlaskConical, + Home, + Lightbulb, + Pill, + Trophy, + Waves, + type LucideIcon, +} from "lucide-react"; + +/** + * v1.17.1 — the single navigation information-model. + * + * Before this module the desktop sidebar and the mobile bottom-nav each + * hand-curated their own destination list, and the two had drifted: the + * sidebar listed Measurements / Mood inline but hid Workouts; the bottom + * bar buried Measurements / Mood in "More" but promoted Workouts — and + * the Coach, the stated differentiator, had no nav home on either. A user + * who learned the product on one platform re-learned it on the other. + * + * This list is the ONE ordered destination model. Both bars render it: + * the sidebar shows every entry in order; the bottom bar keeps its 5-slot + * ergonomic shape (Home · Meds · capture · Insights · More) and derives + * its "More" hub from the SAME list (every destination not already a + * primary slot), so the two bars are re-skins of one story rather than + * two curated lists that drift. + */ +export interface NavDestination { + href: string; + /** i18n key under the `nav.*` namespace. */ + tKey: string; + icon: LucideIcon; + /** + * Stable onboarding-tour anchor. Matches `data-tour-id` lookups in the + * spotlight tour — renaming silently breaks the cutout for that step. + */ + tourId?: string; + /** Gate the entry on the account's `cycleTrackingEnabled` flag. */ + requiresCycle?: boolean; +} + +/** + * The canonical ordered destination list. Cycle sits where it always has + * (after Medications) and is filtered out when the account gate is off; + * the order is otherwise identical for both surfaces. + */ +export const NAV_DESTINATIONS: ReadonlyArray = [ + { href: "/", tKey: "nav.dashboard", icon: Home, tourId: "nav-dashboard" }, + { + href: "/measurements", + tKey: "nav.measurements", + icon: Activity, + tourId: "nav-measurements", + }, + { href: "/mood", tKey: "nav.mood", icon: Waves, tourId: "nav-mood" }, + { + href: "/medications", + tKey: "nav.medications", + icon: Pill, + tourId: "nav-medications", + }, + { + href: "/cycle", + tKey: "nav.cycle", + icon: Droplets, + tourId: "nav-cycle", + requiresCycle: true, + }, + { + href: "/labs", + tKey: "nav.labs", + icon: FlaskConical, + tourId: "nav-labs", + }, + { + href: "/insights/workouts", + tKey: "nav.workouts", + icon: Dumbbell, + tourId: "nav-workouts", + }, + { + href: "/insights", + tKey: "nav.insights", + icon: Lightbulb, + tourId: "nav-insights", + }, + { + href: "/achievements", + tKey: "nav.achievements", + icon: Trophy, + tourId: "nav-achievements", + }, +]; + +/** + * The ordered destinations visible to this account — drops cycle-gated + * entries when the flag is off. Both bars start from this. + */ +export function visibleNavDestinations( + cycleTrackingEnabled: boolean | undefined, +): NavDestination[] { + return NAV_DESTINATIONS.filter( + (d) => !d.requiresCycle || cycleTrackingEnabled === true, + ); +} + +/** + * Whether `href` is the active nav destination for the current `pathname`, + * resolved against the full destination set so the most-specific entry + * wins. Without this, a plain `startsWith("/insights")` would light up + * Insights while the user is on its siblings `/insights/workouts` or + * `/insights/coach` — both of which are now their own top-level nav homes. + * The dashboard (`/`) only matches an exact path. + */ +export function isNavDestinationActive( + href: string, + pathname: string, + destinations: ReadonlyArray = NAV_DESTINATIONS, +): boolean { + if (href === "/") return pathname === "/"; + const matches = (candidate: string) => + pathname === candidate || pathname.startsWith(`${candidate}/`); + if (!matches(href)) return false; + // A longer sibling that also matches is the more specific home — defer + // to it (e.g. on `/insights/coach`, `/insights` must NOT read active). + const moreSpecific = destinations.some( + (d) => d.href !== href && d.href.startsWith(`${href}/`) && matches(d.href), + ); + return !moreSpecific; +} diff --git a/src/components/layout/sidebar-nav.tsx b/src/components/layout/sidebar-nav.tsx index 18ba6e779..c3f1d3bc0 100644 --- a/src/components/layout/sidebar-nav.tsx +++ b/src/components/layout/sidebar-nav.tsx @@ -2,30 +2,26 @@ import { useMemo, useState } from "react"; import { - Activity, Bell, Bug, ChevronsLeft, ChevronsRight, - Droplets, - FlaskConical, - Home, - Lightbulb, LogOut, Monitor, Moon, MoreVertical, - Pill, Settings, Shield, Sun, - Trophy, - Waves, } from "lucide-react"; import Link from "next/link"; import { usePathname } from "next/navigation"; import { useQueryClient } from "@tanstack/react-query"; import { medicationsPrefetchIntentProps } from "@/lib/queries/prefetch-medications"; +import { + isNavDestinationActive, + visibleNavDestinations, +} from "@/components/layout/nav-model"; import { cn } from "@/lib/utils"; import { Logo } from "@/components/ui/logo"; import { useAuth, useLogout } from "@/hooks/use-auth"; @@ -52,51 +48,6 @@ import { const STORAGE_KEY = "healthlog-sidebar-collapsed"; -const navItems = [ - { href: "/", tKey: "nav.dashboard", icon: Home, tourId: "nav-dashboard" }, - { - href: "/measurements", - tKey: "nav.measurements", - icon: Activity, - tourId: "nav-measurements", - }, - { href: "/mood", tKey: "nav.mood", icon: Waves, tourId: "nav-mood" }, - { - href: "/medications", - tKey: "nav.medications", - icon: Pill, - tourId: "nav-medications", - }, - // v1.17.1 — structured lab-result store. Pairs with the Vorsorge - // annual-blood-panel reminder (which records its result here). - { href: "/labs", tKey: "nav.labs", icon: FlaskConical, tourId: "nav-labs" }, - // v1.4.15 Phase B5: `tourId` values match `data-tour-id` lookups - // performed by the onboarding tour. Keep these stable — renaming - // them silently breaks the spotlight cutout for that step. - { - href: "/insights", - tKey: "nav.insights", - icon: Lightbulb, - tourId: "nav-insights", - }, - { - href: "/achievements", - tKey: "nav.achievements", - icon: Trophy, - tourId: "nav-achievements", - }, -]; - -// v1.15.0 — the cycle nav entry, appended to the main list only when the -// account's `cycleTrackingEnabled` gate is true (resolved on `/api/auth/me`). -// Hidden by construction for accounts without the feature. -const cycleNavItem = { - href: "/cycle", - tKey: "nav.cycle", - icon: Droplets, - tourId: "nav-cycle", -} as const; - function getInitials(name: string): string { return name .split(/[\s._-]+/) @@ -287,21 +238,13 @@ export function SidebarNav() { // with no sub-item expansion in the global sidebar — `` // renders its own per-section nav inside the page itself. const onAdminPage = pathname === "/admin" || pathname.startsWith("/admin/"); - // v1.15.5 — surface the cycle entry directly after Medications (before - // Insights) when the gate resolves true, instead of tacking it on at the - // end. Splice by the Medications index so the position survives a reorder - // of the static list; fall back to append if the anchor ever moves. - const visibleNavItems = useMemo(() => { - if (!user?.cycleTrackingEnabled) return navItems; - const afterMedications = - navItems.findIndex((item) => item.href === "/medications") + 1; - if (afterMedications <= 0) return [...navItems, cycleNavItem]; - return [ - ...navItems.slice(0, afterMedications), - cycleNavItem, - ...navItems.slice(afterMedications), - ]; - }, [user?.cycleTrackingEnabled]); + // v1.17.1 (F-1) — the sidebar renders the one shared destination model + // (`nav-model.ts`), the same ordered list the mobile bottom-nav derives + // its "More" hub from. Cycle is filtered in/out by the account gate. + const visibleNavItems = useMemo( + () => visibleNavDestinations(user?.cycleTrackingEnabled), + [user?.cycleTrackingEnabled], + ); const [collapsed, setCollapsed] = useState(() => { if (typeof window === "undefined") return false; try { @@ -404,10 +347,11 @@ export function SidebarNav() { )}
{visibleNavItems.map((item) => { - const isActive = - item.href === "/" - ? pathname === "/" - : pathname.startsWith(item.href); + const isActive = isNavDestinationActive( + item.href, + pathname, + visibleNavItems, + ); const label = t(item.tKey); if (collapsed) {