diff --git a/components/calendar/calendar-app.tsx b/components/calendar/calendar-app.tsx index f45e6a2af..db516779c 100644 --- a/components/calendar/calendar-app.tsx +++ b/components/calendar/calendar-app.tsx @@ -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), }); diff --git a/components/contacts/contacts-app.tsx b/components/contacts/contacts-app.tsx index 7fccc9654..6af969db5 100644 --- a/components/contacts/contacts-app.tsx +++ b/components/contacts/contacts-app.tsx @@ -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(); diff --git a/components/files/__tests__/file-preview-modal-history.test.tsx b/components/files/__tests__/file-preview-modal-history.test.tsx index a9120ab91..b29dcbbfb 100644 --- a/components/files/__tests__/file-preview-modal-history.test.tsx +++ b/components/files/__tests__/file-preview-modal-history.test.tsx @@ -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(); diff --git a/components/files/file-preview-modal.tsx b/components/files/file-preview-modal.tsx index e72f60ab6..37c7dbfd4 100644 --- a/components/files/file-preview-modal.tsx +++ b/components/files/file-preview-modal.tsx @@ -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; diff --git a/components/files/image-preview-modal.tsx b/components/files/image-preview-modal.tsx index ea07b436b..4a58a5afe 100644 --- a/components/files/image-preview-modal.tsx +++ b/components/files/image-preview-modal.tsx @@ -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; diff --git a/components/files/wopi-editor.tsx b/components/files/wopi-editor.tsx index a95effec8..bed4b8d42 100644 --- a/components/files/wopi-editor.tsx +++ b/components/files/wopi-editor.tsx @@ -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(null); const [error, setError] = useState(null); diff --git a/hooks/__tests__/use-calendar-close-on-back.test.tsx b/hooks/__tests__/use-close-on-back.test.tsx similarity index 56% rename from hooks/__tests__/use-calendar-close-on-back.test.tsx rename to hooks/__tests__/use-close-on-back.test.tsx index ef7ff6aba..35909565c 100644 --- a/hooks/__tests__/use-calendar-close-on-back.test.tsx +++ b/hooks/__tests__/use-close-on-back.test.tsx @@ -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 | null)?.__closeOnBack; -async function openOverlay(options: Partial[0]> = {}) { +async function openOverlay(options: Partial[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 }) => {children} : 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(); + }); }); diff --git a/hooks/use-calendar-close-on-back.ts b/hooks/use-calendar-close-on-back.ts deleted file mode 100644 index 9ac8bf1e7..000000000 --- a/hooks/use-calendar-close-on-back.ts +++ /dev/null @@ -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 | 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 | 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 | 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 | null)?.[STATE_KEY] !== entryId) return; - const url = window.location.href; - pendingBack = new Promise((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]); -} diff --git a/hooks/use-close-on-back.ts b/hooks/use-close-on-back.ts index 85c91e084..ba8079478 100644 --- a/hooks/use-close-on-back.ts +++ b/hooks/use-close-on-back.ts @@ -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 | null = null; +const currentEntry = () => (window.history.state as Record | 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 | 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((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(() => ({ rearm: () => rearmRef.current(), armed: () => armedRef.current })); return api; }