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] 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} + /> + )} + ); }