fix: let Back close overlays the same way in calendar, contacts and files

This commit is contained in:
Linus Rath
2026-10-28 07:32:05 +01:00
parent 731ec15af4
commit 4b2aba430c
9 changed files with 101 additions and 123 deletions
+2 -3
View File
@@ -86,7 +86,7 @@ import type { ParsedWebcal } from "@/lib/protocol-handlers/webcal";
import { appPath, buildCalendarPath, parseCalendarPath, type CalendarDeepLink } from "@/lib/deep-links";
import { consumePendingDeepLinkEntry, subscribePendingDeepLink } from "@/lib/deep-link-handoff";
import { useDeepLinkUrl } from "@/hooks/use-deep-link-url";
import { useCloseOnBack } from "@/hooks/use-calendar-close-on-back";
import { useCloseOnBack } from "@/hooks/use-close-on-back";
import { useCalendarEventSearch } from "@/hooks/use-calendar-event-search";
import { CalendarSearchField, CalendarSearchResults } from "@/components/calendar/calendar-search";
import { useProInterfaceActive } from "@/components/pro/pro-interface-redirect";
@@ -962,8 +962,7 @@ export function CalendarApp({ linkSegments: routeSegments }: CalendarAppProps =
setEditTask(null);
}, []);
useCloseOnBack({
open: showEventModal || showTaskModal,
enabled: isEmbedded ? isFocusedProTab : !proInterfaceActive,
active: (showEventModal || showTaskModal) && (isEmbedded ? isFocusedProTab : !proInterfaceActive),
shouldHold: () => eventEditorDirtyRef.current || taskEditorDirtyRef.current,
onBack: () => whenEditorsDiscarded(closeEditors),
});
+2 -2
View File
@@ -202,7 +202,7 @@ function ContactsAppContent({ linkSegments: routeSegments }: ContactsAppProps =
// Set when the app itself goes Back (to the email it came from): the
// guard's history entry is in the way then, and is stepped over.
const leavingRef = useRef(false);
const backGuard = useCloseOnBack(() => {
const backGuard = useCloseOnBack({ active: editorDirty, onBack: () => {
if (leavingRef.current) {
leavingRef.current = false;
window.history.back();
@@ -212,7 +212,7 @@ function ContactsAppContent({ linkSegments: routeSegments }: ContactsAppProps =
if (discard) handleCancelRef.current();
else backGuard.rearm();
});
}, editorDirty);
} });
const routerBack = useCallback(() => {
if (backGuard.armed()) leavingRef.current = true;
router.back();
@@ -88,7 +88,7 @@ describe('FilePreviewModal history lifecycle (modeled queued traversal)', () =>
const { onClose } = openPreview();
await advance(0);
expect(window.history.state).toMatchObject(originalState);
expect(window.history.state.__filePreview).toBeTruthy();
expect(window.history.state.__closeOnBack).toBeTruthy();
clickClose();
expect(onClose).toHaveBeenCalledOnce();
expect(screen.queryByRole('dialog')).toBeNull();
+1 -1
View File
@@ -183,7 +183,7 @@ export function FilePreviewModal({ name, onClose, onDownload, getFileContent, fi
}, []);
// Back closes the preview instead of leaving Files.
useCloseOnBack(onClose);
useCloseOnBack({ onBack: onClose });
const fileType = resolvedFileType;
+1 -1
View File
@@ -27,7 +27,7 @@ export function ImagePreviewModal({ name, onClose, onDownload, getImageUrl, file
const [error, setError] = useState(false);
// Back closes the preview instead of leaving Files, as for every other preview.
useCloseOnBack(onClose);
useCloseOnBack({ onBack: onClose });
useEffect(() => {
let revoke: string | null = null;
+1 -1
View File
@@ -41,7 +41,7 @@ interface LaunchData {
*/
export function WopiEditor({ target, accountId, slot, onClose, closeOnBack = false }: WopiEditorProps) {
const t = useTranslations("files");
useCloseOnBack(onClose, closeOnBack);
useCloseOnBack({ onBack: onClose, active: closeOnBack });
const locale = useLocale();
const [launch, setLaunch] = useState<LaunchData | null>(null);
const [error, setError] = useState<string | null>(null);
@@ -1,14 +1,16 @@
import { StrictMode, type ReactNode } from 'react';
import { renderHook, waitFor } from '@testing-library/react';
import { describe, it, expect, vi } from 'vitest';
import { useCloseOnBack } from '../use-calendar-close-on-back';
import { useCloseOnBack } from '../use-close-on-back';
const entry = () => (window.history.state as Record<string, unknown> | null)?.__closeOnBack;
async function openOverlay(options: Partial<Parameters<typeof useCloseOnBack>[0]> = {}) {
async function openOverlay(options: Partial<Parameters<typeof useCloseOnBack>[0]> = {}, strict = false) {
const before = entry();
const onBack = vi.fn();
const view = renderHook((props: { open: boolean }) => useCloseOnBack({ onBack, ...options, open: props.open }), {
initialProps: { open: true },
const view = renderHook((props: { active: boolean }) => useCloseOnBack({ onBack, ...options, active: props.active }), {
initialProps: { active: true },
wrapper: strict ? ({ children }: { children: ReactNode }) => <StrictMode>{children}</StrictMode> : undefined,
});
await waitFor(() => expect(entry()).toBeTruthy());
await waitFor(() => expect(entry()).not.toBe(before));
@@ -37,7 +39,7 @@ describe('useCloseOnBack (R5)', () => {
const { rerender } = await openOverlay();
// The page rewrites the URL while the overlay is open
window.history.replaceState(window.history.state, '', '/calendar/week/2026-09-14');
rerender({ open: false });
rerender({ active: false });
await waitFor(() => expect(entry()).toBeUndefined());
await waitFor(() => expect(window.location.pathname).toBe('/calendar/week/2026-09-14'));
});
@@ -51,4 +53,32 @@ describe('useCloseOnBack (R5)', () => {
expect(window.history.state).toEqual({ other: true });
expect(window.location.pathname).toBe('/mail');
});
it('holds a new entry on rearm after onBack chose to stay open, and says whether it is armed', async () => {
const { result, onBack, unmount } = await openOverlay();
expect(result.current.armed()).toBe(true);
window.history.back();
await waitFor(() => expect(onBack).toHaveBeenCalledTimes(1));
expect(result.current.armed()).toBe(false);
expect(entry()).toBeUndefined();
result.current.rearm();
expect(result.current.armed()).toBe(true);
expect(entry()).toBeTruthy();
// ...and the next Back reaches onBack again
window.history.back();
await waitFor(() => expect(onBack).toHaveBeenCalledTimes(2));
unmount();
});
it('pushes one entry and no Back under Strict Mode', async () => {
const push = vi.spyOn(window.history, 'pushState');
const back = vi.spyOn(window.history, 'back');
const { unmount } = await openOverlay({}, true);
expect(push).toHaveBeenCalledTimes(1);
expect(back).not.toHaveBeenCalled();
unmount();
await waitFor(() => expect(entry()).toBeUndefined());
push.mockRestore();
back.mockRestore();
});
});
-85
View File
@@ -1,85 +0,0 @@
"use client";
import { useEffect, useRef } from "react";
import { generateUUID } from "@/lib/utils";
const STATE_KEY = "__closeOnBack";
// Closing one overlay leaves a Back traversal in flight; the next one must
// not push its entry until that has landed, or the Back would pop it.
let pendingBack: Promise<void> | null = null;
interface UseCloseOnBackOptions {
/** Whether the overlay is open: it holds a history entry while it is. */
open: boolean;
/** Called when browser Back (or the phone's back gesture) leaves the entry. */
onBack: () => void;
/**
* Whether Back should stay on the overlay for now, e.g. an editor with
* unsaved changes that asks first: the entry is put back, so a later Back
* asks again, and `onBack` decides whether to close.
*/
shouldHold?: () => boolean;
enabled?: boolean;
}
/**
* Browser Back closes an overlay instead of leaving the page (R5). The
* overlay gets a history entry of its own while it is open; closing it any
* other way takes that entry back off, so no stray steps are left for Back
* to walk through. The URL is left alone: the entry copies the current one,
* Next's router state included, and after the clean-up Back the address bar
* is set to whatever the page showed last.
*/
export function useCloseOnBack({ open, onBack, shouldHold, enabled = true }: UseCloseOnBackOptions) {
const onBackRef = useRef(onBack);
onBackRef.current = onBack;
const shouldHoldRef = useRef(shouldHold);
shouldHoldRef.current = shouldHold;
useEffect(() => {
if (!open || !enabled || typeof window === "undefined") return;
const entryId = generateUUID();
let cancelled = false;
let pushed = false;
const push = () => {
window.history.pushState({ ...window.history.state, [STATE_KEY]: entryId }, "");
pushed = true;
};
const onPop = () => {
if ((window.history.state as Record<string, unknown> | null)?.[STATE_KEY] === entryId) return;
pushed = false;
if (shouldHoldRef.current?.()) push();
onBackRef.current();
};
// Deferred so Strict Mode's setup/cleanup/setup doesn't push and pop.
const timer = window.setTimeout(async () => {
if (pendingBack) await pendingBack;
if (cancelled) return;
push();
window.addEventListener("popstate", onPop);
}, 0);
return () => {
cancelled = true;
window.clearTimeout(timer);
window.removeEventListener("popstate", onPop);
if (!pushed || (window.history.state as Record<string, unknown> | null)?.[STATE_KEY] !== entryId) return;
// After this commit's effects, so the URL the page writes for the
// closed state is the one restored once Back has landed.
queueMicrotask(() => {
// A navigation in the same commit (leaving the calendar through the
// rail) has pushed its own entry by now: that one must not be undone.
if ((window.history.state as Record<string, unknown> | null)?.[STATE_KEY] !== entryId) return;
const url = window.location.href;
pendingBack = new Promise<void>((resolve) => {
window.addEventListener("popstate", () => {
if (window.location.href !== url) window.history.replaceState(window.history.state, "", url);
pendingBack = null;
resolve();
}, { once: true });
});
window.history.back();
});
};
}, [open, enabled]);
}
+58 -24
View File
@@ -3,50 +3,77 @@
import { useEffect, useRef, useState } from "react";
import { generateUUID } from "@/lib/utils";
const STATE_KEY = "__closeOnBack";
// Closing one layer can leave an asynchronous Back traversal in flight. The
// next layer must not install its popstate listener until that completes, or
// the traversal would close it at once.
// next layer must not push its entry until that completes, or the traversal
// would pop it and close the layer at once.
let pendingHistoryBack: Promise<void> | null = null;
const currentEntry = () => (window.history.state as Record<string, unknown> | null)?.[STATE_KEY];
interface UseCloseOnBackOptions {
/** Called when browser Back (or the phone's back gesture) leaves the entry. */
onBack: () => void;
/** Whether the layer is open: it holds a history entry while it is. */
active?: boolean;
/**
* Whether Back should stay on the layer for now, e.g. an editor with
* unsaved changes that asks first. Checked as Back lands: the entry is put
* back at once, so another Back while `onBack` asks is caught too, and
* `onBack` decides whether to close.
*/
shouldHold?: () => boolean;
}
export interface CloseOnBack {
/** Holds an entry again after `onBack` decided to stay open. */
rearm: () => void;
/** Whether the entry is there now, e.g. before a `router.back()` that must leave for real. */
armed: () => boolean;
}
/**
* Lets the browser's Back (and the phone's back gesture) close an overlay
* instead of leaving the page: while `active`, the overlay holds a history
* entry of its own, and Back calls `onBack`. Closed any other way, the entry
* is taken back off, so the history ends where it was.
*
* `onBack` runs after the entry is gone. A layer that decides to stay open
* (an editor that asked "Discard changes?" and heard "Keep editing") calls
* the returned `rearm` to hold an entry again. `armed` tells whether the
* entry is there now, e.g. before a `router.back()` that must leave for real.
* Lets the browser's Back close an overlay, panel or editor instead of
* leaving the page (R5). While `active`, the layer holds a history entry of
* its own and Back calls `onBack`. Closed any other way, the entry is taken
* back off, so no stray steps are left for Back to walk through:
* - not when a navigation in the same commit (leaving through the rail)
* has pushed its own entry by then, which must not be undone;
* - and once the clean-up Back has landed, the address bar shows the URL
* the page wrote for the closed state, not the older one of the entry
* below.
* The entry copies the current state, Next's router state included.
*/
export function useCloseOnBack(onBack: () => void, active = true): { rearm: () => void; armed: () => boolean } {
export function useCloseOnBack({ onBack, active = true, shouldHold }: UseCloseOnBackOptions): CloseOnBack {
const onBackRef = useRef(onBack);
onBackRef.current = onBack;
const shouldHoldRef = useRef(shouldHold);
shouldHoldRef.current = shouldHold;
const rearmRef = useRef<() => void>(() => {});
const armedRef = useRef(false);
useEffect(() => {
if (!active) return;
if (!active || typeof window === "undefined") return;
let cancelled = false;
let entryId: string | null = null;
let poppedByBack = false;
const onPop = () => {
if (!entryId) return;
const onPop = (event: PopStateEvent) => {
// Forward into our own entry is not a Back.
if (!entryId || (event.state as Record<string, unknown> | null)?.[STATE_KEY] === entryId) return;
entryId = null;
armedRef.current = false;
poppedByBack = true;
window.removeEventListener("popstate", onPop);
if (shouldHoldRef.current?.()) push();
onBackRef.current();
};
const push = () => {
function push() {
if (cancelled || entryId) return;
entryId = generateUUID();
armedRef.current = true;
poppedByBack = false;
window.history.pushState({ ...window.history.state, __filePreview: entryId }, "");
window.history.pushState({ ...window.history.state, [STATE_KEY]: entryId }, "");
window.addEventListener("popstate", onPop);
};
}
rearmRef.current = push;
// Deferred, so Strict Mode's setup/cleanup/setup replay can cancel the
@@ -63,19 +90,26 @@ export function useCloseOnBack(onBack: () => void, active = true): { rearm: () =
window.clearTimeout(timer);
window.removeEventListener("popstate", onPop);
const ours = entryId;
if (ours && !poppedByBack && window.history.state?.__filePreview === ours) {
if (!ours || currentEntry() !== ours) return;
// After this commit's effects: by then a navigation in the same commit
// has pushed its entry, and the page has written the URL for the
// closed state.
queueMicrotask(() => {
if (currentEntry() !== ours) return;
const url = window.location.href;
pendingHistoryBack = new Promise<void>((resolve) => {
window.addEventListener("popstate", () => {
if (window.location.href !== url) window.history.replaceState(window.history.state, "", url);
pendingHistoryBack = null;
resolve();
}, { once: true });
});
window.history.back();
}
});
};
}, [active]);
// The same object every render, so callers can list it as a dependency.
const [api] = useState(() => ({ rearm: () => rearmRef.current(), armed: () => armedRef.current }));
const [api] = useState<CloseOnBack>(() => ({ rearm: () => rearmRef.current(), armed: () => armedRef.current }));
return api;
}