From d156d227a326e5af03457e432cf04457a46cd02a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 23 May 2026 10:17:41 +0200 Subject: [PATCH] refactor(dashboard): dedup reorder + onboarding tour, gate motion + aria hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - move(id, delta) delegates to reorderWidgets — single swap path - restartOnboardingTour helper extracted; AboutSection + AccountSection consume identical surface - tourMessage + tourMessageType collapsed into one discriminated state - useSortable transition honours prefers-reduced-motion - Drag-handle aria-describedby paragraph gates on widgets.length > 0 Closes M6 + L7 + L8 in v1.4.48 backlog. --- src/components/settings/about-section.tsx | 71 ++++++---------- src/components/settings/account-section.tsx | 73 +++++++--------- .../settings/dashboard-layout-section.tsx | 68 +++++++++++---- .../onboarding/__tests__/tour-restart.test.ts | 77 +++++++++++++++++ src/lib/onboarding/tour-restart.ts | 83 +++++++++++++++++++ 5 files changed, 266 insertions(+), 106 deletions(-) create mode 100644 src/lib/onboarding/__tests__/tour-restart.test.ts create mode 100644 src/lib/onboarding/tour-restart.ts diff --git a/src/components/settings/about-section.tsx b/src/components/settings/about-section.tsx index 8565d8532..2cfdadc4c 100644 --- a/src/components/settings/about-section.tsx +++ b/src/components/settings/about-section.tsx @@ -14,9 +14,9 @@ import { } from "lucide-react"; import { Button } from "@/components/ui/button"; -import { setTourForceLaunch } from "@/components/onboarding/tour-launcher"; import { useAuth } from "@/hooks/use-auth"; import { useFormatters, useTranslations } from "@/lib/i18n/context"; +import { restartOnboardingTour } from "@/lib/onboarding/tour-restart"; import { queryKeys } from "@/lib/query-keys"; interface VersionPayload { @@ -148,53 +148,32 @@ export function AboutSection() { // sources/docs links. Settings → Account still has its own // "Restart onboarding tour" — both buttons share the same flow. const [replayingTour, setReplayingTour] = useState(false); - const [tourMessage, setTourMessage] = useState(null); - const [tourMessageType, setTourMessageType] = useState< - "success" | "error" | null - >(null); + // v1.4.48 M6c — text + type collapsed into one discriminated state so + // the two values can never drift (a "success" message rendered in the + // destructive colour was the latent bug class). + const [tourFeedback, setTourFeedback] = useState<{ + text: string; + type: "success" | "error"; + } | null>(null); async function handleReplayTour() { setReplayingTour(true); - setTourMessage(null); - setTourMessageType(null); - try { - const res = await fetch("/api/onboarding/tour", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ completed: false }), + setTourFeedback(null); + // v1.4.48 M6b — both Settings → Account and Settings → About now + // delegate to the shared `restartOnboardingTour()` worker so the + // server flip + force-launch marker + window event live in one + // place. The helper returns a discriminated result; this surface + // only owns the translation + feedback rendering. + const result = await restartOnboardingTour(user?.id); + if (result.ok) { + setTourFeedback({ + text: t("onboarding.tour.restartConfirmation"), + type: "success", }); - if (!res.ok) { - setTourMessage(t("settings.savingError")); - setTourMessageType("error"); - return; - } - // v1.4.47 W5 — drop a per-user force-launch marker into - // sessionStorage so the next dashboard mount opens the tour - // even when the 24 h auto-launch gate would otherwise suppress - // it (e.g. a first-day user who clicked "Replay the tour" - // from About 30 min after finishing the wizard). The marker is - // session-scoped — a page reload clears it — and consumed - // exactly once by the launcher on its next mount. - if (user?.id) setTourForceLaunch(user.id); - // Fire the same window event Settings → Account uses; a - // dashboard already mounted in another tab / background reopens - // the spotlight immediately. When the user lands on the - // dashboard next, the launcher reads the freshly-flipped - // `onboardingTourCompleted = false` from `/api/auth/me` and the - // force-launch marker set above; either path lands on the tour. - try { - window.dispatchEvent(new CustomEvent("healthlog:tour-restart")); - } catch { - /* ignore — only matters if a dashboard is already mounted */ - } - setTourMessage(t("onboarding.tour.restartConfirmation")); - setTourMessageType("success"); - } catch { - setTourMessage(t("common.networkError")); - setTourMessageType("error"); - } finally { - setReplayingTour(false); + } else { + setTourFeedback({ text: t(result.messageKey), type: "error" }); } + setReplayingTour(false); } // v1.4.36 W4f — the explicit "Check for updates" button is gone; @@ -423,16 +402,16 @@ export function AboutSection() { {t("settings.about.tourReplay")} - {tourMessage && ( + {tourFeedback && (

- {tourMessage} + {tourFeedback.text}

)} diff --git a/src/components/settings/account-section.tsx b/src/components/settings/account-section.tsx index c9b9b4e60..64459b3de 100644 --- a/src/components/settings/account-section.tsx +++ b/src/components/settings/account-section.tsx @@ -39,11 +39,11 @@ import { DateInput } from "@/components/ui/date-input"; import { Label } from "@/components/ui/label"; import { NativeSelect } from "@/components/ui/native-select"; import { PasswordInput } from "@/components/ui/password-input"; -import { setTourForceLaunch } from "@/components/onboarding/tour-launcher"; import { useAuth } from "@/hooks/use-auth"; import { formatDate } from "@/lib/format"; import { locales, localeLabels, type Locale } from "@/lib/i18n/config"; import { useTranslations } from "@/lib/i18n/context"; +import { restartOnboardingTour } from "@/lib/onboarding/tour-restart"; import { describePasskeyError } from "@/lib/passkey-errors"; import { queryKeys } from "@/lib/query-keys"; import { TimezonePicker } from "@/components/settings/timezone-picker"; @@ -135,12 +135,15 @@ export function AccountSection() { // dispatches a `healthlog:tour-restart` window event so the dashboard's // picks it up immediately on the user's next nav. The // confirmation message goes through the same announce channel as - // every other settings save (status: "success" | "error" | null). + // every other settings save. + // v1.4.48 M6c — text + type collapsed into one discriminated state so + // the two values can never drift (same shape as ``'s + // replay button so the surface stays mirrored). const [tourRestarting, setTourRestarting] = useState(false); - const [tourMsg, setTourMsg] = useState(null); - const [tourMsgType, setTourMsgType] = useState<"success" | "error" | null>( - null, - ); + const [tourFeedback, setTourFeedback] = useState<{ + text: string; + type: "success" | "error"; + } | null>(null); // Passkey registration state. const [passkeyLoading, setPasskeyLoading] = useState(false); @@ -280,43 +283,25 @@ export function AccountSection() { async function handleRestartTour() { setTourRestarting(true); - setTourMsg(null); - setTourMsgType(null); - try { - const res = await fetch("/api/onboarding/tour", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ completed: false }), - }); - if (!res.ok) { - setTourMsg(t("settings.savingError")); - setTourMsgType("error"); - return; - } - // v1.4.47 W5 — drop a per-user force-launch marker in - // sessionStorage so the next dashboard mount launches the tour - // even when the new 24 h auto-launch gate would otherwise - // suppress it (e.g. user just finished the wizard 5 min ago). - // The launcher reads-and-clears the marker on mount. - if (user?.id) setTourForceLaunch(user.id); - // Refetch the auth payload so `` re-evaluates - // with the new flag, AND fire a window event so a launcher - // already mounted on the dashboard reopens immediately - // without waiting for a navigation. + setTourFeedback(null); + // v1.4.48 M6b — both Settings → Account and Settings → About now + // delegate to the shared `restartOnboardingTour()` worker so the + // server flip + force-launch marker + window event live in one + // place. Account additionally refetches the auth payload so the + // `onboardingTourCompleted` flag the launcher reads matches the + // server flip immediately — About has no auth handle to refetch + // and relies on the next navigation re-running `/api/auth/me`. + const result = await restartOnboardingTour(user?.id); + if (result.ok) { await refetch(); - try { - window.dispatchEvent(new CustomEvent("healthlog:tour-restart")); - } catch { - /* ignore — only matters if the dashboard is already mounted */ - } - setTourMsg(t("onboarding.tour.restartConfirmation")); - setTourMsgType("success"); - } catch { - setTourMsg(t("common.networkError")); - setTourMsgType("error"); - } finally { - setTourRestarting(false); + setTourFeedback({ + text: t("onboarding.tour.restartConfirmation"), + type: "success", + }); + } else { + setTourFeedback({ text: t(result.messageKey), type: "error" }); } + setTourRestarting(false); } async function handleChangePassword(e: React.FormEvent) { @@ -631,16 +616,16 @@ export function AccountSection() { {t("onboarding.tour.restart")} - {tourMsg && ( + {tourFeedback && (

- {tourMsg} + {tourFeedback.text}

)} diff --git a/src/components/settings/dashboard-layout-section.tsx b/src/components/settings/dashboard-layout-section.tsx index eecab16c8..4f12b85f0 100644 --- a/src/components/settings/dashboard-layout-section.tsx +++ b/src/components/settings/dashboard-layout-section.tsx @@ -29,6 +29,7 @@ import { import { CSS } from "@dnd-kit/utilities"; import { Button } from "@/components/ui/button"; import { Switch } from "@/components/ui/switch"; +import { prefersReducedMotion } from "@/lib/charts/reduced-motion"; import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { @@ -52,6 +53,10 @@ import { * shape (`order: 0..n-1`) so the existing PUT contract stays untouched * and either input mode flushes via the same Save mutation. Exported so * the unit test can pin the contract without spinning up a DndContext. + * + * v1.4.48 M6a — also drives the arrow-button `move()` handler below; + * the previous in-file swap-and-renumber implementation was a second + * copy of the same logic. */ export function reorderWidgets( widgets: readonly { id: string; order: number }[], @@ -70,6 +75,26 @@ export function reorderWidgets( return next.map((w, i) => ({ ...w, order: i })); } +/** + * v1.4.48 M6a — merge the `{ id, order }` shape returned by + * `reorderWidgets()` back into the full `DashboardWidgetConfig[]` so + * the section's draft state keeps every per-row flag (`visible`, + * `tileVisible`) while the order is rewritten. The arrow buttons and + * the @dnd-kit drag-end handler both flow through this helper so + * neither surface can silently drop a flag. + */ +function mergeReorderIntoLayout( + widgets: DashboardLayout["widgets"], + reordered: readonly { id: string; order: number }[], +): DashboardLayout["widgets"] { + const byId = new Map(widgets.map((w) => [w.id, w])); + return reordered.map((r, i) => { + const original = byId.get(r.id as DashboardWidgetId); + if (!original) return { ...r, order: i } as never; + return { ...original, order: i }; + }); +} + const WIDGET_LABEL_KEYS: Record = { weight: "dashboard.weight", bp: "dashboard.bloodPressure", @@ -199,16 +224,25 @@ export function DashboardLayoutSection({ id }: { id: string }) { setDraft({ ...layout, comparisonBaseline: value }); } + /** + * v1.4.48 M6a — the arrow buttons now delegate to `reorderWidgets()` + * so the swap-and-renumber logic lives in exactly one place. The + * neighbour id is derived from the sorted layout index + delta; + * out-of-bounds clicks (top row + ArrowUp, bottom row + ArrowDown) + * short-circuit before the helper sees them so the button disabled + * state stays the single source of truth for the boundary. + */ function move(widgetId: DashboardWidgetId, delta: -1 | 1) { if (!layout) return; const sorted = [...layout.widgets].sort((a, b) => a.order - b.order); const idx = sorted.findIndex((w) => w.id === widgetId); const targetIdx = idx + delta; if (idx < 0 || targetIdx < 0 || targetIdx >= sorted.length) return; - [sorted[idx], sorted[targetIdx]] = [sorted[targetIdx], sorted[idx]]; + const neighbourId = sorted[targetIdx].id; + const reordered = reorderWidgets(layout.widgets, widgetId, neighbourId); setDraft({ ...layout, - widgets: sorted.map((w, i) => ({ ...w, order: i })), + widgets: mergeReorderIntoLayout(layout.widgets, reordered), }); } @@ -229,17 +263,9 @@ export function DashboardLayoutSection({ id }: { id: string }) { String(active.id), String(over.id), ); - // `reorderWidgets` operates on the pure { id, order } shape; map back - // to the full DashboardWidgetConfig so `visible` / `tileVisible` are - // preserved on every row. - const byId = new Map(layout.widgets.map((w) => [w.id, w])); setDraft({ ...layout, - widgets: reordered.map((r, i) => { - const original = byId.get(r.id as DashboardWidgetId); - if (!original) return { ...r, order: i } as never; - return { ...original, order: i }; - }), + widgets: mergeReorderIntoLayout(layout.widgets, reordered), }); } @@ -410,10 +436,14 @@ export function DashboardLayoutSection({ id }: { id: string }) { {/* v1.4.47 W4 — single shared aria-describedby target for all drag handles. Screen readers read this once per focused handle; sighted users see it in the native browser tooltip - via the matching `title` attribute on each handle. */} -

- {t("dashboard.dragHandleHint")} -

+ via the matching `title` attribute on each handle. + v1.4.48 L8 — gate on widget count so an empty layout never + orphans the paragraph (no handles to describe). */} + {layout.widgets.length > 0 && ( +

+ {t("dashboard.dragHandleHint")} +

+ )} )} @@ -509,9 +539,15 @@ function SortableWidgetRow({ isDragging, } = useSortable({ id: widget.id }); + // v1.4.48 L7 — honour the OS `prefers-reduced-motion` preference. The + // rest of HealthLog pairs every transition with a `motion-reduce` + // companion; dnd-kit's default `transform 250ms ease` was the lone + // surface that ignored it. Short-circuit to `none` when reduced + // motion is requested so dragged rows snap to place instead of + // sliding through the list. const style: React.CSSProperties = { transform: CSS.Transform.toString(transform), - transition, + transition: prefersReducedMotion() ? "none" : transition, }; const tileChecked = diff --git a/src/lib/onboarding/__tests__/tour-restart.test.ts b/src/lib/onboarding/__tests__/tour-restart.test.ts new file mode 100644 index 000000000..fb5b0c08b --- /dev/null +++ b/src/lib/onboarding/__tests__/tour-restart.test.ts @@ -0,0 +1,77 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// Stub the launcher import so the helper test never has to instantiate +// the @dnd-kit / launcher render tree just to exercise its +// sessionStorage write side-effect. We assert the helper *calls* it on +// the success branch and skips it when no user-id was passed in. +const setTourForceLaunchSpy = vi.fn(); +vi.mock("@/components/onboarding/tour-launcher", () => ({ + setTourForceLaunch: (userId: string) => setTourForceLaunchSpy(userId), +})); + +import { restartOnboardingTour } from "../tour-restart"; + +describe("restartOnboardingTour()", () => { + beforeEach(() => { + setTourForceLaunchSpy.mockClear(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("success branch — flips the server flag, fires the force-launch marker, returns { ok: true }", async () => { + const fetchImpl = vi + .fn() + .mockResolvedValue(new Response("{}", { status: 200 })); + + const result = await restartOnboardingTour("user-42", { fetchImpl }); + + expect(result).toEqual({ ok: true }); + expect(fetchImpl).toHaveBeenCalledTimes(1); + const [url, init] = fetchImpl.mock.calls[0]; + expect(url).toBe("/api/onboarding/tour"); + expect(init?.method).toBe("POST"); + expect(init?.headers).toEqual({ "Content-Type": "application/json" }); + expect(init?.body).toBe(JSON.stringify({ completed: false })); + // Per-user force-launch marker fired on success. + expect(setTourForceLaunchSpy).toHaveBeenCalledWith("user-42"); + }); + + it("success branch — skips the force-launch marker when no userId was supplied", async () => { + const fetchImpl = vi + .fn() + .mockResolvedValue(new Response("{}", { status: 200 })); + + const result = await restartOnboardingTour(undefined, { fetchImpl }); + + expect(result).toEqual({ ok: true }); + expect(setTourForceLaunchSpy).not.toHaveBeenCalled(); + }); + + it("error-network branch — fetch rejects, returns { ok: false } with common.networkError", async () => { + const fetchImpl = vi + .fn() + .mockRejectedValue(new TypeError("network down")); + + const result = await restartOnboardingTour("user-42", { fetchImpl }); + + expect(result).toEqual({ ok: false, messageKey: "common.networkError" }); + // No force-launch marker should fire on the error branch — a failed + // server flip MUST NOT arm the launcher. + expect(setTourForceLaunchSpy).not.toHaveBeenCalled(); + }); + + it("error-server-422 branch — non-ok response returns { ok: false } with settings.savingError", async () => { + const fetchImpl = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ error: "Unprocessable" }), { + status: 422, + }), + ); + + const result = await restartOnboardingTour("user-42", { fetchImpl }); + + expect(result).toEqual({ ok: false, messageKey: "settings.savingError" }); + expect(setTourForceLaunchSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/onboarding/tour-restart.ts b/src/lib/onboarding/tour-restart.ts new file mode 100644 index 000000000..75c395c8e --- /dev/null +++ b/src/lib/onboarding/tour-restart.ts @@ -0,0 +1,83 @@ +/** + * v1.4.48 M6b — shared "restart onboarding tour" worker. + * + * `` and `` both expose a button that + * resets `users.onboarding_tour_completed = false` and arranges for the + * dashboard's `` to re-open the spotlight on the user's + * next navigation. Until v1.4.48 the two handlers were 90 % identical + * copy-paste — a known drift hazard. This helper owns the surface so + * both call sites narrow to a single mutate-then-route step: + * + * const result = await restartOnboardingTour(user?.id); + * if (result.ok) toast(t("onboarding.tour.restartConfirmation")); + * else toast(t(result.messageKey)); + * + * Side-effects this helper owns: + * + * - PUT `/api/onboarding/tour` with `{ completed: false }` (the same + * server flip both Account and About already drove). + * - On success — write the per-user `setTourForceLaunch(userId)` + * marker into sessionStorage so the next dashboard mount opens + * the tour even when the 24 h auto-launch gate would suppress it. + * - Fire a `healthlog:tour-restart` window event so a dashboard + * mounted in another tab / background reopens the spotlight + * immediately. The dispatch is best-effort; SSR or sandboxed + * iframes that can't construct CustomEvent are tolerated. + * + * Translation + toast rendering are deliberately NOT owned here — the + * helper returns a discriminated `{ ok }` result and lets the React + * component pick the right `useTranslations()` instance. Keeps the + * helper pure of i18n state and easy to unit-test under the default + * vitest "node" environment. + */ + +import { setTourForceLaunch } from "@/components/onboarding/tour-launcher"; + +export type RestartTourResult = + | { ok: true } + | { ok: false; messageKey: string }; + +export interface RestartTourOptions { + /** + * Fetch implementation injected for tests. Defaults to the global + * `fetch` so production code paths read exactly as before. + */ + fetchImpl?: typeof fetch; +} + +/** + * Persist the tour-replay request server-side and arm the launcher + * for the user's next dashboard navigation. + * + * `userId` may be undefined when the auth payload hasn't landed yet — + * the server flip still fires, and the force-launch marker is simply + * skipped (the launcher's secondary "tour-completed = false" path + * still picks the change up on the next `/api/auth/me` read). + */ +export async function restartOnboardingTour( + userId: string | undefined, + opts: RestartTourOptions = {}, +): Promise { + const fetchImpl = opts.fetchImpl ?? fetch; + try { + const res = await fetchImpl("/api/onboarding/tour", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ completed: false }), + }); + if (!res.ok) { + return { ok: false, messageKey: "settings.savingError" }; + } + if (userId) setTourForceLaunch(userId); + try { + if (typeof window !== "undefined") { + window.dispatchEvent(new CustomEvent("healthlog:tour-restart")); + } + } catch { + /* ignore — only matters if a dashboard is already mounted */ + } + return { ok: true }; + } catch { + return { ok: false, messageKey: "common.networkError" }; + } +}