From 7f17f488ab7f0356c48301d0e7087e3b7e8e463f Mon Sep 17 00:00:00 2001 From: npeter83 Date: Thu, 8 Oct 2026 01:16:30 +0200 Subject: [PATCH] fix(nav): a sub-view step closes the overlays standing over it first Leaving the Plex player while one of its layers was up (the info panel when the film ended, the countdown under the back arrow) landed as a user Back: the overlay listener spent it closing the top layer and the player stayed. useHistorySubview's open/back now go through closeOverlaysThenNavigate, like setPage. The RECLOSE comment no longer cites the shared Plex entry. --- frontend/src/lib/history.dom.test.ts | 6 ++-- frontend/src/lib/history.subview.dom.test.ts | 29 ++++++++++++++++-- frontend/src/lib/history.ts | 31 ++++++++++++++------ 3 files changed, 52 insertions(+), 14 deletions(-) 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.