diff --git a/src/components/forms/__tests__/seeded-form-guard-sheet.browser.test.tsx b/src/components/forms/__tests__/seeded-form-guard-sheet.browser.test.tsx new file mode 100644 index 000000000..13a59e524 --- /dev/null +++ b/src/components/forms/__tests__/seeded-form-guard-sheet.browser.test.tsx @@ -0,0 +1,78 @@ +/** + * A form that guards Back (`useSeededFormDismissal({ guardNavigation })`) + * and a sheet opened over it both listen for popstate in the capture phase, + * so the one registered first hears Back first. Here the guard is: the page + * mounts the form, and the sheet history installs at the first sheet open, + * which is the order on `/settings/account` when "All sections" is the first + * sheet of the visit. A Back that closes the sheet is the sheet's, and the + * form must not answer it with its discard question. + */ +import { describe, expect, it, vi } from "vitest"; +import { render } from "vitest-browser-react"; + +import { sheetHistory } from "@/components/ui/sheet-history"; + +import { useSeededFormDismissal } from "../use-seeded-form-dismissal"; + +type Navigation = { currentEntry: { index: number } }; +const index = () => + (window as unknown as { navigation: Navigation }).navigation.currentEntry + .index; + +let popstates = 0; +window.addEventListener("popstate", () => (popstates += 1), true); + +async function settle(count: number): Promise { + const target = popstates + count; + await vi.waitFor(() => expect(popstates).toBeGreaterThanOrEqual(target), { + timeout: 2_000, + }); + for (;;) { + const seen = popstates; + await new Promise((resolve) => setTimeout(resolve, 150)); + if (popstates === seen) return; + } +} + +function EditedForm() { + const { discardDialogOpen } = useSeededFormDismissal({ + seed: "Ada", + value: "Ada L.", + guardNavigation: true, + navigate: () => {}, + }); + return

{discardDialogOpen ? "asking" : "quiet"}

; +} + +describe("a guarded form under a sheet", () => { + // One case: the listener order is set once per page, by the first open. + it("leaves the sheet's Back to the sheet and guards the page's", async () => { + window.history.pushState(null, "", "?section=account"); + const page = index(); + const screen = await render(); + const asking = screen.getByTestId("asking"); + + // Back closes the sheet. + const onBack = vi.fn(); + sheetHistory()!.open(onBack); + expect(index()).toBe(page + 1); + window.history.back(); + await settle(1); + expect(onBack).toHaveBeenCalledTimes(1); + expect(index()).toBe(page); + await expect.element(asking).toHaveTextContent("quiet"); + + // The sheet's own close takes its entry off. + sheetHistory()! + .open(() => {}) + .release(); + await settle(1); + expect(index()).toBe(page); + await expect.element(asking).toHaveTextContent("quiet"); + + // With no sheet open, Back is the form's. + window.history.back(); + await settle(1); + await expect.element(asking).toHaveTextContent("asking"); + }); +}); diff --git a/src/components/forms/use-seeded-form-dismissal.tsx b/src/components/forms/use-seeded-form-dismissal.tsx index 58a3bec84..361476a5b 100644 --- a/src/components/forms/use-seeded-form-dismissal.tsx +++ b/src/components/forms/use-seeded-form-dismissal.tsx @@ -12,6 +12,7 @@ import { AlertDialogHeader, AlertDialogTitle, } from "@/components/ui/alert-dialog"; +import { sheetHistoryOwnsPopstate } from "@/components/ui/sheet-history"; import { useTranslations } from "@/lib/i18n/context"; type CloseRequest = () => void; @@ -347,6 +348,9 @@ export function useSeededFormDismissal({ }); historyTraversalGuardRef.current = historyTraversalGuard; const handlePopState = (event: PopStateEvent) => { + // A Back that closes a sheet over the form is the sheet's to answer; + // whichever capture listener was added first hears it first. + if (sheetHistoryOwnsPopstate()) return; historyTraversalGuard.handlePopState(event); }; diff --git a/src/components/ui/__tests__/sheet-history.browser.test.tsx b/src/components/ui/__tests__/sheet-history.browser.test.tsx new file mode 100644 index 000000000..78ec10989 --- /dev/null +++ b/src/components/ui/__tests__/sheet-history.browser.test.tsx @@ -0,0 +1,100 @@ +/** + * Sheet history against Chromium's own session history: `back()` answers + * with a real popstate a task or more later, and the entries keep their + * state the way the browser keeps it. + * + * The router's half is played by hand where it matters: Next answers a URL + * write one scheduler task later by rewriting the current entry with that + * entry's own state copied in (`HistoryUpdater`), and that write can land + * after a sheet's close has asked for Back but before Back arrives. + */ +import { describe, expect, it, vi } from "vitest"; + +import { DAY_HISTORY_KEY } from "@/components/day/day-url"; + +import { sheetHistory } from "../sheet-history"; + +type Navigation = { currentEntry: { index: number } }; +const index = () => + (window as unknown as { navigation: Navigation }).navigation.currentEntry + .index; +const state = () => (window.history.state ?? {}) as Record; +const MARK = "__healthlogSheet"; + +// Registered before the sheet history installs its own listener at the +// first open, so it hears every popstate, including the ones it swallows. +let popstates = 0; +window.addEventListener("popstate", () => (popstates += 1), true); + +/** Wait for `count` more popstates, then until none follows. */ +async function settle(count: number): Promise { + const target = popstates + count; + await vi.waitFor(() => expect(popstates).toBeGreaterThanOrEqual(target), { + timeout: 2_000, + }); + for (;;) { + const seen = popstates; + await new Promise((resolve) => setTimeout(resolve, 150)); + if (popstates === seen) return; + } +} + +describe("sheet history in the browser", () => { + it("leaves the page entry unmarked when the close also writes the URL", async () => { + const sheets = sheetHistory()!; + window.history.pushState(null, "", "?step=before"); + window.history.pushState(null, "", "?step=page"); + const page = index(); + + // The day's date picker: its handler writes the day into the URL while + // the sheet's entry is current, then the sheet closes. + const picker = sheets.open(() => {}); + expect(index()).toBe(page + 1); + window.history.replaceState(null, "", "?step=page&day=2026-11-10"); + picker.release(); + // The router's answer to that write, copying the entry it finds. + window.history.replaceState( + { ...state(), __NA: true }, + "", + window.location.href, + ); + await settle(1); + + expect(index()).toBe(page); + expect(window.location.search).toBe("?step=page&day=2026-11-10"); + expect(state()[MARK]).toBeUndefined(); + + // The next sheet closes onto the page, not onto the entry before it. + const again = sheets.open(() => {}); + again.release(); + await settle(1); + expect(index()).toBe(page); + expect(window.location.search).toBe("?step=page&day=2026-11-10"); + }); + + it("keeps the page entry's own state through a sheet", async () => { + const sheets = sheetHistory()!; + window.history.pushState( + { [DAY_HISTORY_KEY]: "2026-11-10" }, + "", + "?day=2026-11-10", + ); + const page = index(); + + // Closed by its own button. + sheets.open(() => {}).release(); + await settle(1); + expect(index()).toBe(page); + expect(state()[DAY_HISTORY_KEY]).toBe("2026-11-10"); + + // Closed by Back. + const onBack = vi.fn(); + sheets.open(onBack); + window.history.back(); + await settle(1); + expect(onBack).toHaveBeenCalledTimes(1); + expect(index()).toBe(page); + expect(state()[DAY_HISTORY_KEY]).toBe("2026-11-10"); + expect(state()[MARK]).toBeUndefined(); + }); +}); diff --git a/src/components/ui/sheet-history.ts b/src/components/ui/sheet-history.ts index e1c232679..b7059fd14 100644 --- a/src/components/ui/sheet-history.ts +++ b/src/components/ui/sheet-history.ts @@ -26,7 +26,13 @@ import { URL_STATE_EVENT } from "@/lib/url-state"; * 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. + * rewrites the current entry on every update. A write that carries the + * mark of a sheet that has closed (the router copying an entry it read + * before our Back landed) loses it, or the entry would read as a sheet's + * and the next close would step one Back too far. + * + * A sheet's entry starts as a copy of the page entry's state, so landing + * on the page entry gives it back its own keys (an open day's, the vault's). * * A sheet whose open state already lives in the URL (the vault's `?doc=`) * has its own entry and opts out (`ResponsiveSheet historyEntry={false}`). @@ -74,6 +80,12 @@ export interface SheetHistory { * entry is gone by then, and `restore` puts it back. */ open(onBack: () => void): SheetHistoryEntry; + /** + * Whether the popstate being dispatched is ours: a Back we asked for, or + * one that lands on a sheet's entry or right below the first sheet. Asked + * by other capture listeners, which may hear it before we do. + */ + ownsPopstate(): boolean; } interface Layer { @@ -108,6 +120,14 @@ function samePage(a: string, b: string): boolean { return left.pathname === right.pathname && left.search === right.search; } +/** `data` without the mark of a sheet that is no longer open. */ +function withoutDeadMark(data: unknown, open: readonly Layer[]): unknown { + const id = markOf(data); + return id === undefined || open.some((layer) => layer.id === id) + ? data + : withMark(data, undefined); +} + let sequence = 0; export function createSheetHistory(host: SheetHistoryHost): SheetHistory { @@ -149,16 +169,14 @@ export function createSheetHistory(host: SheetHistoryHost): SheetHistory { function install() { if (installed) return; installed = true; - const below = { - push: history.pushState, - replace: history.replaceState, - }; + 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; } + data = withoutDeadMark(data, layers); const current = layerOf(markOf(history.state)); if (current && url != null && !samePage(location.href, String(url))) { below.replace.call(history, data, unused, url); @@ -172,6 +190,7 @@ export function createSheetHistory(host: SheetHistoryHost): SheetHistory { held.push(() => replace.call(history, data, unused, url)); return; } + data = withoutDeadMark(data, layers); const id = markOf(history.state); const keep = layerOf(id) && markOf(data) === undefined; below.replace.call( @@ -264,11 +283,15 @@ export function createSheetHistory(host: SheetHistoryHost): SheetHistory { } if (!layers.includes(layer)) return; layer.baseHref = location.href; - write("push", withMark(null, layer.id)); + write("push", withMark(history.state, layer.id)); snapshot(); } return { + ownsPopstate: () => + ownBacks > 0 || + layerOf(markOf(history.state)) !== undefined || + (layers.length > 0 && location.href === layers[0].baseHref), open(onBack) { install(); sequence += 1; @@ -314,3 +337,8 @@ export function sheetHistory(): SheetHistory | null { shared ??= createSheetHistory(window as unknown as SheetHistoryHost); return shared; } + +/** Whether the popstate under way belongs to an open or closing sheet. */ +export function sheetHistoryOwnsPopstate(): boolean { + return shared?.ownsPopstate() ?? false; +}