diff --git a/frontend/src/lib/history.dom.test.ts b/frontend/src/lib/history.dom.test.ts index 41160aec..9a5ab3af 100644 --- a/frontend/src/lib/history.dom.test.ts +++ b/frontend/src/lib/history.dom.test.ts @@ -177,9 +177,9 @@ describe("closeOverlaysThenNavigate", () => { } }); - // An overlay's Back callback is a Back STEP: the Plex player keeps its panels on one entry and peels - // one per call. Called once, the entry stayed and the navigation was dropped 1.5 s later — `i`, `d`, - // Shift+W in the player closed the info panel and nothing else. + // An overlay's Back callback is a Back STEP, and the contract lets one entry hold several. When the + // Plex player kept its panels on one entry, a single call left the entry up and the navigation was + // dropped 1.5 s later — `i`, `d`, Shift+W in the player closed the info panel and nothing else. it("asks again, a render apart, when one call peels only a step off the overlay", async () => { const back = stubBack(); try { diff --git a/frontend/src/lib/history.subview.dom.test.ts b/frontend/src/lib/history.subview.dom.test.ts index f6a9b98b..f435f2e1 100644 --- a/frontend/src/lib/history.subview.dom.test.ts +++ b/frontend/src/lib/history.subview.dom.test.ts @@ -7,8 +7,8 @@ // an overlay's programmatic history.back()) restores the sub-view from the URL instead of collapsing // it to root. The overlay/back-ladder half of history.ts is covered in history.dom.test.ts. import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -import { renderHook, act } from "@testing-library/react"; -import { useHistorySubview, type SubviewUrl } from "./history"; +import { renderHook, act, waitFor } from "@testing-library/react"; +import { useBackToClose, useHistorySubview, type SubviewUrl } from "./history"; // A minimal string-view mirror: "root" is the un-mirrored root (encode → null), anything else mirrors // verbatim to ?v=. decode rejects the empty string (→ null, i.e. not a valid sub-view). @@ -207,6 +207,31 @@ describe("useHistorySubview back", () => { expect(back).toHaveBeenCalledTimes(1); }); + // A sub-view left while an overlay of its own is still up — the Plex player's stats readout, a + // panel, the shortcuts sheet: the step back used to land as a USER Back, so the overlay listener + // spent it closing the top overlay and the sub-view stayed where it was. + it("closes the overlays standing over it first, then steps back out of the sub-view", async () => { + const { result } = renderHook(() => useHistorySubview("root", url)); + act(() => result.current.open("player")); + const closed = vi.fn(); + const holder: { unmount?: () => void } = {}; + const layer = renderHook(() => + useBackToClose(() => { + closed(); + holder.unmount?.(); + }), + ); + holder.unmount = layer.unmount; + await act(async () => { + await Promise.resolve(); + }); + expect(window.history.state?._ov).toBeDefined(); + act(() => result.current.back()); + await waitFor(() => expect(result.current.view).toBe("root")); + expect(closed).toHaveBeenCalledTimes(1); + expect(window.history.state?._ov).toBeUndefined(); + }); + it("returns to the root in place when the entry below is another document's", () => { const { result } = renderHook(() => useHistorySubview("root", url)); act(() => result.current.open("a")); diff --git a/frontend/src/lib/history.ts b/frontend/src/lib/history.ts index 0129b6f3..35ff60d7 100644 --- a/frontend/src/lib/history.ts +++ b/frontend/src/lib/history.ts @@ -174,7 +174,12 @@ export function useHistorySubview( // re-run its effect on each parent render (three such effects in PlexBrowse alone). `url` is a // module constant at every call site (PLEX_SUB_URL, MODE_URL) and the body otherwise touches only // module functions and the setter, so there is nothing left to churn on. - const open = useCallback( + // Both move the module to another view, so both are navigations: they close the overlays standing + // over it first (`closeOverlaysThenNavigate`, synchronous when none is open). A step taken under an + // open overlay landed as a USER Back — the overlay listener spent it closing the top overlay, so + // leaving the Plex player with its stats readout up only closed the readout — and a push or a + // replace on top of an overlay's entry is undone when that entry is reclaimed. + const go = useCallback( (next: T, opts?: { replace?: boolean }) => { setView(next); const kept = url?.entry ? url.entry(next) : next; @@ -198,13 +203,21 @@ export function useHistorySubview( }, [url], ); + const open = useCallback( + (next: T, opts?: { replace?: boolean }) => closeOverlaysThenNavigate(() => go(next, opts)), + [go], + ); // With no entry of this document below (see backStaysInDocument), the sub-view's own entry becomes // the root view in place — a step up the module, not out of the app. const rootRef = useLatest(root); - const back = useCallback(() => { - if (backStaysInDocument()) window.history.back(); - else open(rootRef.current, { replace: true }); - }, [open, rootRef]); + const back = useCallback( + () => + closeOverlaysThenNavigate(() => { + if (backStaysInDocument()) window.history.back(); + else go(rootRef.current, { replace: true }); + }), + [go, rootRef], + ); // Memoised as a whole for the same reason: a consumer that wants "re-run when the sub-view moves" // can now write `[sub]` and mean it, instead of re-running on every parent render. return useMemo(() => ({ view, open, back }), [view, open, back]); @@ -302,10 +315,10 @@ function historyInSync(): boolean { // render (measured back()→popstate: 1–13 ms in Chrome); nothing in the app refuses a close today, // but a future "discard your changes?" guard must not be navigated out from under. const CLOSE_TIMEOUT_MS = 1500; -// An overlay's Back callback is a Back STEP, and one entry can hold several steps: the Plex player's -// panels share a single entry and Back peels one per press. Navigating over it asks again, a render -// apart (the callback reads what the last one closed), until the entry is gone or the timeout drops -// the command. +// An overlay's Back callback is a Back STEP: the contract lets one entry hold several, peeled one per +// call. Every overlay today closes in one call (the Plex player gives each layer its own entry), but a +// navigation must not be dropped by one that does not, so it asks again, a render apart (the callback +// reads what the last one closed), until the entry is gone or the timeout drops the command. const RECLOSE_MS = 50; /** Close `targets` (via `close`), then `run` once they have unmounted AND the history has caught up.