mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(nav): stop disabled-module entries flickering into the nav on first paint
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.
This commit is contained in:
1 parent
5e691d3be2
commit
b5eb5274b3
5 files changed
+139
-101
No files matched your search
@@ -61,13 +61,24 @@ function render() {
|
||||
}
|
||||
|
||||
describe("<BottomNav> 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("<BottomNav> 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("<BottomNav> 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: {} };
|
||||
});
|
||||
|
||||
|
||||
@@ -72,111 +72,96 @@ function render({
|
||||
);
|
||||
}
|
||||
|
||||
describe("<SidebarNav> 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("<SidebarNav> first-paint fail-closed module gate (v1.25.12)", () => {
|
||||
it("renders core destinations on first paint regardless of the module map", () => {
|
||||
const maps: Record<string, boolean>[] = [
|
||||
{},
|
||||
{ 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("<SidebarNav> 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("<SidebarNav> 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("<SidebarNav> 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(/<a[^>]*href="\/coach"[^>]*>/);
|
||||
const insights = html.match(/<a[^>]*href="\/insights"[^>]*>/);
|
||||
expect(coach?.[0]).toMatch(/aria-current="page"/);
|
||||
expect(insights?.[0]).not.toMatch(/aria-current="page"/);
|
||||
expect(html).toContain('href="/measurements"');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -242,12 +242,24 @@ export type ModuleVisibilityMap = Partial<Record<ModuleKey, boolean>>;
|
||||
* 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) => ({
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in new issue
Block a user