refactor(dashboard): dedup reorder + onboarding tour, gate motion + aria hint

- 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.
This commit is contained in:
Marc-André Bombeck committed 2026-05-23 10:24:13 +02:00
1 parent 12b7b4a6ef
commit d156d227a3
5 files changed
+266 -106

No files matched your search

+25 -46
View File
@@ -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<string | null>(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")}
</Button>
</div>
{tourMessage && (
{tourFeedback && (
<p
role="alert"
className={`mt-2 text-xs ${
tourMessageType === "success"
tourFeedback.type === "success"
? "text-dracula-green"
: "text-destructive"
}`}
>
{tourMessage}
{tourFeedback.text}
</p>
)}
</div>
+29 -44
View File
@@ -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
// <TourLauncher> 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 `<AboutSection>`'s
// replay button so the surface stays mirrored).
const [tourRestarting, setTourRestarting] = useState(false);
const [tourMsg, setTourMsg] = useState<string | null>(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 `<TourLauncher>` 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")}
</Button>
</div>
{tourMsg && (
{tourFeedback && (
<p
role="alert"
className={`mt-2 text-xs ${
tourMsgType === "success"
tourFeedback.type === "success"
? "text-dracula-green"
: "text-destructive"
}`}
>
{tourMsg}
{tourFeedback.text}
</p>
)}
</div>
@@ -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<DashboardWidgetId, string> = {
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. */}
<p id={dragHintId} className="text-muted-foreground sr-only">
{t("dashboard.dragHandleHint")}
</p>
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 && (
<p id={dragHintId} className="text-muted-foreground sr-only">
{t("dashboard.dragHandleHint")}
</p>
)}
</div>
)}
@@ -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 =
@@ -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<typeof fetch>()
.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<typeof fetch>()
.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<typeof fetch>()
.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<typeof fetch>().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();
});
});
+83
View File
@@ -0,0 +1,83 @@
/**
* v1.4.48 M6b — shared "restart onboarding tour" worker.
*
* `<AccountSection>` and `<AboutSection>` both expose a button that
* resets `users.onboarding_tour_completed = false` and arranges for the
* dashboard's `<TourLauncher>` 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<RestartTourResult> {
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" };
}
}