mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-12 22:26:13 +00:00
fix(sheets): keep Back on the page a closed sheet returns to
Three ways the sheet's history entry could still cost a Back: - A close that also writes the URL (the day's date picker, the life event sheet opened by ?add=lifeEvent) let the router's answer to that write land on the page entry with the closed sheet's mark. The next sheet close then stepped one entry past the page, and on an installed app opened from a notification that closed the app. A write that carries the mark of a sheet that is gone now loses it. - A sheet's entry started empty, and landing below it copied that empty state onto the page entry, so an open day or document lost the key it keys its own Back on. The sheet's entry now starts as a copy of the page entry's state. - The account form's Back guard heard the sheet's own Back first when it was mounted before the first sheet of the visit, and asked to discard the form's edits for closing the section list. The guard now leaves a popstate the sheet history owns to it. The browser tests run against Chromium's session history and fail without each fix.
This commit is contained in:
1 parent
3faf1b1c8b
commit
555a529260
4 files changed
+216
-6
No files matched your search
@@ -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<void> {
|
||||
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 <p data-testid="asking">{discardDialogOpen ? "asking" : "quiet"}</p>;
|
||||
}
|
||||
|
||||
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(<EditedForm />);
|
||||
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");
|
||||
});
|
||||
});
|
||||
@@ -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<T>({
|
||||
});
|
||||
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);
|
||||
};
|
||||
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
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<void> {
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
}
|
||||
Reference in new issue
Block a user