From b5eb5274b34bfc889d2acf00357699beaaf6334d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 30 Jun 2026 23:42:09 +0200 Subject: [PATCH] fix(nav): stop disabled-module entries flickering into the nav on first paint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resolved per-user module map rides the client-only `/api/auth/me` query, so it is still undefined on SSR and the first client paint. The nav model's module gate fails open (`modules?.[key] !== false`), so every module-gated entry — cycle, mood, labs, coach, insights … — rendered for one frame before the query resolved and filtered it out. A user without a module saw its entry flash into the sidebar and the mobile bar, then vanish. Gate the module-dependent filtering behind `useMounted()` in both bars. SSR and the first client paint now render only the core (non-module-gated) destinations; the real module map applies once mounted. Before mount a gated entry is treated as hidden — fail-closed — so SSR and first paint are identical (no hydration mismatch) and a disabled module can never flicker in. The pure nav-model helpers keep their fail-open default for callers that pass a settled map; the bars thread the mounted flag in. Rework the sidebar and bottom-nav SSR tests to assert the fail-closed first-paint contract (core-only, gated entries absent regardless of the map); the resolved post-mount set stays covered by the pure-helper tests in nav-model.test.ts and by e2e. --- .../layout/__tests__/bottom-nav.test.tsx | 34 +++- .../layout/__tests__/sidebar-nav.test.tsx | 153 ++++++++---------- src/components/layout/bottom-nav.tsx | 21 ++- src/components/layout/nav-model.ts | 21 ++- src/components/layout/sidebar-nav.tsx | 11 +- 5 files changed, 139 insertions(+), 101 deletions(-) diff --git a/src/components/layout/__tests__/bottom-nav.test.tsx b/src/components/layout/__tests__/bottom-nav.test.tsx index b1de1dbd8..fba5ead13 100644 --- a/src/components/layout/__tests__/bottom-nav.test.tsx +++ b/src/components/layout/__tests__/bottom-nav.test.tsx @@ -61,13 +61,24 @@ function render() { } describe(" iOS-parity layout", () => { - it("renders the flanking primary anchors (Home, Meds, Insights)", () => { + it("renders the always-on flanking primary anchors (Home, Meds)", () => { const html = render(); - for (const href of ["/", "/medications", "/insights"]) { + // Home + Meds are not module-gated, so they anchor the strip on first + // paint. The Insights slot IS module-gated; v1.25.12 gates it behind + // hydration (fail-closed pre-mount) so a disabled module can't flicker + // in — its presence is asserted in the mounted/e2e path, not here. + for (const href of ["/", "/medications"]) { expect(html).toContain(`href="${href}"`); } }); + it("keeps the module-gated Insights slot OUT of the first-paint markup (v1.25.12)", () => { + const html = render(); + // Pre-hydration the module map isn't settled; the Insights primary slot + // therefore fails closed (absent) rather than flickering in. + expect(html).not.toContain('href="/insights"'); + }); + it("renders the center capture action as a dialog-opening button, not a link", () => { const html = render(); expect(html).toContain('data-testid="bottom-nav-capture"'); @@ -100,11 +111,13 @@ describe(" iOS-parity layout", () => { it("each strip entry meets the 44 px tap-target floor (min-h-11 min-w-11)", () => { const html = render(); - // The four flanking strip slots (Home, Meds, Insights, More) carry - // `min-h-11 min-w-11`. We don't hard-pin the count because a future - // entry would also satisfy the contract. + // The flanking strip slots carry `min-h-11 min-w-11`. On first paint the + // module-gated Insights slot is fail-closed (v1.25.12), so the strip + // shows Home, Meds and More — three slots. We don't hard-pin the count + // because the mounted render adds Insights back and a future entry would + // also satisfy the contract. const matches = html.match(/min-h-11 min-w-11/g) ?? []; - expect(matches.length).toBeGreaterThanOrEqual(4); + expect(matches.length).toBeGreaterThanOrEqual(3); }); it("the center capture FAB is a larger, elevated tap target", () => { @@ -164,10 +177,15 @@ describe(" iOS-parity layout", () => { mockUserRef.value = { id: "u1", modules: {} }; }); - it("keeps the Insights primary slot when the insights module is on", () => { + it("still fails the Insights slot closed on first paint even when the module is on (v1.25.12)", () => { + // Pre-hydration the slot is fail-closed regardless of the map, so the + // disabled-module flicker cannot occur. The mounted render restores the + // slot when the module is on (the resolved set is the pure + // `mobileMoreHubDestinations(..., mounted: true)` contract in + // nav-model.test.ts, plus the e2e mounted render). mockUserRef.value = { id: "u1", modules: { insights: true } }; const html = render(); - expect(html).toContain('href="/insights"'); + expect(html).not.toContain('href="/insights"'); mockUserRef.value = { id: "u1", modules: {} }; }); diff --git a/src/components/layout/__tests__/sidebar-nav.test.tsx b/src/components/layout/__tests__/sidebar-nav.test.tsx index 68ddc5248..f31a5088a 100644 --- a/src/components/layout/__tests__/sidebar-nav.test.tsx +++ b/src/components/layout/__tests__/sidebar-nav.test.tsx @@ -72,111 +72,96 @@ function render({ ); } -describe(" cycle entry gate (v1.15.0 / v1.18.0 module map)", () => { - it("renders the Cycle entry when the cycle module is enabled", () => { - const html = render({ modules: { cycle: true } }); - expect(html).toContain('href="/cycle"'); - expect(html).toContain("Cycle"); +// v1.25.12 — these suites render the FIRST-PAINT (pre-hydration) markup. +// `renderToStaticMarkup` drives `useMounted()` to its server snapshot +// (`false`), so the sidebar applies its fail-CLOSED gate: only CORE +// (non-module-gated) destinations render and EVERY module-gated entry is +// absent regardless of the module map. This is the fix for the disabled- +// module flicker — a gated entry can no longer paint for one frame before the +// `/api/auth/me` query resolves. The resolved, post-mount visible set +// ("cycle shows when its module is on", "a disabled module drops out") is the +// pure `visibleNavDestinations(modules, true)` contract verified in +// `nav-model.test.ts`; the mounted render is exercised in e2e. + +// CORE destinations carry no `requiresModule` and always render. +const CORE_HREFS = ["/", "/measurements", "/checkups"] as const; +// A representative slice of the module-gated entries that must stay out of +// the first-paint markup whatever the module map says. +const GATED_HREFS = [ + "/mood", + "/cycle", + "/medications", + "/labs", + "/illness", + "/insights", + "/coach", + "/achievements", +] as const; + +describe(" first-paint fail-closed module gate (v1.25.12)", () => { + it("renders core destinations on first paint regardless of the module map", () => { + const maps: Record[] = [ + {}, + { cycle: true, mood: true, labs: true, coach: true }, + { cycle: false, mood: false }, + ]; + for (const modules of maps) { + const html = render({ modules }); + for (const href of CORE_HREFS) { + expect(html).toContain(`href="${href}"`); + } + } }); - it("hides the Cycle entry when the cycle module is disabled", () => { - const html = render({ modules: { cycle: false } }); - expect(html).not.toContain('href="/cycle"'); - }); - - it("places the Cycle entry between Mood and Medications when enabled", () => { - const html = render({ modules: { cycle: true } }); - const mood = html.indexOf('href="/mood"'); - const cycle = html.indexOf('href="/cycle"'); - const med = html.indexOf('href="/medications"'); - expect(mood).toBeGreaterThan(-1); - expect(cycle).toBeGreaterThan(-1); - expect(med).toBeGreaterThan(-1); - // v1.19.1 (S4) — Cycle sits after Mood and before Medications in document - // order; Medications opens the fixed clinical/insight spine below it. - expect(mood).toBeLessThan(cycle); - expect(cycle).toBeLessThan(med); - }); -}); - -describe(" module gating (v1.18.0)", () => { - it("hides a disabled module's nav entry (mood / labs / coach / achievements)", () => { - const html = render({ - modules: { - mood: false, - labs: false, - coach: false, - achievements: false, - }, - }); - expect(html).not.toContain('href="/mood"'); - expect(html).not.toContain('href="/labs"'); - expect(html).not.toContain('href="/coach"'); - expect(html).not.toContain('href="/achievements"'); - // Core destinations stay regardless of the module map. - expect(html).toContain('href="/measurements"'); - expect(html).toContain('href="/medications"'); - expect(html).toContain('href="/insights"'); - expect(html).toContain('href="/checkups"'); - }); - - it("renders a module's nav entry when its module is enabled", () => { + it("keeps every module-gated entry OUT of the first-paint markup, even when its module is enabled", () => { + // The flicker bug: a gated entry painted before the module query + // resolved. Pre-mount it must be absent even with the module turned on. const html = render({ modules: { + cycle: true, mood: true, labs: true, coach: true, achievements: true, + insights: true, }, }); - expect(html).toContain('href="/mood"'); - expect(html).toContain('href="/labs"'); - expect(html).toContain('href="/coach"'); - expect(html).toContain('href="/achievements"'); + for (const href of GATED_HREFS) { + expect(html).not.toContain(`href="${href}"`); + } }); - it("fails open: an empty module map keeps every gated entry visible", () => { + it("keeps gated entries out when the module map is empty (no fail-open flicker)", () => { const html = render({ modules: {} }); - expect(html).toContain('href="/mood"'); - expect(html).toContain('href="/cycle"'); - expect(html).toContain('href="/labs"'); - expect(html).toContain('href="/coach"'); - expect(html).toContain('href="/achievements"'); + for (const href of GATED_HREFS) { + expect(html).not.toContain(`href="${href}"`); + } + }); + + it("keeps gated entries out when the module is explicitly disabled", () => { + const html = render({ + modules: { mood: false, labs: false, coach: false, achievements: false }, + }); + for (const href of GATED_HREFS) { + expect(html).not.toContain(`href="${href}"`); + } + }); + + it("does not render the deprecated /insights/workouts sub-link", () => { + const html = render(); + expect(html).not.toContain('href="/insights/workouts"'); }); }); describe(" targets deprecation (v1.8.6)", () => { it("no longer renders the deprecated /targets entry", () => { // The Targets (Zielwerte) page is deprecated; target editing moved - // inline into Insights, so the sidebar drops the entry. The main nav - // links render into the SSR markup, so asserting absence here is a - // real regression net (Insights, the sibling that stays, still shows). + // inline into Insights, so the sidebar drops the entry. Assert absence + // against a stable CORE sibling that renders on first paint + // (Measurements stays; Insights is module-gated and pre-mount absent). const html = render(); expect(html).not.toContain('href="/targets"'); - expect(html).toContain('href="/insights"'); - }); -}); - -describe(" unified destination model (v1.17.1 F-1 / F-3)", () => { - it("surfaces the Coach as a first-class sidebar destination", () => { - // Pre-unify the sidebar had no Coach home while the mobile bar missed - // it too; both now render the one shared model so both carry it. - // (v1.18.0 — Workouts left the left nav for its Insights pill, so it is - // no longer a sidebar destination.) - const html = render(); - expect(html).toContain('href="/coach"'); - expect(html).toContain("Coach"); - expect(html).not.toContain('href="/insights/workouts"'); - }); - - it("marks Coach active without also marking Insights active", () => { - const html = render({ pathname: "/coach" }); - // The Coach link carries aria-current="page"; the standalone /coach - // route is not a sibling of /insights, so the Insights link must not. - const coach = html.match(/]*href="\/coach"[^>]*>/); - const insights = html.match(/]*href="\/insights"[^>]*>/); - expect(coach?.[0]).toMatch(/aria-current="page"/); - expect(insights?.[0]).not.toMatch(/aria-current="page"/); + expect(html).toContain('href="/measurements"'); }); }); diff --git a/src/components/layout/bottom-nav.tsx b/src/components/layout/bottom-nav.tsx index 659559860..e607fad92 100644 --- a/src/components/layout/bottom-nav.tsx +++ b/src/components/layout/bottom-nav.tsx @@ -19,6 +19,7 @@ import { import type { ModuleKey } from "@/lib/modules/registry"; import { useMemo, useState } from "react"; import { useAuth } from "@/hooks/use-auth"; +import { useMounted } from "@/hooks/use-mounted"; import { Sheet, SheetContent, @@ -85,6 +86,13 @@ export function BottomNav() { const [moreOpen, setMoreOpen] = useState(false); const [captureOpen, setCaptureOpen] = useState(false); + // The module map rides the client-only `/api/auth/me` query, so it is not + // settled on SSR or the first client paint. Gating the module filter behind + // `useMounted()` keeps SSR and first paint identical and stops a disabled + // module from flickering into the hub / primary slot before the query + // resolves (the #418-class divergence); once mounted the real map applies. + const mounted = useMounted(); + // v1.17.1 (F-1) — the More hub is the model-computed hub: every visible // feature destination that isn't a primary slot, plus the shared utility // tail. Cycle is gated by the same flag the sidebar uses, so the two @@ -93,21 +101,24 @@ export function BottomNav() { () => mobileMoreHubDestinations({ modules: user?.modules, + mounted, }), - [user?.modules], + [user?.modules, mounted], ); // v1.18.0 — drop a module-gated primary slot (Insights) when the account - // has that module disabled. Fail-open: a missing key / unloaded map keeps - // the slot, mirroring the gate + More-hub default-on contract. + // has that module disabled. Fail-closed until mounted (so a disabled + // Insights never flickers in), then the real map applies: a missing key / + // unloaded map keeps the slot, mirroring the gate + More-hub default-on + // contract. const primaryRight = useMemo( () => PRIMARY_RIGHT.filter( (item) => !item.requiresModule || - user?.modules?.[item.requiresModule] !== false, + (mounted && user?.modules?.[item.requiresModule] !== false), ), - [user?.modules], + [user?.modules, mounted], ); function isActiveLink(href: string) { diff --git a/src/components/layout/nav-model.ts b/src/components/layout/nav-model.ts index c4cb8ba72..6f27b106a 100644 --- a/src/components/layout/nav-model.ts +++ b/src/components/layout/nav-model.ts @@ -242,12 +242,24 @@ export type ModuleVisibilityMap = Partial>; * Whether a destination is visible under the given module map. Core * destinations (no `requiresModule`) always pass; a gated entry passes * unless its module resolves to an explicit `false`. + * + * `mounted` (default `true`) is the hydration gate the nav bars thread in. + * The resolved module map rides the client-only `/api/auth/me` query, which + * can still be unresolved on SSR and the first client paint. Reading the map + * then would fail OPEN and flicker a disabled module's entry in for one frame + * before the query lands and filters it out (the #418-class SSR/client + * divergence). So before mount a gated entry is treated as hidden — + * fail-CLOSED — making SSR and first paint identical (core-only); once + * mounted the real map applies. The default keeps the pure helper fail-open + * for non-component callers that pass a settled map. */ function isNavDestinationVisible( d: NavDestination, modules: ModuleVisibilityMap | undefined, + mounted = true, ): boolean { if (!d.requiresModule) return true; + if (!mounted) return false; return modules?.[d.requiresModule] !== false; } @@ -260,8 +272,11 @@ function isNavDestinationVisible( */ export function visibleNavDestinations( modules: ModuleVisibilityMap | undefined, + mounted = true, ): NavDestination[] { - return NAV_DESTINATIONS.filter((d) => isNavDestinationVisible(d, modules)); + return NAV_DESTINATIONS.filter((d) => + isNavDestinationVisible(d, modules, mounted), + ); } /** @@ -293,8 +308,10 @@ export interface MobileMoreHubEntry { */ export function mobileMoreHubDestinations(opts: { modules: ModuleVisibilityMap | undefined; + /** Hydration gate — see `isNavDestinationVisible`. Defaults to mounted. */ + mounted?: boolean; }): MobileMoreHubEntry[] { - const features = visibleNavDestinations(opts.modules) + const features = visibleNavDestinations(opts.modules, opts.mounted ?? true) .filter((d) => !BOTTOM_NAV_PRIMARY_SLOT_HREFS.includes(d.href)) .map((d) => ({ href: d.href, tKey: d.tKey, icon: d.icon })); const tail = visibleUtilityDestinations().map((d) => ({ diff --git a/src/components/layout/sidebar-nav.tsx b/src/components/layout/sidebar-nav.tsx index d7a96237e..e99c069ee 100644 --- a/src/components/layout/sidebar-nav.tsx +++ b/src/components/layout/sidebar-nav.tsx @@ -25,6 +25,7 @@ import { import { cn } from "@/lib/utils"; import { Logo } from "@/components/ui/logo"; import { useAuth, useLogout } from "@/hooks/use-auth"; +import { useMounted } from "@/hooks/use-mounted"; import { useTheme } from "@/components/providers"; import { useTranslations } from "@/lib/i18n/context"; import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; @@ -241,9 +242,15 @@ export function SidebarNav() { // its "More" hub from. v1.18.0 — entries are filtered by the account's // resolved module map (mood, cycle, labs, coach, achievements …); cycle + // coach are delegated server-side and already reflected in that map. + // The module map rides the client-only `/api/auth/me` query, so it is not + // settled on SSR or the first client paint. Gating the filter behind + // `useMounted()` keeps SSR and first paint identical (core destinations + // only) and stops a disabled module's entry from flickering in for one + // frame before the query resolves; once mounted the real map applies. + const mounted = useMounted(); const visibleNavItems = useMemo( - () => visibleNavDestinations(user?.modules), - [user?.modules], + () => visibleNavDestinations(user?.modules, mounted), + [user?.modules, mounted], ); const [collapsed, setCollapsed] = useState(() => { if (typeof window === "undefined") return false;