From ffde57869b58a0827e7959c7bd0f334cacb45a40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 11 Nov 2026 21:32:07 +0100 Subject: [PATCH 1/9] fix(sheets): ask before a dismissal drops typed input The eight-step medication wizard, the add sheets on /measurements, /mood and /labs, and the visit, vaccination, practitioner and checkup forms closed on a stray backdrop tap or swipe and lost what was typed. The dashboard quick entry and the capture menu asked first, through two hand-copied dialogs and a DOM check that read the first sheet body in the document and counted the date field's prefilled "now" as input. ResponsiveSheet now takes a dirty flag, or a check it hands its own body, and asks with the existing discard dialog. Forms that keep a seed pass whether they differ from it; the wizard compares what a save would send. Plain add forms pass the DOM check, which skips the date and time fields again. The three copies of the dialog are gone. --- .../baselines/source-function-lines.json | 7 +- .../baselines/version-stamp-comments.json | 9 +- src/app/__tests__/sheet-dirty-check.test.ts | 140 ------ src/app/labs/page.tsx | 6 +- src/app/measurements/page.tsx | 6 +- src/app/mood/page-client.tsx | 6 +- .../dashboard/quick-entry-sheets.tsx | 136 +---- src/components/encounters/encounter-sheet.tsx | 4 +- .../__tests__/capture-picker-discard.test.tsx | 40 +- src/components/layout/capture-picker.tsx | 69 +-- .../checkups/reminder-form-sheet.tsx | 4 + .../vorsorge-section.tsx | 2 + .../wizard/medication-wizard-dialog.tsx | 34 +- .../practitioners/practitioner-sheet.tsx | 4 +- .../ui/__tests__/responsive-sheet.test.tsx | 76 ++- .../ui/__tests__/sheet-dirty-check.test.ts | 100 ++++ src/components/ui/responsive-sheet.tsx | 473 +++++++++++------- .../vaccinations/vaccination-sheet.tsx | 4 +- .../workouts/manual-workout-sheet.tsx | 104 ++-- 19 files changed, 580 insertions(+), 644 deletions(-) delete mode 100644 src/app/__tests__/sheet-dirty-check.test.ts create mode 100644 src/components/ui/__tests__/sheet-dirty-check.test.ts diff --git a/src/__tests__/baselines/source-function-lines.json b/src/__tests__/baselines/source-function-lines.json index 1ab04100f..c5d17530a 100644 --- a/src/__tests__/baselines/source-function-lines.json +++ b/src/__tests__/baselines/source-function-lines.json @@ -45,7 +45,7 @@ "app/insights/catalog/page.tsx :: MetricCatalogPage": 204, "app/insights/medications/page.tsx :: InsightsMedikamentePage": 265, "app/insights/page-client.tsx :: InsightsPageClient": 375, - "app/labs/page.tsx :: LabsPage": 216, + "app/labs/page.tsx :: LabsPage": 217, "app/medications/page-client.tsx :: MedicationsPageClient": 590, "app/notifications/page.tsx :: NotificationsPage": 327, "components/activity/activity-calendar-card.tsx :: ActivityCalendarCard": 470, @@ -139,12 +139,11 @@ "components/labs/ocr-row-editor.tsx :: OcrRowEditor": 378, "components/layout/auth-shell.tsx :: AuthShell": 518, "components/layout/bottom-nav.tsx :: BottomNav": 262, - "components/layout/capture-picker.tsx :: CapturePicker": 235, "components/layout/sidebar-nav.tsx :: SidebarNav": 356, "components/layout/top-bar.tsx :: TopBar": 212, "components/links/entity-link-picker.tsx :: EntityLinkPicker": 392, "components/measurement-reminders/vorsorge-dashboard-card.tsx :: VorsorgeDashboardCard": 215, - "components/measurement-reminders/vorsorge-section.tsx :: VorsorgeSection": 352, + "components/measurement-reminders/vorsorge-section.tsx :: VorsorgeSection": 354, "components/measurements/measurement-form.tsx :: MeasurementForm": 500, "components/medications/detail/efficacy/efficacy-chart.tsx :: EfficacyChart": 361, "components/medications/detail/medication-detail-tabs.tsx :: MedicationDetailTabs": 767, @@ -230,7 +229,7 @@ "components/timeline/timeline-chart.tsx :: ChartBody": 236, "components/timeline/timeline-chronicle.tsx :: TimelineChronicle": 246, "components/timeline/timeline-view.tsx :: TimelineView": 486, - "components/ui/responsive-sheet.tsx :: ResponsiveSheet": 239, + "components/ui/responsive-sheet.tsx :: ResponsiveSheet": 253, "components/vaccinations/catalog-picker.tsx :: CatalogPicker": 221, "components/vaccinations/custom-vaccine-sheet.tsx :: CustomVaccineSheet": 246, "components/vaccinations/vaccination-form.tsx :: VaccinationForm": 218, diff --git a/src/__tests__/baselines/version-stamp-comments.json b/src/__tests__/baselines/version-stamp-comments.json index 0804205bb..2e6046700 100644 --- a/src/__tests__/baselines/version-stamp-comments.json +++ b/src/__tests__/baselines/version-stamp-comments.json @@ -1,7 +1,7 @@ { "$comment": "Comment lines that open with a release version, per file under src/. Maintained by src/__tests__/version-stamp-ratchet.test.ts.", - "count": 2892, - "total": 9213, + "count": 2886, + "total": 9194, "entries": { "__tests__/account-payload-consumer-guard.test.ts": 2, "__tests__/acting-account-boundary-guard.test.ts": 7, @@ -80,7 +80,6 @@ "app/__tests__/not-found-copy.test.ts": 1, "app/__tests__/quick-add-labels.test.ts": 3, "app/__tests__/root-loading-silhouette.test.tsx": 1, - "app/__tests__/sheet-dirty-check.test.ts": 1, "app/about/page.tsx": 1, "app/achievements/__tests__/page.test.tsx": 2, "app/achievements/page.tsx": 4, @@ -808,7 +807,7 @@ "components/dashboard/chart-reveal.tsx": 1, "components/dashboard/dashboard-gates.ts": 9, "components/dashboard/medication-intake-quick-add.tsx": 11, - "components/dashboard/quick-entry-sheets.tsx": 6, + "components/dashboard/quick-entry-sheets.tsx": 3, "components/dashboard/range-display.tsx": 1, "components/dashboard/recent-workouts-tile.tsx": 3, "components/data-list/__tests__/selection.test.ts": 1, @@ -1096,7 +1095,7 @@ "components/layout/account-switcher-menu.tsx": 1, "components/layout/auth-shell.tsx": 14, "components/layout/bottom-nav.tsx": 8, - "components/layout/capture-picker.tsx": 5, + "components/layout/capture-picker.tsx": 4, "components/layout/nav-model.ts": 17, "components/layout/offline-banner.tsx": 3, "components/layout/record-session-fence-bridge.tsx": 1, diff --git a/src/app/__tests__/sheet-dirty-check.test.ts b/src/app/__tests__/sheet-dirty-check.test.ts deleted file mode 100644 index 9a27f1056..000000000 --- a/src/app/__tests__/sheet-dirty-check.test.ts +++ /dev/null @@ -1,140 +0,0 @@ -/** - * v1.11.5 — `sheetBodyHasUnsavedInput` dirty-check coverage. - * - * The quick-entry sheets confirm-on-dismiss only when the body holds unsaved - * input. The original walk read `input` / `textarea` only, so a user who - * changed just the mood `role="radio"` selector or opened a Radix Select - * (type / medication) could lose the change on a backdrop dismiss with no - * confirm. These cases lock the extended detection in — and pin the - * over-trigger guard so a pristine sheet (no radio chosen, every Select on its - * default and closed) still reads clean. - * - * The repo runs vitest in the Node environment (no jsdom / happy-dom). The - * function only ever touches `document.querySelector` once (the sheet body) and - * then `body.querySelectorAll` / `body.querySelector` against three fixed - * selectors, so we drive it with a hand-rolled fake `document` that answers - * exactly those selectors — no DOM engine, matching the repo's node-only test - * convention (cf. the SSR-string tests elsewhere). - */ -import { afterEach, describe, expect, it } from "vitest"; -import { sheetBodyHasUnsavedInput } from "@/components/dashboard/quick-entry-sheets"; - -const BODY_SELECTOR = '[data-slot="responsive-sheet-body"]'; -const MOOD_RADIO_SELECTOR = '[role="radio"][aria-checked="true"]'; -const OPEN_SELECT_SELECTOR = '[role="combobox"][data-state="open"]'; - -interface FakeField { - type: string; - value?: string; - disabled?: boolean; - checked?: boolean; - defaultChecked?: boolean; -} - -interface SheetFixture { - fields?: FakeField[]; - moodRadioSelected?: boolean; - openSelect?: boolean; -} - -/** Install a fake `document` whose queries answer the given fixture. */ -function installFakeDocument(fixture: SheetFixture | null): void { - const body = fixture - ? { - querySelectorAll: (sel: string) => - sel === "input, textarea" ? (fixture.fields ?? []) : [], - querySelector: (sel: string) => { - if (sel === MOOD_RADIO_SELECTOR) - return fixture.moodRadioSelected ? {} : null; - if (sel === OPEN_SELECT_SELECTOR) - return fixture.openSelect ? {} : null; - return null; - }, - } - : null; - - (globalThis as { document?: unknown }).document = { - querySelector: (sel: string) => (sel === BODY_SELECTOR ? body : null), - }; -} - -afterEach(() => { - delete (globalThis as { document?: unknown }).document; -}); - -describe("sheetBodyHasUnsavedInput — mood radio", () => { - it("is clean when no mood radio is selected (pristine)", () => { - installFakeDocument({ moodRadioSelected: false }); - expect(sheetBodyHasUnsavedInput()).toBe(false); - }); - - it("is dirty when a mood radio is selected", () => { - installFakeDocument({ moodRadioSelected: true }); - expect(sheetBodyHasUnsavedInput()).toBe(true); - }); -}); - -describe("sheetBodyHasUnsavedInput — Radix Select", () => { - it("is clean for a closed Select on its default value (no over-trigger)", () => { - // A measurement-type / medication Select mounts with a non-placeholder - // DEFAULT and is closed — a pristine sheet must NOT prompt the confirm. - installFakeDocument({ openSelect: false }); - expect(sheetBodyHasUnsavedInput()).toBe(false); - }); - - it("is dirty while a Select dropdown is open (mid-interaction)", () => { - installFakeDocument({ openSelect: true }); - expect(sheetBodyHasUnsavedInput()).toBe(true); - }); -}); - -describe("sheetBodyHasUnsavedInput — text fields still honoured", () => { - it("is clean for an empty pristine sheet", () => { - installFakeDocument({ - fields: [ - { type: "text", value: "" }, - { type: "textarea", value: "" }, - { type: "datetime-local", value: "2026-06-04T08:00" }, - ], - }); - expect(sheetBodyHasUnsavedInput()).toBe(false); - }); - - it("is dirty when a text input carries a value", () => { - installFakeDocument({ fields: [{ type: "text", value: "75.5" }] }); - expect(sheetBodyHasUnsavedInput()).toBe(true); - }); - - it("ignores a prefilled date/time picker (v1.11.4 H1)", () => { - installFakeDocument({ - fields: [{ type: "datetime-local", value: "2026-06-04T08:00" }], - }); - expect(sheetBodyHasUnsavedInput()).toBe(false); - }); - - it("ignores a disabled field carrying a value", () => { - installFakeDocument({ - fields: [{ type: "text", value: "ignored", disabled: true }], - }); - expect(sheetBodyHasUnsavedInput()).toBe(false); - }); - - it("is dirty for a native checkbox toggled off its default", () => { - installFakeDocument({ - fields: [{ type: "checkbox", checked: true, defaultChecked: false }], - }); - expect(sheetBodyHasUnsavedInput()).toBe(true); - }); -}); - -describe("sheetBodyHasUnsavedInput — no sheet body", () => { - it("is clean when no sheet body is mounted", () => { - installFakeDocument(null); - expect(sheetBodyHasUnsavedInput()).toBe(false); - }); - - it("is clean when document is undefined (SSR)", () => { - delete (globalThis as { document?: unknown }).document; - expect(sheetBodyHasUnsavedInput()).toBe(false); - }); -}); diff --git a/src/app/labs/page.tsx b/src/app/labs/page.tsx index 4105720f0..599c7b169 100644 --- a/src/app/labs/page.tsx +++ b/src/app/labs/page.tsx @@ -31,7 +31,10 @@ import { } from "@/components/ui/dropdown-menu"; import { PageHeader } from "@/components/ui/page-header"; import { PullToRefreshIndicator } from "@/components/ui/pull-to-refresh-indicator"; -import { ResponsiveSheet } from "@/components/ui/responsive-sheet"; +import { + ResponsiveSheet, + sheetBodyHasUnsavedInput, +} from "@/components/ui/responsive-sheet"; import { useAuth } from "@/hooks/use-auth"; import { useMounted } from "@/hooks/use-mounted"; import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; @@ -218,6 +221,7 @@ export default function LabsPage() { { setDialogOpen(open); if (!open) { diff --git a/src/app/mood/page-client.tsx b/src/app/mood/page-client.tsx index 0139f4f56..161f5e112 100644 --- a/src/app/mood/page-client.tsx +++ b/src/app/mood/page-client.tsx @@ -13,7 +13,10 @@ import { Button } from "@/components/ui/button"; import { PageHeader } from "@/components/ui/page-header"; import { PageAuthGate } from "@/components/ui/page-auth-gate"; import { PullToRefreshIndicator } from "@/components/ui/pull-to-refresh-indicator"; -import { ResponsiveSheet } from "@/components/ui/responsive-sheet"; +import { + ResponsiveSheet, + sheetBodyHasUnsavedInput, +} from "@/components/ui/responsive-sheet"; import { ArrowRight, Plus, Wrench } from "lucide-react"; import { useRouter, useSearchParams } from "next/navigation"; import { useEffect } from "react"; @@ -177,6 +180,7 @@ export default function MoodPageClient() { } > diff --git a/src/components/dashboard/quick-entry-sheets.tsx b/src/components/dashboard/quick-entry-sheets.tsx index 074b25e27..c416033fc 100644 --- a/src/components/dashboard/quick-entry-sheets.tsx +++ b/src/components/dashboard/quick-entry-sheets.tsx @@ -3,14 +3,17 @@ /** * Quick-entry sheets for the dashboard: measurement, mood, and medication * intake behind the shared ResponsiveSheet primitive - * (bottom-sheet on `( - '[data-slot="responsive-sheet-body"]', - ); - if (!body) return false; - const fields = body.querySelectorAll( - "input, textarea", - ); - for (const field of fields) { - if (field.disabled || field.type === "hidden") continue; - // Skip pickers the form prefills with "now" — an untouched default - // is not user input (v1.11.3 QA H1). - if (PREFILLED_PICKER_TYPES.has(field.type)) continue; - if (field.type === "checkbox" || field.type === "radio") { - const box = field as HTMLInputElement; - if (box.checked !== box.defaultChecked) return true; - continue; - } - if (field.value.trim() !== "") return true; - } - // v1.11.5 — the input/textarea walk misses two non-text controls a user can - // change before dismissing: the mood `role="radio"` selector and a Radix - // Select (measurement type / medication). Losing either silently on a - // backdrop-dismiss is the same data-loss class the H1 confirm-on-dirty guard - // was meant to close. - // - // Mood radios start with nothing selected (`mood = ""`), so ANY - // `aria-checked="true"` is a deliberate user choice — flag it. - if (body.querySelector('[role="radio"][aria-checked="true"]') !== null) { - return true; - } - // Radix Select trigger renders `role="combobox"`. A trigger with - // `data-state="open"` is mid-interaction (the dropdown is up) — the user is - // actively choosing, so a dismiss should confirm rather than drop the pick. - // We deliberately do NOT treat a merely non-placeholder value as dirty: the - // type / medication selects mount with a non-placeholder DEFAULT, and - // flagging that would over-trigger the confirm on a pristine sheet. - if (body.querySelector('[role="combobox"][data-state="open"]') !== null) { - return true; - } - return false; -} - export function QuickEntrySheets({ open, onClose, @@ -181,21 +111,8 @@ export function QuickEntrySheets({ ); const [lifeEventFooterEl, setLifeEventFooterEl] = useState(null); - // v1.11.3 — when an open quick-entry sheet is dismissed with - // unsaved input, hold the close in this flag and surface a confirm - // instead of nulling the dialog outright. Cleared once the user - // confirms (close) or keeps editing (dismiss the confirm). - const [confirmDiscardOpen, setConfirmDiscardOpen] = useState(false); - - // Intercept a sheet dismiss: close immediately when the form is - // clean, otherwise keep the sheet open and ask before discarding. function handleQuickEntryOpenChange(nextOpen: boolean) { - if (nextOpen) return; - if (sheetBodyHasUnsavedInput()) { - setConfirmDiscardOpen(true); - return; - } - onClose(); + if (!nextOpen) onClose(); } return ( @@ -203,6 +120,7 @@ export function QuickEntrySheets({ } > @@ -215,6 +133,7 @@ export function QuickEntrySheets({ } > @@ -231,6 +150,7 @@ export function QuickEntrySheets({ } @@ -267,6 +188,7 @@ export function QuickEntrySheets({ } @@ -285,6 +207,7 @@ export function QuickEntrySheets({ } > @@ -296,39 +219,6 @@ export function QuickEntrySheets({ /> )} - - {/* v1.11.3 — confirm before discarding a partly-filled - quick-entry form when the sheet is dismissed by an overlay - tap, Escape, or a mobile swipe-down. Keeps the sheet open - until the user decides. */} - - - - - {t("dashboard.quickEntryDiscard.title")} - - - {t("dashboard.quickEntryDiscard.description")} - - - - - {t("dashboard.quickEntryDiscard.cancel")} - - { - setConfirmDiscardOpen(false); - onClose(); - }} - > - {t("dashboard.quickEntryDiscard.confirm")} - - - - ); } diff --git a/src/components/encounters/encounter-sheet.tsx b/src/components/encounters/encounter-sheet.tsx index 8ce3a0bf1..c3483585b 100644 --- a/src/components/encounters/encounter-sheet.tsx +++ b/src/components/encounters/encounter-sheet.tsx @@ -92,7 +92,7 @@ export function EncounterSheet({ // varying its `key`, so a second edit never shows the first one's values and // a create after an edit starts empty — without an effect that copies props // into state and briefly renders the stale pair. - const [draft, setDraft] = useState(() => + const [seed] = useState(() => encounter ? draftFromEncounter(encounter) : emptyDraft({ @@ -101,6 +101,7 @@ export function EncounterSheet({ ...(kind ? { kind } : {}), }), ); + const [draft, setDraft] = useState(seed); const [error, setError] = useState(null); const isEdit = encounter !== null; @@ -140,6 +141,7 @@ export function EncounterSheet({ ({ cursor: 0, values: [] as unknown[], })); -const captureState = vi.hoisted(() => ({ - dirty: true, -})); // v1.36.x — the picker asks what the record allows before it offers a kind. // These fixtures are the caller's own record, which is every kind; the @@ -57,29 +54,17 @@ vi.mock("@/lib/i18n/context", () => ({ useTranslations: () => ({ t: (key: string) => key }), })); -vi.mock("@/components/dashboard/quick-entry-sheets", () => ({ - sheetBodyHasUnsavedInput: () => captureState.dirty, -})); - function markedComponent(displayName: string) { const Component = ({ children }: { children?: ReactNode }) => children; Component.displayName = displayName; return Component; } +const sheetBodyHasUnsavedInput = vi.hoisted(() => () => true); + vi.mock("@/components/ui/responsive-sheet", () => ({ ResponsiveSheet: markedComponent("ResponsiveSheet"), -})); - -vi.mock("@/components/ui/alert-dialog", () => ({ - AlertDialog: markedComponent("AlertDialog"), - AlertDialogAction: markedComponent("AlertDialogAction"), - AlertDialogCancel: markedComponent("AlertDialogCancel"), - AlertDialogContent: markedComponent("AlertDialogContent"), - AlertDialogDescription: markedComponent("AlertDialogDescription"), - AlertDialogFooter: markedComponent("AlertDialogFooter"), - AlertDialogHeader: markedComponent("AlertDialogHeader"), - AlertDialogTitle: markedComponent("AlertDialogTitle"), + sheetBodyHasUnsavedInput, })); vi.mock("@/components/dashboard/quick-entry-forms.lazy", () => ({ @@ -141,14 +126,13 @@ function formSheet(tree: ReactNode): TestElement { return sheets[1]; } -describe(" — confirmed capture-form discard", () => { +describe(" — capture-form discard", () => { beforeEach(() => { hookState.cursor = 0; hookState.values.length = 0; - captureState.dirty = true; }); - it("asks before discarding a dirty form and closes on confirm", () => { + it("hands the form sheet the dirty check, so a dismissal asks first", () => { let tree = renderPicker(); // Choose the mood capture; the form sheet opens with the mood form. @@ -158,19 +142,13 @@ describe(" — confirmed capture-form discard", () => { tree = renderPicker(); expect(formSheet(tree).props.open).toBe(true); expect(findMarked(tree, "MoodForm")).not.toBeNull(); + // The sheet asks before a dismissal drops typed input + // (`responsive-sheet.test.tsx` pins the asking). + expect(formSheet(tree).props.dirty).toBe(sheetBodyHasUnsavedInput); - // A dirty dismiss must not close outright — it raises the confirm. + // A close the sheet lets through closes the form. (formSheet(tree).props.onOpenChange as (open: boolean) => void)(false); tree = renderPicker(); - const confirmDiscard = findMarked(tree, "AlertDialogAction"); - expect(confirmDiscard).not.toBeNull(); - // The form stays open behind the confirm. - expect(formSheet(tree).props.open).toBe(true); - expect(findMarked(tree, "MoodForm")).not.toBeNull(); - - // Confirming the discard closes the form. - (confirmDiscard?.props.onClick as () => void)(); - tree = renderPicker(); expect(formSheet(tree).props.open).toBe(false); expect(findMarked(tree, "MoodForm")).toBeNull(); }); diff --git a/src/components/layout/capture-picker.tsx b/src/components/layout/capture-picker.tsx index ddf72586f..59f738192 100644 --- a/src/components/layout/capture-picker.tsx +++ b/src/components/layout/capture-picker.tsx @@ -10,7 +10,10 @@ import { Waves, } from "lucide-react"; -import { ResponsiveSheet } from "@/components/ui/responsive-sheet"; +import { + ResponsiveSheet, + sheetBodyHasUnsavedInput, +} from "@/components/ui/responsive-sheet"; import { ManualWorkoutForm, MeasurementForm, @@ -19,17 +22,6 @@ import { SymptomEntryForm, } from "@/components/dashboard/quick-entry-forms.lazy"; import { LifeEventForm } from "@/components/timeline/life-event-form.lazy"; -import { - AlertDialog, - AlertDialogAction, - AlertDialogCancel, - AlertDialogContent, - AlertDialogDescription, - AlertDialogFooter, - AlertDialogHeader, - AlertDialogTitle, -} from "@/components/ui/alert-dialog"; -import { sheetBodyHasUnsavedInput } from "@/components/dashboard/quick-entry-sheets"; import { useTranslations } from "@/lib/i18n/context"; import { isSurfaceVisible, type SurfaceModuleMap } from "@/lib/modules/surface"; import type { ShareDomain } from "@/lib/sharing/scope"; @@ -60,11 +52,9 @@ import { * from their own pages (`/measurements`, `/mood`, dashboard) and from * the More hub. The picker is an additional fast path, not a removal. * - * v1.30.1 — the form sheet shares the dashboard quick-entry sheets' - * confirm-before-discard guard (`sheetBodyHasUnsavedInput()`), so a - * swipe-down / backdrop tap / Escape on a half-filled form from the - * primary mobile capture path asks before dropping typed input, - * instead of closing unconditionally. + * The form sheet asks before a swipe-down, backdrop tap, Escape or Back + * drops a half-filled form (`ResponsiveSheet`'s `dirty`), the same way the + * dashboard quick-entry sheets do. */ export type CaptureKind = @@ -192,7 +182,6 @@ export function CapturePicker({ // v1.30.1 — mirrors `QuickEntrySheets`' `confirmDiscardOpen`: hold a // dismiss attempt here instead of closing outright when the form // body carries unsaved input. - const [confirmDiscardOpen, setConfirmDiscardOpen] = useState(false); function chooseKind(next: CaptureKind) { if (!offered.includes(next)) return; @@ -206,16 +195,8 @@ export function CapturePicker({ setKind(null); } - // Intercept a form-sheet dismiss: close immediately when the form is - // clean, otherwise keep the sheet open and ask before discarding — - // same contract as `QuickEntrySheets.handleQuickEntryOpenChange`. function handleFormOpenChange(next: boolean) { - if (next) return; - if (sheetBodyHasUnsavedInput()) { - setConfirmDiscardOpen(true); - return; - } - closeForm(); + if (!next) closeForm(); } const allOptions: ReadonlyArray<{ @@ -318,6 +299,7 @@ export function CapturePicker({ } > @@ -370,39 +352,6 @@ export function CapturePicker({ /> )} - - {/* v1.30.1 — confirm before discarding a partly-filled capture - form when the sheet is dismissed by an overlay tap, Escape, - or a mobile swipe-down. Copy shared with the dashboard - quick-entry sheets' identical guard. */} - - - - - {t("dashboard.quickEntryDiscard.title")} - - - {t("dashboard.quickEntryDiscard.description")} - - - - - {t("dashboard.quickEntryDiscard.cancel")} - - { - setConfirmDiscardOpen(false); - closeForm(); - }} - > - {t("dashboard.quickEntryDiscard.confirm")} - - - - ); } diff --git a/src/components/measurement-reminders/checkups/reminder-form-sheet.tsx b/src/components/measurement-reminders/checkups/reminder-form-sheet.tsx index 4678e58e9..9ec98457b 100644 --- a/src/components/measurement-reminders/checkups/reminder-form-sheet.tsx +++ b/src/components/measurement-reminders/checkups/reminder-form-sheet.tsx @@ -29,6 +29,7 @@ export function ReminderFormSheet({ isEdit, form, setForm, + dirty, saving, onClose, onSubmit, @@ -37,6 +38,8 @@ export function ReminderFormSheet({ isEdit: boolean; form: FormState; setForm: Dispatch>; + /** The form differs from what it opened with. */ + dirty: boolean; saving: boolean; onClose: () => void; onSubmit: () => void; @@ -48,6 +51,7 @@ export function ReminderFormSheet({ onOpenChange={(open) => { if (!open) onClose(); }} + dirty={dirty} title={t( isEdit ? "measurementReminders.form.editTitle" diff --git a/src/components/measurement-reminders/vorsorge-section.tsx b/src/components/measurement-reminders/vorsorge-section.tsx index 055c731c9..6726430f3 100644 --- a/src/components/measurement-reminders/vorsorge-section.tsx +++ b/src/components/measurement-reminders/vorsorge-section.tsx @@ -109,6 +109,7 @@ export function VorsorgeSection({ function openCreate() { setForm(EMPTY_FORM); + setInitialForm(EMPTY_FORM); setEditing("new"); } @@ -277,6 +278,7 @@ export function VorsorgeSection({ isEdit={isEdit} form={form} setForm={setForm} + dirty={JSON.stringify(form) !== JSON.stringify(initialForm)} saving={saving} onClose={closeSheet} onSubmit={submit} diff --git a/src/components/medications/wizard/medication-wizard-dialog.tsx b/src/components/medications/wizard/medication-wizard-dialog.tsx index d96cbcddc..c9c21b855 100644 --- a/src/components/medications/wizard/medication-wizard-dialog.tsx +++ b/src/components/medications/wizard/medication-wizard-dialog.tsx @@ -108,6 +108,26 @@ const STEP_ICONS = { type StepNumber = keyof typeof STEP_ICONS; +/** + * Whether closing would lose input: what a save would send now against what + * it would have sent on opening. Moving between steps or schedules is not + * input. + */ +function wizardHasInput( + seed: WizardPayload, + payload: WizardPayload, + mode: MedicationWizardDialogProps["mode"], +): boolean { + try { + return ( + JSON.stringify(buildCreateBody(payload, mode)) !== + JSON.stringify(buildCreateBody(seed, mode)) + ); + } catch { + return true; + } +} + export function MedicationWizardDialog(props: MedicationWizardDialogProps) { // Key the inner state container on (open + mode + medication id) so // every open gets a fresh state tree. React unmounts the inner @@ -139,18 +159,15 @@ function WizardDialogShell({ const router = useRouter(); const queryClient = useQueryClient(); - const [payload, setPayload] = useState(() => + const [seed] = useState(() => mode === "edit" && initial ? hydrateWizardPayload(initial) : emptyWizardPayload(), ); - const [step, setStep] = useState(() => { - if (mode === "edit" && initial) { - const hydrated = hydrateWizardPayload(initial); - return landingStepForEdit(hydrated) as StepNumber; - } - return 1; - }); + const [payload, setPayload] = useState(seed); + const [step, setStep] = useState(() => + mode === "edit" && initial ? (landingStepForEdit(seed) as StepNumber) : 1, + ); const [submitting, setSubmitting] = useState(false); const [submitError, setSubmitError] = useState(null); const [nlOpen, setNlOpen] = useState(false); @@ -415,6 +432,7 @@ function WizardDialogShell({ wizardHasInput(seed, payload, mode)} title={headerTitle} hideHeader showCloseButton diff --git a/src/components/practitioners/practitioner-sheet.tsx b/src/components/practitioners/practitioner-sheet.tsx index 54118fa5a..6bcf0136c 100644 --- a/src/components/practitioners/practitioner-sheet.tsx +++ b/src/components/practitioners/practitioner-sheet.tsx @@ -82,7 +82,8 @@ export function PractitionerSheet({ // open, so a second edit never shows the first one's values and a create // after an edit starts empty — without an effect that copies props into // state and briefly renders the stale pair. - const [draft, setDraft] = useState(() => draftFrom(practitioner)); + const [seed] = useState(() => draftFrom(practitioner)); + const [draft, setDraft] = useState(seed); const [error, setError] = useState(null); const pending = create.isPending || update.isPending; @@ -122,6 +123,7 @@ export function PractitionerSheet({ void) | null } = { + onOpenChange: null, +}; vi.mock("@/hooks/use-is-mobile", () => ({ useIsMobile: () => mobile, })); +vi.mock("@/components/forms/use-seeded-form-dismissal", () => ({ + SeededFormDiscardDialog: ({ open }: { open: boolean }) => + open ?
: null, +})); + vi.mock("@/components/ui/sheet", () => ({ - Sheet: ({ children, open }: { children: React.ReactNode; open: boolean }) => - open ?
{children}
: null, + Sheet: ({ + children, + open, + onOpenChange, + }: { + children: React.ReactNode; + open: boolean; + onOpenChange: (open: boolean) => void; + }) => { + radix.onOpenChange = onOpenChange; + return open ?
{children}
: null; + }, SheetContent: ({ children, className, @@ -62,8 +82,18 @@ vi.mock("@/components/ui/sheet", () => ({ })); vi.mock("@/components/ui/dialog", () => ({ - Dialog: ({ children, open }: { children: React.ReactNode; open: boolean }) => - open ?
{children}
: null, + Dialog: ({ + children, + open, + onOpenChange, + }: { + children: React.ReactNode; + open: boolean; + onOpenChange: (open: boolean) => void; + }) => { + radix.onOpenChange = onOpenChange; + return open ?
{children}
: null; + }, DialogContent: ({ children, className, @@ -325,6 +355,44 @@ describe("", () => { }); }); +describe(" dismissal with unsaved input", () => { + function dismiss(dirty: boolean | (() => boolean) | undefined) { + const onOpenChange = vi.fn(); + renderToStaticMarkup( + +

body

+ , + ); + radix.onOpenChange?.(false); + return onOpenChange; + } + + it.each([true, () => true])( + "keeps the sheet open when the form has input (%s)", + (dirty) => { + mobile = true; + expect(dismiss(dirty)).not.toHaveBeenCalled(); + mobile = false; + expect(dismiss(dirty)).not.toHaveBeenCalled(); + }, + ); + + it.each([false, () => false, undefined])( + "closes at once when there is nothing to lose (%s)", + (dirty) => { + mobile = true; + expect(dismiss(dirty)).toHaveBeenCalledWith(false); + mobile = false; + expect(dismiss(dirty)).toHaveBeenCalledWith(false); + }, + ); +}); + describe("keepKeyboardDownOnOpen (phone branch open focus)", () => { function setup(activeInside: boolean) { const focus = vi.fn(); diff --git a/src/components/ui/__tests__/sheet-dirty-check.test.ts b/src/components/ui/__tests__/sheet-dirty-check.test.ts new file mode 100644 index 000000000..80868167b --- /dev/null +++ b/src/components/ui/__tests__/sheet-dirty-check.test.ts @@ -0,0 +1,100 @@ +/** + * `sheetBodyHasUnsavedInput`, the fallback dirty check an add form passes to + * `ResponsiveSheet`. It reads the one sheet body it is handed. Tests run in + * Node, so the body is a fake that answers the three queries the check makes. + */ +import { describe, expect, it } from "vitest"; + +import { sheetBodyHasUnsavedInput } from "@/components/ui/responsive-sheet"; + +const MOOD_RADIO_SELECTOR = '[role="radio"][aria-checked="true"]'; +const OPEN_SELECT_SELECTOR = '[role="combobox"][data-state="open"]'; + +interface FakeField { + type: string; + value?: string; + disabled?: boolean; + checked?: boolean; + defaultChecked?: boolean; + /** The text overlay of a `DateField` / `TimeField`. */ + inDateOrTimeField?: boolean; +} + +function fakeBody(fixture: { + fields?: FakeField[]; + moodRadioSelected?: boolean; + openSelect?: boolean; +}): HTMLElement { + const fields = (fixture.fields ?? []).map((field) => ({ + ...field, + value: field.value ?? "", + closest: () => (field.inDateOrTimeField ? {} : null), + })); + return { + querySelectorAll: (selector: string) => + selector === "input, textarea" ? fields : [], + querySelector: (selector: string) => { + if (selector === MOOD_RADIO_SELECTOR) { + return fixture.moodRadioSelected ? {} : null; + } + if (selector === OPEN_SELECT_SELECTOR) { + return fixture.openSelect ? {} : null; + } + return null; + }, + } as unknown as HTMLElement; +} + +describe("sheetBodyHasUnsavedInput", () => { + it("is clean for a pristine form whose date and time read 'now'", () => { + const body = fakeBody({ + fields: [ + { type: "text", value: "" }, + { type: "textarea", value: "" }, + { type: "text", value: "04.06.2026", inDateOrTimeField: true }, + { type: "text", value: "08:00", inDateOrTimeField: true }, + { type: "hidden", value: "2026-06-04" }, + ], + }); + expect(sheetBodyHasUnsavedInput(body)).toBe(false); + }); + + it("is dirty when a text field carries a value", () => { + const body = fakeBody({ fields: [{ type: "text", value: "75.5" }] }); + expect(sheetBodyHasUnsavedInput(body)).toBe(true); + }); + + it("ignores a disabled field carrying a value", () => { + const body = fakeBody({ + fields: [{ type: "text", value: "ignored", disabled: true }], + }); + expect(sheetBodyHasUnsavedInput(body)).toBe(false); + }); + + it("is dirty for a checkbox moved off its default", () => { + const body = fakeBody({ + fields: [{ type: "checkbox", checked: true, defaultChecked: false }], + }); + expect(sheetBodyHasUnsavedInput(body)).toBe(true); + }); + + it("is dirty when a mood face is chosen, clean when none is", () => { + expect( + sheetBodyHasUnsavedInput(fakeBody({ moodRadioSelected: true })), + ).toBe(true); + expect( + sheetBodyHasUnsavedInput(fakeBody({ moodRadioSelected: false })), + ).toBe(false); + }); + + it("is dirty while a Select is open, clean on its closed default", () => { + expect(sheetBodyHasUnsavedInput(fakeBody({ openSelect: true }))).toBe(true); + expect(sheetBodyHasUnsavedInput(fakeBody({ openSelect: false }))).toBe( + false, + ); + }); + + it("is clean without a body", () => { + expect(sheetBodyHasUnsavedInput(null)).toBe(false); + }); +}); diff --git a/src/components/ui/responsive-sheet.tsx b/src/components/ui/responsive-sheet.tsx index 93134ef60..41f9f5925 100644 --- a/src/components/ui/responsive-sheet.tsx +++ b/src/components/ui/responsive-sheet.tsx @@ -4,6 +4,7 @@ import * as React from "react"; import { cn } from "@/lib/utils"; import { useIsMobile } from "@/hooks/use-is-mobile"; +import { SeededFormDiscardDialog } from "@/components/forms/use-seeded-form-dismissal"; import { Dialog, DialogContent, @@ -95,6 +96,14 @@ export interface ResponsiveSheetProps { * `event.preventDefault()` and focus the opener. */ onCloseAutoFocus?: (event: Event) => void; + /** + * Whether the form holds input a dismissal would lose. A backdrop tap, + * Escape, a swipe down and the X then ask before closing; the form's + * own Cancel and Save close it as before. A function is read at the moment + * of the dismissal and gets this sheet's body: pass + * `sheetBodyHasUnsavedInput` for an add form that keeps its state inside. + */ + dirty?: boolean | ((body: HTMLElement | null) => boolean); children: React.ReactNode; } @@ -148,6 +157,82 @@ export function keepKeyboardDownOnOpen(event: Event): void { } } +/** + * The fallback dirty check for an add form that keeps its state inside the + * sheet body: any text field with a value, a checkbox or radio moved off its + * default, a chosen `role="radio"` (the mood faces) or a Select that is open. + * The date and time fields are skipped, because the forms fill them with + * "now". An edit form starts filled and has to say for itself whether + * anything changed. + */ +export function sheetBodyHasUnsavedInput(body: HTMLElement | null): boolean { + if (!body) return false; + const fields = body.querySelectorAll( + "input, textarea", + ); + for (const field of fields) { + if (field.disabled || field.type === "hidden") continue; + if (field.closest('[data-slot="date-field"], [data-slot="time-field"]')) { + continue; + } + if (field.type === "checkbox" || field.type === "radio") { + const box = field as HTMLInputElement; + if (box.checked !== box.defaultChecked) return true; + continue; + } + if (field.value.trim() !== "") return true; + } + // Mood faces start with none chosen, so any chosen one is input. + if (body.querySelector('[role="radio"][aria-checked="true"]') !== null) { + return true; + } + // A Select that is open is a choice being made. One merely off its + // placeholder is not: the type and medication selects open on a default. + if (body.querySelector('[role="combobox"][data-state="open"]') !== null) { + return true; + } + return false; +} + +/** How a sheet closes: a dismissal with unsaved input asks first. */ +function useSheetDismissal({ + open, + onOpenChange, + dirty, +}: Pick) { + const bodyRef = React.useRef(null); + const [asking, setAsking] = React.useState(false); + if (!open && asking) setAsking(false); + + const hasUnsavedInput = () => + typeof dirty === "function" ? dirty(bodyRef.current) : dirty === true; + + // Every dismissal Radix reports (backdrop, Escape, swipe, the X). + const handleOpenChange = (next: boolean) => { + if (!next && hasUnsavedInput()) { + setAsking(true); + return; + } + onOpenChange(next); + }; + + // A sibling of the sheet, not a child: inside it, a touch on the question + // would reach the sheet's drag through the React tree. + const discard = + dirty === undefined ? null : ( + { + setAsking(false); + onOpenChange(false); + }} + onCancel={() => setAsking(false)} + /> + ); + + return { bodyRef, handleOpenChange, discard }; +} + export function ResponsiveSheet({ open, onOpenChange, @@ -161,67 +246,87 @@ export function ResponsiveSheet({ showCloseButton = true, contentWidth = "md", onCloseAutoFocus, + dirty, children, }: ResponsiveSheetProps) { const isMobile = useIsMobile(); + const { bodyRef, handleOpenChange, discard } = useSheetDismissal({ + open, + onOpenChange, + dirty, + }); if (isMobile) { return ( - - - {hideHeader ? ( - - {title} - {description ? ( - {description} - ) : null} - - ) : ( - - {headerAction ? ( - <> -
+ <> + + + {hideHeader ? ( + + {title} + {description ? ( + {description} + ) : null} + + ) : ( + + {headerAction ? ( + <> +
+ + {title} + + {description ? ( + + {description} + + ) : null} +
+
{headerAction}
+ + ) : ( + <> {title} @@ -230,160 +335,154 @@ export function ResponsiveSheet({ {description} ) : null} + + )} + + )} +
+ {children} +
+ {footer ? ( + + {footer} + + ) : null} + + + {discard} + + ); + } + + return ( + <> + + + {hideHeader ? ( + + {title} + {description ? ( + {description} + ) : null} + + ) : ( + + {headerAction ? ( + <> +
+ {title} + {description ? ( + {description} + ) : null}
{headerAction}
) : ( <> - - {title} - + {title} {description ? ( - - {description} - + {description} ) : null} )} - + )}
`'s deliberate 4 px hover bleed + // (`-mx-1 w-[calc(100%+0.5rem)]`) both produced a horizontal + // scrollbar on the desktop dialog. `clip` is not `visible`, so the + // promotion does not happen and the vertical scroll is kept. + // + // Content that genuinely needs to scroll sideways (a wide table) + // owns its own `overflow-x-auto` wrapper — that is the correct + // place for it, and it still works inside a clipped port. + // + // `-mr-3 pr-3` reclaims a 12 px scroll gutter from the dialog's own + // `p-6` padding: the vertical scrollbar would otherwise render flush + // against the body's right edge, exactly where a right-aligned child + // (a slider's high anchor label) sits, painting on top of the last + // characters. The margin gives the padding straight back, so content + // alignment is unchanged — the scrollbar just lives beside the text + // now, not on it. The mobile Sheet branch has its own `p-4` and + // needs nothing. + "-mr-3 flex min-h-0 flex-1 flex-col gap-4 overflow-x-clip overflow-y-auto pr-3", bodyClassName, )} > {children}
{footer ? ( - {footer} - + ) : null} - - - ); - } - - return ( - - - {hideHeader ? ( - - {title} - {description ? ( - {description} - ) : null} - - ) : ( - - {headerAction ? ( - <> -
- {title} - {description ? ( - {description} - ) : null} -
-
{headerAction}
- - ) : ( - <> - {title} - {description ? ( - {description} - ) : null} - - )} - - )} -
`'s deliberate 4 px hover bleed - // (`-mx-1 w-[calc(100%+0.5rem)]`) both produced a horizontal - // scrollbar on the desktop dialog. `clip` is not `visible`, so the - // promotion does not happen and the vertical scroll is kept. - // - // Content that genuinely needs to scroll sideways (a wide table) - // owns its own `overflow-x-auto` wrapper — that is the correct - // place for it, and it still works inside a clipped port. - // - // `-mr-3 pr-3` reclaims a 12 px scroll gutter from the dialog's own - // `p-6` padding: the vertical scrollbar would otherwise render flush - // against the body's right edge, exactly where a right-aligned child - // (a slider's high anchor label) sits, painting on top of the last - // characters. The margin gives the padding straight back, so content - // alignment is unchanged — the scrollbar just lives beside the text - // now, not on it. The mobile Sheet branch has its own `p-4` and - // needs nothing. - "-mr-3 flex min-h-0 flex-1 flex-col gap-4 overflow-x-clip overflow-y-auto pr-3", - bodyClassName, - )} - > - {children} -
- {footer ? ( - - {footer} - - ) : null} - - + + + {discard} + ); } diff --git a/src/components/vaccinations/vaccination-sheet.tsx b/src/components/vaccinations/vaccination-sheet.tsx index a63972299..dffedee21 100644 --- a/src/components/vaccinations/vaccination-sheet.tsx +++ b/src/components/vaccinations/vaccination-sheet.tsx @@ -76,9 +76,10 @@ export function VaccinationSheet({ const detail = useVaccination(vaccination?.id ?? null); const [confirmingDelete, setConfirmingDelete] = useState(false); // Seeded once at mount; the caller remounts on every open by varying `key`. - const [draft, setDraft] = useState(() => + const [seed] = useState(() => vaccination ? draftFromVaccination(vaccination) : emptyDraft(), ); + const [draft, setDraft] = useState(seed); const [error, setError] = useState(null); const isEdit = vaccination !== null; @@ -111,6 +112,7 @@ export function VaccinationSheet({ (null); - const [confirmDiscardOpen, setConfirmDiscardOpen] = useState(false); const editDirty = useRef(false); const handleDirtyChange = useCallback((dirty: boolean) => { editDirty.current = dirty; }, []); - function handleOpenChange(next: boolean) { - if (next) return onOpenChange(true); - if (edit ? editDirty.current : sheetBodyHasUnsavedInput()) { - setConfirmDiscardOpen(true); - return; - } - onOpenChange(false); - } - return ( - <> - } - > - {open && ( - onOpenChange(false)} - onCancel={() => onOpenChange(false)} - footerSlot={footerEl} - edit={edit} - onDirtyChange={edit ? handleDirtyChange : undefined} - /> - )} - - - - - - {t("dashboard.quickEntryDiscard.title")} - - - {t("dashboard.quickEntryDiscard.description")} - - - - - {t("dashboard.quickEntryDiscard.cancel")} - - { - setConfirmDiscardOpen(false); - onOpenChange(false); - }} - > - {t("dashboard.quickEntryDiscard.confirm")} - - - - - + editDirty.current : sheetBodyHasUnsavedInput} + title={t( + edit + ? "insights.workouts.manual.editTitle" + : "insights.workouts.manual.sheetTitle", + )} + description={t("insights.workouts.manual.sheetDescription")} + footer={
} + > + {open && ( + onOpenChange(false)} + onCancel={() => onOpenChange(false)} + footerSlot={footerEl} + edit={edit} + onDirtyChange={edit ? handleDirtyChange : undefined} + /> + )} + ); } From 6f9b42c35e6f5839ef92083e34264a67620b8ed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 11 Nov 2026 21:33:48 +0100 Subject: [PATCH 2/9] fix(sheets): let Back close the open sheet Back on a phone (the Android button, the edge swipe of the installed app) with a sheet open went back a page under it: the sheet and what was typed in it were gone, in the edit sheets too. Only the account form guarded Back, behind an option. Opening a ResponsiveSheet now pushes one history entry with the page's URL. Back lands below it and closes the sheet, asking first when it holds unsaved input or when its owner keeps it open; a sheet closed any other way takes its entry off again, so the next Back still leaves the page. A history write made while that Back is under way waits for it, a link followed from a sheet replaces the sheet's entry, and the page entry keeps the URL the page has now, so a stripped one-shot `?add=` does not come back. The vault's document sheet already owns `?doc=` and opts out. --- .../baselines/source-function-lines.json | 4 +- src/__tests__/url-state-writes-guard.test.ts | 2 + .../documents/document-detail-sheet.tsx | 2 + .../ui/__tests__/sheet-history.test.ts | 286 ++++++++++++++++ src/components/ui/responsive-sheet.tsx | 53 ++- src/components/ui/sheet-history.ts | 316 ++++++++++++++++++ 6 files changed, 658 insertions(+), 5 deletions(-) create mode 100644 src/components/ui/__tests__/sheet-history.test.ts create mode 100644 src/components/ui/sheet-history.ts diff --git a/src/__tests__/baselines/source-function-lines.json b/src/__tests__/baselines/source-function-lines.json index c5d17530a..512931154 100644 --- a/src/__tests__/baselines/source-function-lines.json +++ b/src/__tests__/baselines/source-function-lines.json @@ -90,7 +90,7 @@ "components/documents/document-ai-section.tsx :: DocumentAiSection": 262, "components/documents/document-bulk-bar.tsx :: DocumentBulkBar": 204, "components/documents/document-card.tsx :: DocumentCard": 354, - "components/documents/document-detail-sheet.tsx :: DocumentDetailSheet": 325, + "components/documents/document-detail-sheet.tsx :: DocumentDetailSheet": 327, "components/documents/document-facts-review.tsx :: DocumentFactsSection": 242, "components/documents/document-facts-review.tsx :: FactRow": 261, "components/documents/document-filter-bar.tsx :: DocumentFilterBar": 348, @@ -229,7 +229,7 @@ "components/timeline/timeline-chart.tsx :: ChartBody": 236, "components/timeline/timeline-chronicle.tsx :: TimelineChronicle": 246, "components/timeline/timeline-view.tsx :: TimelineView": 486, - "components/ui/responsive-sheet.tsx :: ResponsiveSheet": 253, + "components/ui/responsive-sheet.tsx :: ResponsiveSheet": 255, "components/vaccinations/catalog-picker.tsx :: CatalogPicker": 221, "components/vaccinations/custom-vaccine-sheet.tsx :: CustomVaccineSheet": 246, "components/vaccinations/vaccination-form.tsx :: VaccinationForm": 218, diff --git a/src/__tests__/url-state-writes-guard.test.ts b/src/__tests__/url-state-writes-guard.test.ts index bb4a087f9..29b7605fc 100644 --- a/src/__tests__/url-state-writes-guard.test.ts +++ b/src/__tests__/url-state-writes-guard.test.ts @@ -24,6 +24,8 @@ const ALLOWED: Record = { "lib/url-state.ts": "the helper itself", "components/clinician/share-unlock-gate.tsx": "scrubs a passphrase from the fragment synchronously, before the unlock request leaves; no query state", + "components/ui/sheet-history.ts": + "a sheet's own history entry: the page's URL unchanged, never query state", "components/documents/vault/selection-history.ts": "closeDocumentSelectionHistoryEntry takes a history-shaped argument for its tests; the page passes `urlStateHistory`", }; diff --git a/src/components/documents/document-detail-sheet.tsx b/src/components/documents/document-detail-sheet.tsx index 25e3cd02a..a3fb0308d 100644 --- a/src/components/documents/document-detail-sheet.tsx +++ b/src/components/documents/document-detail-sheet.tsx @@ -131,6 +131,8 @@ export function DocumentDetailSheet({ void; + +/** + * A session history the way a browser keeps it: `back()` is queued and + * answered later with a popstate, capture listeners hear it before bubble + * ones, and the router is a bubble listener registered first. + */ +function fakeBrowser(start = "https://app.test/measurements") { + const entries: Array<{ state: unknown; href: string }> = [ + { state: { __NA: true }, href: start }, + ]; + let index = 0; + const queued: Array<() => void> = []; + const capture: Listener[] = []; + const bubble: Listener[] = []; + const routerHeard: string[] = []; + const timers = new Map void>(); + let timerId = 0; + + const resolve = (url: string | URL | null | undefined) => + url == null + ? entries[index].href + : new URL(String(url), entries[index].href).href; + + function dispatch() { + let stopped = false; + const event = { stopImmediatePropagation: () => (stopped = true) }; + for (const listener of [...capture, ...bubble]) { + if (stopped) break; + listener(event); + } + } + + function traverse(delta: number) { + const target = index + delta; + if (target < 0 || target >= entries.length) return; + index = target; + dispatch(); + } + + const host: SheetHistoryHost = { + history: { + get state() { + return entries[index].state; + }, + pushState(data, _unused, url) { + entries.splice(index + 1); + entries.push({ state: data, href: resolve(url) }); + index += 1; + }, + replaceState(data, _unused, url) { + entries[index] = { state: data, href: resolve(url) }; + }, + back() { + queued.push(() => traverse(-1)); + }, + }, + location: { + get href() { + return entries[index].href; + }, + }, + addEventListener(_type, listener, useCapture) { + (useCapture ? capture : bubble).push(listener); + }, + dispatchEvent: () => true, + setTimeout(callback) { + timerId += 1; + timers.set(timerId, callback); + return timerId; + }, + clearTimeout(handle) { + timers.delete(handle as number); + }, + }; + // The router listens from the start, in the bubble phase. + bubble.push(() => routerHeard.push(entries[index].href)); + + return { + host, + entries, + get index() { + return index; + }, + routerHeard, + /** The browser answers the Backs that were asked for. */ + settle() { + while (queued.length > 0) queued.shift()!(); + }, + /** The person presses Back. */ + pressBack() { + traverse(-1); + }, + /** The person picks an entry further back from the Back button's list. */ + go(delta: number) { + traverse(delta); + }, + runTimers() { + const pending = [...timers.values()]; + timers.clear(); + for (const run of pending) run(); + }, + }; +} + +function marked(state: unknown): boolean { + return ( + state !== null && + typeof state === "object" && + "__healthlogSheet" in (state as Record) + ); +} + +describe("sheet history", () => { + it("Back closes the sheet and the page under it stays where it was", () => { + const browser = fakeBrowser(); + const sheets = createSheetHistory(browser.host); + const onBack = vi.fn(); + sheets.open(onBack); + expect(browser.entries).toHaveLength(2); + + browser.pressBack(); + + expect(onBack).toHaveBeenCalledTimes(1); + expect(browser.index).toBe(0); + expect(browser.host.location.href).toBe("https://app.test/measurements"); + // Nothing changed for the page, so the router never hears of it. + expect(browser.routerHeard).toEqual([]); + }); + + it("a sheet closed by its own controls takes its entry off again", () => { + const browser = fakeBrowser(); + const sheets = createSheetHistory(browser.host); + const onBack = vi.fn(); + const entry = sheets.open(onBack); + + entry.release(); + browser.settle(); + + expect(browser.index).toBe(0); + expect(onBack).not.toHaveBeenCalled(); + expect(browser.routerHeard).toEqual([]); + expect(marked(browser.host.history.state)).toBe(false); + }); + + it("a page pushed right after the close is not undone by the close's Back", () => { + const browser = fakeBrowser("https://app.test/medications"); + const sheets = createSheetHistory(browser.host); + const entry = sheets.open(vi.fn()); + + // A save closes the wizard and opens the new medication at once. + entry.release(); + browser.host.history.pushState({ __NA: true }, "", "/medications/m1"); + browser.settle(); + + expect(browser.entries.map((e) => e.href)).toEqual([ + "https://app.test/medications", + "https://app.test/medications/m1", + ]); + expect(browser.index).toBe(1); + }); + + it("a link followed from inside the sheet replaces the sheet's entry", () => { + const browser = fakeBrowser("https://app.test/settings"); + const sheets = createSheetHistory(browser.host); + const entry = sheets.open(vi.fn()); + + browser.host.history.pushState({ __NA: true }, "", "/settings/security"); + entry.release(); + browser.settle(); + + expect(browser.entries.map((e) => e.href)).toEqual([ + "https://app.test/settings", + "https://app.test/settings/security", + ]); + expect(browser.index).toBe(1); + }); + + it("Back on a sheet that stays open (unsaved input) gives it its entry back", () => { + const browser = fakeBrowser(); + const sheets = createSheetHistory(browser.host); + let restore = () => {}; + const entry = sheets.open(() => restore()); + restore = entry.restore; + + browser.pressBack(); + expect(browser.index).toBe(1); + expect(marked(browser.host.history.state)).toBe(true); + + // Once it does close, its entry goes and one Back leaves the page. + entry.release(); + browser.settle(); + expect(browser.index).toBe(0); + }); + + it("Back closes only the top sheet of two", () => { + const browser = fakeBrowser(); + const sheets = createSheetHistory(browser.host); + const lower = vi.fn(); + const upper = vi.fn(); + sheets.open(lower); + sheets.open(upper); + + browser.pressBack(); + + expect(upper).toHaveBeenCalledTimes(1); + expect(lower).not.toHaveBeenCalled(); + expect(browser.index).toBe(1); + expect(marked(browser.host.history.state)).toBe(true); + }); + + it("two sheets closed lower first still leave no entry behind", () => { + const browser = fakeBrowser(); + const sheets = createSheetHistory(browser.host); + const lower = sheets.open(vi.fn()); + const upper = sheets.open(vi.fn()); + + lower.release(); + upper.release(); + browser.settle(); + + expect(browser.index).toBe(0); + expect(marked(browser.host.history.state)).toBe(false); + }); + + it("a URL the page replaced while the sheet was open is the one it keeps", () => { + // `?add=` opens the sheet on the first paint and is stripped after it. + const browser = fakeBrowser("https://app.test/measurements?add=WEIGHT"); + const sheets = createSheetHistory(browser.host); + const entry = sheets.open(vi.fn()); + browser.host.history.replaceState(null, "", "/measurements"); + // The router rewrites the current entry; the sheet keeps its mark. + expect(marked(browser.host.history.state)).toBe(true); + + entry.release(); + browser.settle(); + + expect(browser.index).toBe(0); + expect(browser.host.location.href).toBe("https://app.test/measurements"); + expect(browser.routerHeard).toEqual([]); + }); + + it("Back on a sheet opened by a one-shot URL does not bring the URL back", () => { + const browser = fakeBrowser("https://app.test/measurements?add=WEIGHT"); + const sheets = createSheetHistory(browser.host); + const onBack = vi.fn(); + sheets.open(onBack); + browser.host.history.replaceState(null, "", "/measurements"); + + browser.pressBack(); + + expect(onBack).toHaveBeenCalledTimes(1); + expect(browser.host.location.href).toBe("https://app.test/measurements"); + expect(browser.routerHeard).toEqual([]); + }); + + it("a Back that jumps past the page is the router's to handle", () => { + const browser = fakeBrowser("https://app.test/"); + browser.host.history.pushState({ __NA: true }, "", "/labs"); + const sheets = createSheetHistory(browser.host); + const onBack = vi.fn(); + sheets.open(onBack); + + browser.go(-2); + + expect(onBack).toHaveBeenCalledTimes(1); + expect(browser.routerHeard).toEqual(["https://app.test/"]); + }); + + it("writes stop waiting when the close's Back never arrives", () => { + const browser = fakeBrowser("https://app.test/medications"); + const sheets = createSheetHistory(browser.host); + const entry = sheets.open(vi.fn()); + + entry.release(); + browser.host.history.pushState({ __NA: true }, "", "/medications/m1"); + expect(browser.host.location.href).toBe("https://app.test/medications"); + + browser.runTimers(); + expect(browser.host.location.href).toBe("https://app.test/medications/m1"); + }); +}); diff --git a/src/components/ui/responsive-sheet.tsx b/src/components/ui/responsive-sheet.tsx index 41f9f5925..779bddc6d 100644 --- a/src/components/ui/responsive-sheet.tsx +++ b/src/components/ui/responsive-sheet.tsx @@ -5,6 +5,10 @@ import * as React from "react"; import { cn } from "@/lib/utils"; import { useIsMobile } from "@/hooks/use-is-mobile"; import { SeededFormDiscardDialog } from "@/components/forms/use-seeded-form-dismissal"; +import { + sheetHistory, + type SheetHistoryEntry, +} from "@/components/ui/sheet-history"; import { Dialog, DialogContent, @@ -98,12 +102,17 @@ export interface ResponsiveSheetProps { onCloseAutoFocus?: (event: Event) => void; /** * Whether the form holds input a dismissal would lose. A backdrop tap, - * Escape, a swipe down and the X then ask before closing; the form's + * Escape, a swipe down, the X and Back then ask before closing; the form's * own Cancel and Save close it as before. A function is read at the moment * of the dismissal and gets this sheet's body: pass * `sheetBodyHasUnsavedInput` for an add form that keeps its state inside. */ dirty?: boolean | ((body: HTMLElement | null) => boolean); + /** + * Back closes the sheet (`sheet-history.ts`). False for a sheet whose open + * state lives in the URL already, which has its own history entry. + */ + historyEntry?: boolean; children: React.ReactNode; } @@ -194,14 +203,23 @@ export function sheetBodyHasUnsavedInput(body: HTMLElement | null): boolean { return false; } -/** How a sheet closes: a dismissal with unsaved input asks first. */ +/** + * How a sheet closes: a dismissal with unsaved input asks first, and Back + * closes the sheet (asking the same way) instead of leaving the page. + */ function useSheetDismissal({ open, onOpenChange, dirty, -}: Pick) { + historyEntry, +}: Pick< + ResponsiveSheetProps, + "open" | "onOpenChange" | "dirty" | "historyEntry" +>) { const bodyRef = React.useRef(null); + const entryRef = React.useRef(null); const [asking, setAsking] = React.useState(false); + const [backs, countBack] = React.useReducer((n: number) => n + 1, 0); if (!open && asking) setAsking(false); const hasUnsavedInput = () => @@ -216,6 +234,33 @@ function useSheetDismissal({ onOpenChange(next); }; + const handleBack = React.useEffectEvent(() => { + if (hasUnsavedInput()) { + entryRef.current?.restore(); + setAsking(true); + return; + } + countBack(); + onOpenChange(false); + }); + + React.useEffect(() => { + if (!open || historyEntry === false) return; + const entry = sheetHistory()?.open(() => handleBack()); + if (!entry) return; + entryRef.current = entry; + return () => { + entry.release(); + if (entryRef.current === entry) entryRef.current = null; + }; + }, [open, historyEntry]); + + // Back was answered and the sheet is still open: its owner kept it (its + // own discard question, a save under way), so Back must keep working. + React.useEffect(() => { + if (backs > 0 && open) entryRef.current?.restore(); + }, [backs, open]); + // A sibling of the sheet, not a child: inside it, a touch on the question // would reach the sheet's drag through the React tree. const discard = @@ -247,6 +292,7 @@ export function ResponsiveSheet({ contentWidth = "md", onCloseAutoFocus, dirty, + historyEntry, children, }: ResponsiveSheetProps) { const isMobile = useIsMobile(); @@ -254,6 +300,7 @@ export function ResponsiveSheet({ open, onOpenChange, dirty, + historyEntry, }); if (isMobile) { diff --git a/src/components/ui/sheet-history.ts b/src/components/ui/sheet-history.ts new file mode 100644 index 000000000..e1c232679 --- /dev/null +++ b/src/components/ui/sheet-history.ts @@ -0,0 +1,316 @@ +"use client"; + +import { URL_STATE_EVENT } from "@/lib/url-state"; + +/** + * Back closes the open sheet instead of leaving the page under it. + * + * Opening a sheet pushes one history entry with the page's own URL, marked + * with the sheet's id. Back (the Android button, the edge swipe of an + * installed app, the browser's button) then lands on the entry below, and + * the sheet hears it as a close it may refuse: one with unsaved input asks + * first and puts its entry back. Closing the sheet any other way takes its + * entry off again, so one Back after a closed sheet still leaves the page. + * + * Four things keep that from fighting the router: + * + * - Landing on the entry right below a sheet (by Back or by the sheet's + * own close) is never shown to Next: nothing changed for the page, and + * the entry below may still hold a URL the page has since replaced (a + * stripped `?add=`), which Next would restore and so reopen the sheet. + * That entry is given the page's current URL and state instead. + * - A history write made while the sheet's own Back is under way waits for + * it. A close followed by `router.push` would otherwise push the new page + * and have the queued Back undo it. + * - A navigation pushed while a sheet's entry is current (a link in the + * sheet) replaces that entry, so Back from the new page returns to the + * page, not to a sheet entry nobody shows any more. + * - A replace of a sheet's entry keeps the sheet's mark: the router + * rewrites the current entry on every update. + * + * A sheet whose open state already lives in the URL (the vault's `?doc=`) + * has its own entry and opts out (`ResponsiveSheet historyEntry={false}`). + */ + +const SHEET_ENTRY = "__healthlogSheet"; + +/** How long a Back of our own may take before writes stop waiting for it. */ +const SHEET_BACK_GRACE_MS = 500; + +type HistoryWrite = ( + data: unknown, + unused: string, + url?: string | URL | null, +) => void; + +export interface SheetHistoryHost { + history: { + readonly state: unknown; + pushState: HistoryWrite; + replaceState: HistoryWrite; + back(): void; + }; + location: { readonly href: string }; + addEventListener( + type: "popstate", + listener: (event: { stopImmediatePropagation(): void }) => void, + capture: boolean, + ): void; + dispatchEvent(event: Event): boolean; + setTimeout(callback: () => void, ms: number): unknown; + clearTimeout(handle: unknown): void; +} + +export interface SheetHistoryEntry { + /** The sheet closed some other way than Back: take its entry off. */ + release(): void; + /** The sheet stayed open after Back (unsaved input): push its entry again. */ + restore(): void; +} + +export interface SheetHistory { + /** + * A sheet opened. `onBack` runs when Back lands below its entry; the + * entry is gone by then, and `restore` puts it back. + */ + open(onBack: () => void): SheetHistoryEntry; +} + +interface Layer { + id: string; + onBack: () => void; + /** The URL of the entry the sheet was opened over. */ + baseHref: string; + /** Its own entry as last written, which the page entry takes on close. */ + snapshot: { state: unknown; href: string } | null; +} + +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function markOf(state: unknown): string | undefined { + if (!isRecord(state)) return undefined; + const id = state[SHEET_ENTRY]; + return typeof id === "string" ? id : undefined; +} + +function withMark(state: unknown, id: string | undefined): unknown { + const rest = isRecord(state) ? { ...state } : {}; + delete rest[SHEET_ENTRY]; + if (id !== undefined) rest[SHEET_ENTRY] = id; + return rest; +} + +function samePage(a: string, b: string): boolean { + const left = new URL(a); + const right = new URL(b, a); + return left.pathname === right.pathname && left.search === right.search; +} + +let sequence = 0; + +export function createSheetHistory(host: SheetHistoryHost): SheetHistory { + const { history, location } = host; + const prefix = Math.random().toString(36).slice(2, 8); + const layers: Layer[] = []; + /** Backs we asked for and whose popstate has not arrived yet. */ + let ownBacks = 0; + /** What the entry we land on takes once our Backs have arrived. */ + let landing: { state: unknown; href: string } | null = null; + /** Writes held back while our Back is under way. */ + let held: Array<() => void> = []; + let grace: unknown = null; + /** Set around our own writes, which pass the wrappers untouched. */ + let ours = false; + let installed = false; + let push: HistoryWrite = () => undefined; + let replace: HistoryWrite = () => undefined; + + const layerOf = (id: string | undefined) => + id === undefined ? undefined : layers.find((layer) => layer.id === id); + + function write(kind: "push" | "replace", data: unknown, url?: string) { + ours = true; + try { + // Through whatever wraps `history` (Next copies its own state in). + if (kind === "push") history.pushState(data, "", url); + else history.replaceState(data, "", url); + } finally { + ours = false; + } + } + + function snapshot() { + const layer = layerOf(markOf(history.state)); + if (layer) layer.snapshot = { state: history.state, href: location.href }; + } + + function install() { + if (installed) return; + installed = true; + const below = { + push: history.pushState, + replace: history.replaceState, + }; + push = (data, unused, url) => { + if (ours) return below.push.call(history, data, unused, url); + if (ownBacks > 0) { + held.push(() => push.call(history, data, unused, url)); + return; + } + const current = layerOf(markOf(history.state)); + if (current && url != null && !samePage(location.href, String(url))) { + below.replace.call(history, data, unused, url); + return; + } + below.push.call(history, data, unused, url); + }; + replace = (data, unused, url) => { + if (ours) return below.replace.call(history, data, unused, url); + if (ownBacks > 0) { + held.push(() => replace.call(history, data, unused, url)); + return; + } + const id = markOf(history.state); + const keep = layerOf(id) && markOf(data) === undefined; + below.replace.call( + history, + keep ? withMark(data, id) : data, + unused, + url, + ); + snapshot(); + }; + history.pushState = push; + history.replaceState = replace; + host.addEventListener("popstate", onPopState, true); + } + + function armGrace() { + if (grace !== null) host.clearTimeout(grace); + grace = host.setTimeout(() => { + // The Back never arrived (a traversal can be dropped while another + // settles). Stop holding writes; where we stand is unknown, so the + // landing is left alone. + grace = null; + ownBacks = 0; + landing = null; + flush(); + }, SHEET_BACK_GRACE_MS); + } + + function goBack() { + ownBacks += 1; + history.back(); + armGrace(); + } + + function land(target: { state: unknown; href: string }) { + write( + "replace", + withMark(target.state, markOf(history.state)), + target.href, + ); + snapshot(); + } + + function flush() { + if (grace !== null) { + host.clearTimeout(grace); + grace = null; + } + const writes = held; + held = []; + for (const run of writes) run(); + // `useUrlSearch` readers were told about a write that only lands now. + if (writes.length > 0) host.dispatchEvent(new Event(URL_STATE_EVENT)); + } + + function onPopState(event: { stopImmediatePropagation(): void }) { + if (ownBacks > 0) { + ownBacks -= 1; + event.stopImmediatePropagation(); + if (ownBacks > 0) return; + const id = markOf(history.state); + if (id !== undefined && !layerOf(id)) { + // The entry of a sheet that closed while another stood over it. + goBack(); + return; + } + if (landing) land(landing); + landing = null; + flush(); + return; + } + + const id = markOf(history.state); + const kept = layerOf(id); + const closing = layers.splice(kept ? layers.indexOf(kept) + 1 : 0); + if (closing.length === 0) return; + const top = closing[closing.length - 1]; + if (location.href === closing[0].baseHref) { + // One step below the sheet, on the page it was opened over. + event.stopImmediatePropagation(); + if (top.snapshot) land(top.snapshot); + } + for (const layer of closing.reverse()) layer.onBack(); + } + + function pushEntry(layer: Layer) { + if (ownBacks > 0) { + held.push(() => pushEntry(layer)); + return; + } + if (!layers.includes(layer)) return; + layer.baseHref = location.href; + write("push", withMark(null, layer.id)); + snapshot(); + } + + return { + open(onBack) { + install(); + sequence += 1; + const layer: Layer = { + id: `${prefix}-${sequence}`, + onBack, + baseHref: location.href, + snapshot: null, + }; + layers.push(layer); + pushEntry(layer); + return { + release() { + const index = layers.indexOf(layer); + if (index === -1) return; + layers.splice(index, 1); + // Below another sheet's entry, or taken by a navigation: a later + // Back of ours steps over it, or there is nothing to take off. + if (markOf(history.state) !== layer.id) return; + if (ownBacks === 0) { + landing = layer.snapshot ?? { + state: history.state, + href: location.href, + }; + } + goBack(); + }, + restore() { + if (layers.includes(layer)) return; + layers.push(layer); + pushEntry(layer); + }, + }; + }, + }; +} + +let shared: SheetHistory | null = null; + +/** The one instance for the page, or null outside a browser. */ +export function sheetHistory(): SheetHistory | null { + if (typeof window === "undefined") return null; + shared ??= createSheetHistory(window as unknown as SheetHistoryHost); + return shared; +} From 2a60f21a8b14c12cab9b54075289462649ef1407 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 11 Nov 2026 21:39:27 +0100 Subject: [PATCH 3/9] fix(ui): open hint content on a tap instead of a hover Seven places kept information in a hover tooltip, which never opens on a phone: the measurement-diversity lightbulb on the metric pages, the sleep-source warning on the dashboard tile, the score coverage breakdown, the target status pill, the range bar's marker, the colour hint of the dashboard averages and the full token and user names in the admin token list. The lightbulb showed a button that did nothing, the warning a sign nobody could read. They open on a tap now: InfoPopover for the two icon buttons, a Popover on the meter, pill, marker, averages and cells. Where the trigger sits in a link (a dashboard tile, a lab row) its tap opens the popover instead of the page. --- .../baselines/source-function-lines.json | 2 +- .../__tests__/api-token-truncate.test.tsx | 14 +- .../admin/api-token-overview-section.tsx | 44 ++--- src/components/charts/trend-card.tsx | 184 ++++++++++-------- .../insights/derived/coverage-meter.tsx | 170 ++++++++-------- .../insights/measurement-diversity-nudge.tsx | 40 ++-- .../sleep-source-discrepancy-marker.tsx | 63 +++--- .../targets/__tests__/range-bar.test.tsx | 9 +- .../__tests__/target-status-pill.test.tsx | 7 +- src/components/targets/range-bar.tsx | 53 ++--- src/components/targets/target-status-pill.tsx | 63 +++--- 11 files changed, 323 insertions(+), 326 deletions(-) diff --git a/src/__tests__/baselines/source-function-lines.json b/src/__tests__/baselines/source-function-lines.json index 512931154..e033dd14a 100644 --- a/src/__tests__/baselines/source-function-lines.json +++ b/src/__tests__/baselines/source-function-lines.json @@ -70,7 +70,7 @@ "components/charts/medication-compliance-chart.tsx :: MedicationComplianceChart": 543, "components/charts/mood-chart.tsx :: MoodChart": 896, "components/charts/mood-heatmap.tsx :: MoodHeatmap": 296, - "components/charts/trend-card.tsx :: TrendCard": 365, + "components/charts/trend-card.tsx :: TrendCard": 351, "components/clinician/clinician-view.tsx :: ClinicianView": 223, "components/command-palette/command-palette.tsx :: PaletteBody": 333, "components/custom-metrics/custom-metric-chart.tsx :: CustomMetricChart": 231, diff --git a/src/components/admin/__tests__/api-token-truncate.test.tsx b/src/components/admin/__tests__/api-token-truncate.test.tsx index aab52dfa4..403d45c3b 100644 --- a/src/components/admin/__tests__/api-token-truncate.test.tsx +++ b/src/components/admin/__tests__/api-token-truncate.test.tsx @@ -16,7 +16,7 @@ import { renderToStaticMarkup } from "react-dom/server"; * * The fix is aggressive: every cell that could carry a long string * gets `truncate` (overflow-hidden + text-ellipsis + whitespace-nowrap) - * AND a tooltip on hover/long-press so the full value is still + * AND a popover on a tap so the full value is still * discoverable. This is the same pattern UserManagementSection uses * — apply it consistently here. */ @@ -83,7 +83,7 @@ function render() { ); } -describe("ApiTokenOverviewSection — truncate + tooltip (4th attempt)", () => { +describe("ApiTokenOverviewSection — truncate + popover", () => { it("mobile token-name span does NOT carry the contradictory `break-all` modifier", () => { // `truncate` already sets `white-space: nowrap`, which beats // `word-break: break-all` per CSS spec — `break-all` was dead @@ -125,11 +125,11 @@ describe("ApiTokenOverviewSection — truncate + tooltip (4th attempt)", () => { ); }); - it("renders shadcn tooltip wrappers around truncated cells so the full value stays discoverable", () => { + it("makes each truncated cell a button that opens the full value on a tap", () => { const html = render(); - // Tooltip primitive uses `data-slot="tooltip-content"` for the - // hover bubble. Verify at least one tooltip-content node was - // rendered next to a truncated cell. - expect(html).toContain('data-slot="tooltip-trigger"'); + // A hover tooltip never opens on a phone; the popover trigger does. + expect(html).toMatch( + /
- - {/* v1.4.29 — sub-row pair clips rather than wraps at ` - - {t(avg7LabelKey)}:{" "} - {avg7Hint ? ( - - - - {avg7 !== null ? renderPair(avg7, secondary?.avg7) : "—"} - - - -
{avg7Hint}
- - - ) : ( - - {avg7 !== null ? renderPair(avg7, secondary?.avg7) : "—"} - - )} - {trend7Delta != null && ( - - ({formatDelta(trend7Delta)}) - - )} - - - {t("charts.avg30dShort")}:{" "} - {avg30Hint ? ( - - - - {avg30 !== null ? renderPair(avg30, secondary?.avg30) : "—"} - - - -
{avg30Hint}
- - - ) : ( - - {avg30 !== null ? renderPair(avg30, secondary?.avg30) : "—"} - - )} - - {/* v1.4.28 — the all-time third sub-value retired +
+ + {t(avg7LabelKey)}:{" "} + {avg7Hint ? ( + + {avg7 !== null ? renderPair(avg7, secondary?.avg7) : "—"} + + ) : ( + + {avg7 !== null ? renderPair(avg7, secondary?.avg7) : "—"} + + )} + {trend7Delta != null && ( + + ({formatDelta(trend7Delta)}) + + )} + + + {t("charts.avg30dShort")}:{" "} + {avg30Hint ? ( + + {avg30 !== null ? renderPair(avg30, secondary?.avg30) : "—"} + + ) : ( + + {avg30 !== null ? renderPair(avg30, secondary?.avg30) : "—"} + + )} + + {/* v1.4.28 — the all-time third sub-value retired alongside the `avgAllTime*` props. The BD-Zielbereich tile was the only consumer; the same number stays visible on the Insights blood-pressure target panel with full context. Every dashboard tile now ships the same two sub-rows. */} -
- +
); } diff --git a/src/components/insights/derived/coverage-meter.tsx b/src/components/insights/derived/coverage-meter.tsx index 46730ce0c..f4f688dbd 100644 --- a/src/components/insights/derived/coverage-meter.tsx +++ b/src/components/insights/derived/coverage-meter.tsx @@ -1,12 +1,10 @@ "use client"; -import { useId } from "react"; import { - Tooltip, - TooltipContent, - TooltipProvider, - TooltipTrigger, -} from "@/components/ui/tooltip"; + Popover, + PopoverContent, + PopoverTrigger, +} from "@/components/ui/popover"; import { useTranslations } from "@/lib/i18n/context"; import { cn } from "@/lib/utils"; import type { @@ -20,7 +18,7 @@ import type { * * `●●●●○` filled/hollow dots driven by the `presentInputs / requiredInputs` * ratio off `Derived.coverage`, tinted by the `confidence.band`, with an - * accessible breakdown tooltip (present vs required inputs, history days, + * accessible breakdown on a tap (present vs required inputs, history days, * and the named missing inputs). This is the one place the whole app speaks * data-confidence — every score tile, anatomy view, correlation card, * briefing and report header renders it identically. @@ -29,7 +27,8 @@ import type { * shown WITH its confidence, never silently downgraded and never blank. The * dots read as intentional ("4 of 5 inputs · 80%"), not apologetic. * - * CSS-only dots + a Radix `Tooltip` for the breakdown — 0 KB runtime. + * CSS-only dots + a `Popover` for the breakdown: it opens on a tap, which a + * hover tooltip never does on a phone. */ export interface CoverageMeterProps { @@ -46,7 +45,7 @@ export interface CoverageMeterProps { size?: "sm" | "md"; /** * Replaces the bare `{present}/{required}` fraction, in the visible - * label, the `aria-label` and the tooltip heading alike. + * label, the `aria-label` and the breakdown heading alike. * * The default fraction reads as a proportion of the reader's own data, * and for one caller it is not: the Health Score's composite counts @@ -94,7 +93,6 @@ export function CoverageMeter({ className, }: CoverageMeterProps) { const { t } = useTranslations(); - const tooltipId = useId(); const required = clampInt(coverage.requiredInputs, 0, 999); const present = clampInt(coverage.presentInputs, 0, required || 999); @@ -128,90 +126,86 @@ export function CoverageMeter({ }); return ( - - - - - - -
-

{summaryLabel}

-

- {t("insights.derived.coverage.historyDays", { - count: clampInt(coverage.historyDays, 0, 99999), + {summaryLabel} + + + +

+

{summaryLabel}

+

+ {t("insights.derived.coverage.historyDays", { + count: clampInt(coverage.historyDays, 0, 99999), + })} +

+ {coverage.missing.length > 0 && ( +

+ {t("insights.derived.coverage.missing", { + list: coverage.missing.join(", "), })}

- {coverage.missing.length > 0 && ( -

- {t("insights.derived.coverage.missing", { - list: coverage.missing.join(", "), - })} -

- )} -
- - - + )} +
+ + ); } diff --git a/src/components/insights/measurement-diversity-nudge.tsx b/src/components/insights/measurement-diversity-nudge.tsx index ad4d1e775..8663287c4 100644 --- a/src/components/insights/measurement-diversity-nudge.tsx +++ b/src/components/insights/measurement-diversity-nudge.tsx @@ -3,12 +3,7 @@ import { useQuery } from "@tanstack/react-query"; import { Lightbulb } from "lucide-react"; -import { - Tooltip, - TooltipContent, - TooltipProvider, - TooltipTrigger, -} from "@/components/ui/tooltip"; +import { InfoPopover } from "@/components/ui/info-popover"; import { useAuth } from "@/hooks/use-auth"; import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; @@ -29,7 +24,7 @@ import { detectMeasurementDiversity } from "@/lib/insights/measurement-diversity * inline `role="note"` block confused on the category pages (it read * like a data finding rather than a tip), so the nudge now renders as a * small `Lightbulb` glyph beside the page title. The hint text the block - * used to show inline lives in a hover / focus Tooltip; the glyph mounts + * used to show inline opens on a tap (`InfoPopover`); the glyph mounts * only when the spread is actually lopsided, so a healthy metric shows * no extra affordance at all. * @@ -95,26 +90,15 @@ export function MeasurementDiversityNudge({ const triggerLabel = t("insights.subPage.diversity.trigger"); return ( - - - - - - - {message} - - - + ); } diff --git a/src/components/insights/sleep-source-discrepancy-marker.tsx b/src/components/insights/sleep-source-discrepancy-marker.tsx index 8c972f80f..bac82012a 100644 --- a/src/components/insights/sleep-source-discrepancy-marker.tsx +++ b/src/components/insights/sleep-source-discrepancy-marker.tsx @@ -2,12 +2,7 @@ import { TriangleAlert } from "lucide-react"; -import { - Tooltip, - TooltipContent, - TooltipProvider, - TooltipTrigger, -} from "@/components/ui/tooltip"; +import { InfoPopover } from "@/components/ui/info-popover"; import { useTranslations } from "@/lib/i18n/context"; import { formatDurationMinutes } from "@/lib/i18n/duration"; import { MEASUREMENT_SOURCE_SETTINGS_LABEL_KEYS } from "@/lib/i18n/source-labels"; @@ -17,7 +12,7 @@ import { MEASUREMENT_SOURCE_SETTINGS_LABEL_KEYS } from "@/lib/i18n/source-labels * `sleep-hypnogram.tsx` (which introduced it in v1.28.21) so the dashboard * sleep tile and the hypnogram render the identical hint off the identical * wire shape. Observational only: the headline number always stays the - * winning writer's total; the tooltip lists each writer's own total so a + * winning writer's total; the popover lists each writer's own total so a * multi-tracker user can see WHY the night reads surprising. */ @@ -35,7 +30,7 @@ export interface SleepSourceDiscrepancyDto { * Localised device-type label keys (`Measurement.deviceType`), reused from * the source-priority settings surface. Appended in parentheses so two * writer apps behind the same source (watch vs phone under Apple Health) - * stay distinguishable in the discrepancy tooltip. + * stay distinguishable in the discrepancy popover. */ const DEVICE_TYPE_LABEL_KEYS: Record = { watch: "settings.sections.sources.deviceLabels.watch", @@ -64,7 +59,7 @@ export function sourceDisplayName( export interface SleepSourceDiscrepancyMarkerProps { discrepancy: SleepSourceDiscrepancyDto | null | undefined; - /** Tooltip alignment relative to the trigger (default `"start"`). */ + /** Popover alignment relative to the trigger (default `"start"`). */ align?: "start" | "center" | "end"; } @@ -82,36 +77,36 @@ export function SleepSourceDiscrepancyMarker({ const { t } = useTranslations(); if (!discrepancy) return null; return ( - - - - - - -

- {t("insights.sleep.sourceDiscrepancy.tooltipTitle")} -

-

+ // On the dashboard the marker sits in the tile's link: its tap opens the + // popover, not the tile. + { + event.preventDefault(); + event.stopPropagation(); + }} + > + + + {t("insights.sleep.sourceDiscrepancy.tooltipTitle")} + {discrepancy.sources .map( (b) => `${sourceDisplayName(b.source, b.deviceType, t)} ${formatDurationMinutes(b.asleepMinutes, t)}`, ) .join(" · ")} -

- - - + + } + label={t("insights.sleep.sourceDiscrepancy.tooltipTitle")} + icon={TriangleAlert} + className="-mx-3" + align={align} + triggerDataSlot="sleep-source-discrepancy" + bodyDataSlot="sleep-source-discrepancy-body" + /> + ); } diff --git a/src/components/targets/__tests__/range-bar.test.tsx b/src/components/targets/__tests__/range-bar.test.tsx index bb44a7a84..e3edc111c 100644 --- a/src/components/targets/__tests__/range-bar.test.tsx +++ b/src/components/targets/__tests__/range-bar.test.tsx @@ -85,16 +85,17 @@ describe("", () => { expect(marker).toBeLessThan(100 - EDGE_PADDING_PERCENT); }); - it("makes the marker focusable with a text alternative for the value + range (2026-07-17 a11y audit M1)", () => { + it("makes the marker a button named with the value + range, so a tap or a key opens its numbers", () => { const html = render({ value: 72, min: 60, max: 100, unit: "bpm", }); - expect(html).toContain('tabindex="0"'); - expect(html).toContain('role="img"'); - expect(html).toContain('aria-label="Current: 72 bpm'); + expect(html).toMatch( + /
- - - - {statusLabel} - - - - {range && ( -

- {t("targets.targetRangeValue", { - min: String(range.min), - max: String(range.max), - unit, - })} -

- )} -

{t("targets.sourceLabel", { source })}

- - - + + + + {statusLabel} + + + + {range && ( +

+ {t("targets.targetRangeValue", { + min: String(range.min), + max: String(range.max), + unit, + })} +

+ )} +

{t("targets.sourceLabel", { source })}

+ + ); } From 3a249bccbac18cbfc9ff852ed80e41091e6d331a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 11 Nov 2026 21:43:10 +0100 Subject: [PATCH 4/9] fix(settings): keep note and paste fields at 16 px on a phone On /settings/anamnesis the four note fields were raw textareas styled by three copies of a class constant with text-sm (14 px), and on /settings/export the three paste fields used text-xs (12 px) and the backup passphrase text-sm. iOS zooms the page into such a field on focus and leaves it zoomed. The notes use the Textarea primitive now and the export fields keep the small size only under a fine pointer. The 16 px guard read the five field primitives and could not see a call site override their size, or a raw field styled from a constant. It now parses every field call site; four that are still small sit on a list that only shrinks. The focus guard drops the three records files, whose fields come from the primitive now. --- src/__tests__/pwa-mobile-shell-guard.test.ts | 182 +++++++++++++++++- src/__tests__/text-field-focus-guard.test.ts | 3 - .../records/about-me-note-manager.tsx | 10 +- .../records/allergy-free-text-note.tsx | 9 +- src/components/records/conditions-manager.tsx | 9 +- src/components/settings/export-section.tsx | 2 +- .../settings/import-panel/csv-import-card.tsx | 2 +- .../import-panel/dose-history-import-card.tsx | 2 +- .../import-panel/json-import-card.tsx | 2 +- 9 files changed, 193 insertions(+), 28 deletions(-) diff --git a/src/__tests__/pwa-mobile-shell-guard.test.ts b/src/__tests__/pwa-mobile-shell-guard.test.ts index 74da13d74..ad045d3d8 100644 --- a/src/__tests__/pwa-mobile-shell-guard.test.ts +++ b/src/__tests__/pwa-mobile-shell-guard.test.ts @@ -9,17 +9,21 @@ * 5 px content box and pushed the logo across its bottom border). * 2. The stylesheet carries the rules the shell and the keyboard bridge * depend on. - * 3. Every form-field primitive keeps 16 px type on a touch screen: iOS - * Safari zooms the page into a focused field below 16 px, and a width + * 3. Every form field keeps 16 px type on a touch screen: iOS Safari + * zooms the page into a focused field below 16 px, and a width * breakpoint alone (`md:text-sm`) handed 14 px to a phone held sideways * and to every iPad. The small size is allowed only behind - * `pointer-fine:`. + * `pointer-fine:`. That holds for the primitives and for every call + * site, whose `text-sm` replaces the primitive's size, and for a raw + * `