From bb4dc124301e5fc8d48b8d3c1602031b0b0675de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 9 May 2026 20:25:28 +0200 Subject: [PATCH] fix(dashboard): no onboarding flicker on load; collapsed by default when incomplete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs fixed at once. The previous version of the dashboard onboarding checklist (a) flashed for ~500 ms on every page-load even for fully onboarded users, and (b) auto-rendered expanded, pushing the real dashboard tiles below the fold each visit. Flicker root cause: the component decided render/no-render off `shouldShowChecklist({ measurementCount, ... })` while `measurementCount` was still 0 because the analytics query was in flight. For a user with 30 measurements + a non-null `onboardingCompletedAt`, `stillInSetup` flipped from `true` (load) to `false` (post-fetch) — exactly long enough to be visible. Fix: refuse to render until `analyticsQuery.data !== undefined`. Tanstack-query writes `data` exactly once per fetch, and the dashboard's `/api/analytics` query shares the cache key, so the checklist almost always reads cached data synchronously and never blocks UI. Auto-open: introduce an `expanded` state, default `false`, persisted to `localStorage[healthlog-getting-started-expanded]` so a deliberate expand survives reloads but new users see a collapsed shell. Header becomes a chevron toggle (`aria-expanded` / `aria-controls`); the progress meter stays visible in both states so a glance still shows how much setup is left. `e2e/onboarding-flicker.spec.ts` covers both contracts: a sample-every-50ms loop during a deliberately-slowed analytics fetch proves the card never paints for a complete-onboarding user, and a second test mounts an incomplete user, asserts `aria-expanded=false` and that `#getting-started-body` is NOT in the DOM until clicked. Co-Authored-By: Claude Opus 4.7 (1M context) --- e2e/onboarding-flicker.spec.ts | 214 +++++++++++++++++ .../onboarding/getting-started-checklist.tsx | 219 ++++++++++++------ 2 files changed, 363 insertions(+), 70 deletions(-) create mode 100644 e2e/onboarding-flicker.spec.ts diff --git a/e2e/onboarding-flicker.spec.ts b/e2e/onboarding-flicker.spec.ts new file mode 100644 index 000000000..888aa0ded --- /dev/null +++ b/e2e/onboarding-flicker.spec.ts @@ -0,0 +1,214 @@ +import { expect, test } from "@playwright/test"; + +import { STORAGE_STATE_PATH } from "./setup/global-setup"; + +/** + * v1.4.15 phase-A3 fix #3 — onboarding card flicker on dashboard load. + * + * The previous version of `` rendered against a + * default-true `shouldShowChecklist` decision while `measurementCount` + * was still 0 (analytics query in flight). For a fully-onboarded user + * (`onboardingCompletedAt != null` AND `measurementCount >= 5`), the + * card briefly flashed before tanstack-query wrote the real value and + * `show` flipped to false on the next render. + * + * This spec asserts the card is NEVER visible — at any sampled instant + * during page load — when both: + * - the auth user has `onboardingCompletedAt != null`, AND + * - `/api/analytics` returns ≥5 measurements. + * + * We slow `/api/analytics` deliberately so any "render-before-data" + * race condition would be fully visible to a Playwright `isVisible()` + * probe; with the fix in place, the card stays unmounted because the + * component refuses to render until `analyticsQuery.data !== undefined`. + */ +test.describe("dashboard onboarding card flicker guard", () => { + test.use({ storageState: STORAGE_STATE_PATH }); + + test("complete-onboarding user never sees the card during load", async ({ + page, + }) => { + // Set the seed user to onboarding-complete BEFORE navigation. The + // global setup leaves `onboardingCompletedAt` null by default so we + // intercept `/api/auth/me` and pin a non-null timestamp. + await page.route("**/api/auth/me", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + data: { + id: "user_e2e", + username: "e2e-tester", + email: "e2e@healthlog.test", + role: "ADMIN", + heightCm: 180, + dateOfBirth: "1990-01-01", + gender: "MALE", + timezone: "Europe/Berlin", + onboardingCompletedAt: "2025-01-01T00:00:00.000Z", + gravatarUrl: null, + glucoseUnit: "mg/dL", + }, + error: null, + }), + }), + ); + + // Slow the analytics endpoint by ~250 ms so a race-condition flash + // would be unmistakable to the visibility probes below; the fix + // works by NOT rendering until this resolves, so even with the + // delay the card stays invisible the whole time. + await page.route("**/api/analytics", async (route) => { + await new Promise((resolve) => setTimeout(resolve, 250)); + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + data: { + summaries: { + // ≥5 readings → past the `measurementCount < 5` setup gate. + WEIGHT: { + latest: 80, + avg7: 80, + avg30: 80, + slope30: { slope: 0, direction: "stable" }, + count: 30, + }, + }, + bpInTargetPct: null, + glucoseByContext: {}, + }, + error: null, + }), + }); + }); + + await page.route("**/api/mood/analytics", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + data: { entries: [], summary: { count: 0 } }, + error: null, + }), + }), + ); + + const card = page.locator('[data-testid="onboarding-card"]'); + + await page.goto("/", { waitUntil: "domcontentloaded" }); + + // Sample visibility every 50 ms for ~600 ms (covers the 250 ms + // analytics delay + render commit + a buffer). If the card mounts + // at any sampled instant, the assertion fails. + const samples: boolean[] = []; + for (let i = 0; i < 12; i++) { + samples.push(await card.isVisible().catch(() => false)); + await page.waitForTimeout(50); + } + + expect(samples.every((visible) => visible === false)).toBe(true); + + // After the network has settled the card is still hidden — the + // user is past the setup gate, so `shouldShowChecklist` returns + // false even with real data. + await page.waitForLoadState("networkidle"); + await expect(card).toBeHidden(); + }); + + test("incomplete-onboarding user sees the card collapsed by default", async ({ + page, + }) => { + // Onboarding is incomplete AND profile fields are missing → the + // `shouldShowChecklist` gate is true. With the fix, the card mounts + // collapsed: header + progress meter visible, full row list hidden + // until the user clicks the chevron. + await page.route("**/api/auth/me", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + data: { + id: "user_e2e", + username: "e2e-tester", + email: "e2e@healthlog.test", + role: "ADMIN", + heightCm: null, + dateOfBirth: null, + gender: null, + timezone: "Europe/Berlin", + onboardingCompletedAt: null, + gravatarUrl: null, + glucoseUnit: "mg/dL", + }, + error: null, + }), + }), + ); + await page.route("**/api/analytics", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + data: { summaries: {}, bpInTargetPct: null, glucoseByContext: {} }, + error: null, + }), + }), + ); + await page.route("**/api/mood/analytics", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + data: { entries: [], summary: { count: 0 } }, + error: null, + }), + }), + ); + await page.route("**/api/medications", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ data: [], error: null }), + }), + ); + await page.route("**/api/withings/status", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ data: { connected: false }, error: null }), + }), + ); + await page.route("**/api/notifications/preferences", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ data: { channels: [] }, error: null }), + }), + ); + + // Make sure no localStorage carries an `expanded=1` from a previous + // session — the spec asserts the *default* is collapsed. + await page.addInitScript(() => { + try { + window.localStorage.removeItem("healthlog-getting-started-expanded"); + } catch { + /* storage unavailable */ + } + }); + + const card = page.locator('[data-testid="onboarding-card"]'); + + await page.goto("/", { waitUntil: "domcontentloaded" }); + await page.waitForLoadState("networkidle"); + + await expect(card).toBeVisible({ timeout: 10_000 }); + + // The collapsed shell exposes the toggle as `aria-expanded=false`; + // the row list (`#getting-started-body`) is unmounted, not just + // hidden, so a `count()` query returns 0 in the collapsed state. + const toggle = card.getByRole("button", { expanded: false }); + await expect(toggle).toBeVisible(); + await expect(card.locator("#getting-started-body")).toHaveCount(0); + }); +}); diff --git a/src/components/onboarding/getting-started-checklist.tsx b/src/components/onboarding/getting-started-checklist.tsx index db535f2ec..74f8c90ad 100644 --- a/src/components/onboarding/getting-started-checklist.tsx +++ b/src/components/onboarding/getting-started-checklist.tsx @@ -3,7 +3,16 @@ import { useEffect, useMemo, useState } from "react"; import Link from "next/link"; import { useQuery } from "@tanstack/react-query"; -import { Activity, Bell, Check, Pill, User2, Wifi, X } from "lucide-react"; +import { + Activity, + Bell, + Check, + ChevronDown, + Pill, + User2, + Wifi, + X, +} from "lucide-react"; import type { LucideIcon } from "lucide-react"; import { Button } from "@/components/ui/button"; @@ -19,6 +28,7 @@ import { const DISMISSED_ITEMS_KEY = "healthlog-getting-started-dismissed"; const DISMISSED_ALL_KEY = "healthlog-getting-started-hidden"; +const EXPANDED_KEY = "healthlog-getting-started-expanded"; const ITEM_ICONS: Record = { profile: User2, @@ -102,6 +112,21 @@ function readDismissedAll(): boolean { } } +function readExpanded(): boolean { + // Default = collapsed. v1.4.15 phase-A3 fix #3: the card auto-opening + // on every page-load was the dominant complaint after the flicker — + // it pushed the actual dashboard tiles below the fold every visit. + // We persist the user's last choice so a deliberate expand survives + // reloads, but the *default* for a brand-new user (no key set) is + // collapsed. + if (typeof window === "undefined") return false; + try { + return window.localStorage.getItem(EXPANDED_KEY) === "1"; + } catch { + return false; + } +} + /** * Persistent dashboard checklist surfaced to brand-new users. Mirrors * the Linear/Notion pattern: visible until the user has either @@ -118,6 +143,12 @@ export function GettingStartedChecklist() { const [dismissedAll, setDismissedAll] = useState(() => readDismissedAll(), ); + // v1.4.15 phase-A3 fix #3 — collapsed by default. The card used to + // render expanded on every load, pushing actual dashboard tiles below + // the fold. The collapsed shell still surfaces the progress meter + + // CTA-row count via `gettingStarted.progress`, so users see at a + // glance how much setup is left without the visual noise. + const [expanded, setExpanded] = useState(() => readExpanded()); // Sync state to localStorage. Effect, not setState-in-effect. useEffect(() => { @@ -141,6 +172,15 @@ export function GettingStartedChecklist() { } }, [dismissedAll]); + useEffect(() => { + if (typeof window === "undefined") return; + try { + window.localStorage.setItem(EXPANDED_KEY, expanded ? "1" : "0"); + } catch { + /* storage may be full or disabled */ + } + }, [expanded]); + // Light-weight queries: each fetch is small and cached by tanstack. // We rely on the React Query cache the dashboard already uses for // analytics, so this won't fire a second request when the dashboard @@ -152,7 +192,7 @@ export function GettingStartedChecklist() { // makes the canonical consumers' `data?.summaries` / `data?.length` // reads silently return undefined — that's how the v1.4.2 dashboard // ended up rendering only the mood tile. - const { data: analyticsData } = useQuery({ + const analyticsQuery = useQuery({ queryKey: ["analytics"], queryFn: async () => { const res = await fetch("/api/analytics"); @@ -162,6 +202,7 @@ export function GettingStartedChecklist() { }, enabled: !!user, }); + const analyticsData = analyticsQuery.data; const { data: medsData } = useQuery>({ queryKey: ["medications"], @@ -256,25 +297,58 @@ export function GettingStartedChecklist() { items, }); - if (!user || !show) return null; + // v1.4.15 phase-A3 fix #3 — flicker guard. Until BOTH the auth user + // AND the analytics query have resolved we render NOTHING. The + // previous version rendered a default-true `show` branch while + // `measurementCount` was still 0 (because analytics hadn't returned), + // so a user whose actual `measurementCount >= 5` saw the card flash + // for ~500 ms before `shouldShowChecklist` flipped to false. We rely + // on `analyticsQuery.data === undefined` as the loading sentinel — + // tanstack-query writes `data` exactly once per fetch, so this is + // race-free across hot-reloads and stale-cache invalidation. (The + // dashboard's `/api/analytics` query shares the cache key, so the + // checklist almost always sees the cached value synchronously on + // mount and never blocks UI.) + if (!user) return null; + if (analyticsQuery.data === undefined) return null; + if (!show) return null; return (
-
-

- {t("gettingStarted.title")} -

-

- {t("gettingStarted.subtitle")} -

-
+
- {/* Progress meter — non-decorative, exposed to screen readers. */} + {/* Progress meter — non-decorative, exposed to screen readers. + Stays visible in both expanded and collapsed states so a + glance at the card tells the user how much setup is left + without forcing the full row-list back open. */}
-
    - {visible.map((item) => { - const Icon = ITEM_ICONS[item.id]; - const labels = ITEM_LABEL_KEYS[item.id]; - return ( -
  • -
  • - {item.done ? ( - - ) : ( - - )} - -
    - -

    - {t(labels.description)} -

    -
    - {!item.done ? ( - - ) : null} - -
    • - ); - })} -
+ {item.done ? ( + + ) : ( + + )} + +
+

+ {t(labels.title)} +

+

+ {t(labels.description)} +

+
+ {!item.done ? ( + + ) : null} + + + ); + })} + + ) : null}
); }