diff --git a/frontend/src/App.keymap.contract.test.ts b/frontend/src/App.keymap.contract.test.ts deleted file mode 100644 index ee26db20..00000000 --- a/frontend/src/App.keymap.contract.test.ts +++ /dev/null @@ -1,33 +0,0 @@ -// The global Shift+W wiring, pinned at its call site. -// -// What this is: a source contract, the same idiom as uiBoundary.contract.test.ts — a structural rule -// asserted directly rather than left to prose. What it is NOT: a behaviour test. The behaviour lives -// in history.dom.test.ts and history.burial.dom.test.ts, which drive `navigateOverOverlays` against -// real overlay entries; the end-to-end gesture (Shift+W over an open player, then Back) is a browser -// check, because jsdom reproduces history ordering only as bookkeeping. -// -// It exists because the composition line itself was unpinned: App.tsx is the one module no test -// renders (main.tsx lazy-imports it and nothing else does), globalKeymap.dom.test.tsx drives the -// hook with its OWN handler, and history.dom.test.ts drives the helper with its own navigate — so -// dropping `navigateOverOverlays` from App left all 818 tests green while the user-visible bug came -// straight back (round 2, verified by running that revert). -import { readFileSync } from "node:fs"; -import { fileURLToPath } from "node:url"; - -import { describe, expect, it } from "vitest"; - -const APP = readFileSync(fileURLToPath(new URL("./App.tsx", import.meta.url)), "utf8"); - -describe("App · global keymap wiring", () => { - it("routes the global home shortcut through navigateOverOverlays", () => { - const call = /useGlobalKeymap\(([\s\S]*?)\);/.exec(APP)?.[1]; - // A rename or a move is a legitimate change — but then this test is the thing that has to be - // updated deliberately, which is the whole point of pinning it. - expect( - call, - "App.tsx no longer calls useGlobalKeymap in a shape this test can read", - ).toBeTypeOf("string"); - expect(call).toContain("navigateOverOverlays"); - expect(call).toContain('setPage("home")'); - }); -}); diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 707e07c6..869d7533 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -37,7 +37,6 @@ import Toaster from "./components/ui/Toaster"; import KeyboardHelp from "./components/ui/KeyboardHelp"; import AppCommandPalette from "./components/AppCommandPalette"; import { useGlobalKeymap } from "./lib/keymap"; -import { navigateOverOverlays } from "./lib/history"; import { StateMessage } from "./components/ui/QueryState"; import ErrorDialog from "./components/ErrorDialog"; import VersionBanner from "./components/VersionBanner"; @@ -142,13 +141,10 @@ export default function App() { // Home ("mit nézzek?"). Bound only once signed in, because that is where the overlays are mounted // (bottom of this tree) and where the modules exist at all — see useGlobalKeymap on why binding it // above the signed-out branch is worse than not binding it. - // Through `navigateOverOverlays`, because a global key can fire while an overlay is open and does - // not know it: the page change unmounts the player (it lives inside the module), whose reconcile - // then reclaimed the entry this push had just made — Shift+W over a playing video closed the - // player and snapped straight back to the feed. `setPage`'s boolean is load-bearing here, not - // decoration: pressed while ALREADY on Home (the default landing page) it navigates nowhere, and - // burying the open player's entry under a push that never happened orphaned it — see `buryUnder`. - useGlobalKeymap(!!meQuery.data, () => navigateOverOverlays(() => setPage("home"))); + // A global key can fire while an overlay is open; `setPage` closes every overlay before it pushes, + // so Shift+W over the player lands on Home with the player closed — also when Home is already the + // page underneath (owner decision 2026-09-09: the key always ends with the answer on screen). + useGlobalKeymap(!!meQuery.data, () => setPage("home")); // Pin this tab to whatever account it first loaded as. Without this, a tab that never picked // an account (it was riding the session's default) would silently swap identity when ANOTHER @@ -279,8 +275,8 @@ export default function App() { label: t("common.accessRequestsReview"), onClick: () => { focusAccessRequestsTab(); - // Over whatever overlay the toast was clicked in — as navigateTo does (see lib/nav). - navigateOverOverlays(() => setPage("users")); + // Clicked inside whatever overlay hosts the toast; setPage closes it before pushing. + setPage("users"); }, }, }); diff --git a/frontend/src/components/ChannelNameLink.tsx b/frontend/src/components/ChannelNameLink.tsx index f567573c..2be89068 100644 --- a/frontend/src/components/ChannelNameLink.tsx +++ b/frontend/src/components/ChannelNameLink.tsx @@ -11,7 +11,7 @@ import { urlWithParams } from "../lib/urlState"; // The href is the same URL `openChannel` pushes — `?chan=` on the current page — so a new tab // lands where the in-app navigation would have. A plain left click is still handled in-app, because // the SPA path does more than navigate (the player has to close first and reclaim its own -// Back-to-close history entry; see closeThenNavigate). +// Back-to-close history entry; see lib/history.closeOverlaysThenNavigate). type Props = Omit, "href" | "onClick" | "title"> & { channelId: string; title: string | null | undefined; diff --git a/frontend/src/components/NavigationProvider.dom.test.tsx b/frontend/src/components/NavigationProvider.dom.test.tsx index dc6e38a8..34606a34 100644 --- a/frontend/src/components/NavigationProvider.dom.test.tsx +++ b/frontend/src/components/NavigationProvider.dom.test.tsx @@ -1,16 +1,14 @@ // @vitest-environment jsdom // -// `setPage`'s no-op contract — the PRODUCER side of the history-burial fix, which nothing rendered -// before this. `setPage` early-returns when the requested page is already current with no channel -// page over it, and reports that by returning `false`; `lib/history.buryUnder` reads the boolean to -// decide whether the overlay entries beneath a navigation are still reclaimable. Get it wrong and -// the round-2 bug is back: Shift+W over the player on Home buries an entry nothing pushed past, and -// the user's next browser Back does nothing. +// The navigation actions against the overlay history stack: `setPage` and `openChannel` never push +// under an open overlay — they close it and push once its Back-to-close entry is reclaimed — and +// `setPage`'s same-page guard, which decides whether anything is pushed at all. // -// The type does NOT fence this. `NavigationActions` pins the SHAPE (`=> boolean`), so deleting one -// `return` fails tsc — but round 3 measured that swapping the two values (early return `true`, tail -// `false`) passes typecheck and all 822 tests. Hence assertions on the value AND on the effects the -// early return skips, which is what the boolean actually stands for. +// The overlay half is what used to live at each call site (Shift+W in App, a toast's "View", the +// palette, the player's channel link) and could be forgotten at any of them; it now lives in the two +// actions, so this file is where it is pinned. The end-to-end gesture — Shift+W over the player, +// then browser Back — is still checked in a real browser: jsdom reproduces history ordering only as +// bookkeeping. import { describe, it, expect, afterEach, beforeEach, vi } from "vitest"; import { renderHook, act, cleanup } from "@testing-library/react"; import type { ReactNode } from "react"; @@ -27,6 +25,7 @@ vi.mock("react-i18next", async (importOriginal) => { }); import { NavigationProvider, useNavigation, useNavigationActions } from "./NavigationProvider"; +import { useBackToClose } from "../lib/history"; const wrapper = ({ children }: { children: ReactNode }) => ( {children} @@ -34,6 +33,25 @@ const wrapper = ({ children }: { children: ReactNode }) => ( const render = () => renderHook(() => ({ ...useNavigation(), ...useNavigationActions() }), { wrapper }); +const settle = () => + act(async () => { + await new Promise((r) => setTimeout(r, 0)); + }); + +/** An open overlay (the player) whose close really unmounts it; its reclaiming back() is stubbed to + * land its popstate at once. */ +async function openOverlay() { + const back = vi.spyOn(window.history, "back").mockImplementation(() => { + window.dispatchEvent(new PopStateEvent("popstate", { state: window.history.state })); + }); + const holder: { unmount?: () => void } = {}; + const onClose = vi.fn(() => holder.unmount?.()); + const layer = renderHook(() => useBackToClose(onClose)); + holder.unmount = layer.unmount; + await settle(); + return { back, onClose }; +} + beforeEach(() => { localStorage.clear(); sessionStorage.setItem("siftlode.activeAccount", "1"); // account-scoped page memory @@ -41,46 +59,29 @@ beforeEach(() => { }); // Not automatic: vitest runs without `globals`, so RTL never registers its own afterEach — leaving a // provider (and its popstate listener) mounted for the next case. Same reason Home.dom.test.tsx does. -afterEach(cleanup); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); -describe("NavigationProvider · setPage reports whether it navigated", () => { - it("returns true and pushes an entry for a real page change", () => { +describe("NavigationProvider · setPage's same-page guard", () => { + it("pushes an entry for a real page change", () => { const { result } = render(); const push = vi.spyOn(window.history, "pushState"); - try { - let reported: boolean | undefined; - act(() => { - reported = result.current.setPage("plex"); - }); - expect(reported).toBe(true); - expect(push).toHaveBeenCalledTimes(1); - expect(result.current.page).toBe("plex"); - } finally { - push.mockRestore(); - } + act(() => result.current.setPage("plex")); + expect(push).toHaveBeenCalledTimes(1); + expect(result.current.page).toBe("plex"); }); - it("returns false and pushes nothing when it is already that page", () => { + it("pushes nothing when it is already that page", () => { const { result } = render(); - act(() => { - result.current.setPage("plex"); - }); + act(() => result.current.setPage("plex")); const push = vi.spyOn(window.history, "pushState"); - try { - let reported: boolean | undefined; - act(() => { - reported = result.current.setPage("plex"); - }); - // Both halves matter: `false` is what `buryUnder` reads, and "pushed nothing" is what makes - // that answer true. A value-only assertion would pass against an inverted pair. - expect(reported).toBe(false); - expect(push).not.toHaveBeenCalled(); - } finally { - push.mockRestore(); - } + act(() => result.current.setPage("plex")); + expect(push).not.toHaveBeenCalled(); }); - it("returns true even for the current page when a channel page is over it", () => { + it("still navigates to the current page when a channel page is over it", () => { // The nav rail's own module while a channel page is open over it is a real navigation: it closes // the channel page. The `&& !channelViewRef.current` half of the guard is what makes it one. const { result } = render(); @@ -90,22 +91,64 @@ describe("NavigationProvider · setPage reports whether it navigated", () => { // asking for "feed" was just an ordinary page change and deleting the clause left it green. const current = result.current.page; expect(current).toBe("home"); - act(() => { - result.current.openChannel("UC1", "A channel"); - }); + act(() => result.current.openChannel("UC1", "A channel")); expect(result.current.channelView).not.toBeNull(); const push = vi.spyOn(window.history, "pushState"); - try { - let reported: boolean | undefined; - act(() => { - reported = result.current.setPage(current); - }); - expect(reported).toBe(true); - expect(push).toHaveBeenCalledTimes(1); - expect(result.current.channelView).toBeNull(); - } finally { - push.mockRestore(); - } + act(() => result.current.setPage(current)); + expect(push).toHaveBeenCalledTimes(1); + expect(result.current.channelView).toBeNull(); + }); +}); + +describe("NavigationProvider · never under an open overlay", () => { + it("setPage closes the overlay first and pushes only once its entry is reclaimed", async () => { + const { result } = render(); + const { back, onClose } = await openOverlay(); + const push = vi.spyOn(window.history, "pushState"); + + act(() => result.current.setPage("plex")); + expect(onClose).toHaveBeenCalledTimes(1); + expect(push).not.toHaveBeenCalled(); // not while the player's entry is still current + + await settle(); + await settle(); + expect(back).toHaveBeenCalledTimes(1); + expect(push).toHaveBeenCalledTimes(1); + expect(result.current.page).toBe("plex"); + expect(window.history.state).toMatchObject({ sfPage: "plex" }); + }); + + // Shift+W on Home with the player open (owner decision 2026-09-09): nothing to push, but the player + // closes, and its entry is reclaimed rather than orphaned under a push that never happened. + it("setPage to the current page closes the overlay and pushes nothing", async () => { + const { result } = render(); + const { back, onClose } = await openOverlay(); + const push = vi.spyOn(window.history, "pushState"); + + act(() => result.current.setPage(result.current.page)); + await settle(); + await settle(); + expect(onClose).toHaveBeenCalledTimes(1); + expect(back).toHaveBeenCalledTimes(1); + expect(push).not.toHaveBeenCalled(); + }); + + // The player's channel link and the palette's channel rows. + it("openChannel closes the overlay first and pushes the channel once its entry is reclaimed", async () => { + const { result } = render(); + const { back, onClose } = await openOverlay(); + const push = vi.spyOn(window.history, "pushState"); + + act(() => result.current.openChannel("UC1", "A channel")); + expect(onClose).toHaveBeenCalledTimes(1); + expect(push).not.toHaveBeenCalled(); + + await settle(); + await settle(); + expect(back).toHaveBeenCalledTimes(1); + expect(push).toHaveBeenCalledTimes(1); + expect(result.current.channelView).toEqual({ id: "UC1", name: "A channel" }); + expect(window.history.state).toMatchObject({ _chan: "UC1" }); }); }); diff --git a/frontend/src/components/NavigationProvider.tsx b/frontend/src/components/NavigationProvider.tsx index 2186076f..ec2168e5 100644 --- a/frontend/src/components/NavigationProvider.tsx +++ b/frontend/src/components/NavigationProvider.tsx @@ -14,7 +14,7 @@ import { pageTitleKey } from "../lib/pageMeta"; import { setAccountRaw } from "../lib/storage"; import { useAccountRehydrate } from "../lib/useAccountRehydrate"; import { setNavigator } from "../lib/nav"; -import { nextNavId, notifyNav } from "../lib/history"; +import { closeOverlaysThenNavigate, nextNavId, notifyNav } from "../lib/history"; import { channelViewFrom, loadInitialPage, PAGE_KEY, type ChannelView } from "./navigationInit"; // Owns the app's navigation + browser-history state, lifted out of the App god component so pages @@ -37,10 +37,11 @@ export interface NavigationState { } export interface NavigationActions { - /** Switch modules. Returns whether it actually navigated — it is a no-op when the requested page - * is already current with no channel page over it, and a caller that buries overlay history - * entries under the push has to know that (see `lib/history.navigateOverOverlays`). */ - setPage: (next: Page) => boolean; + /** Switch modules. Closes every open overlay first and pushes only once their history entries are + * back (`lib/history.closeOverlaysThenNavigate`), so over an open player the push lands a moment + * after the call; with nothing open it is synchronous. The requested page being already current + * (with no channel page over it) pushes nothing — but the overlays still close. */ + setPage: (next: Page) => void; openChannel: (id: string, name?: string) => void; closeChannel: () => void; enterYtSearch: (q: string) => void; @@ -53,7 +54,7 @@ const StateContext = createContext({ ytSearch: null, }); const ActionsContext = createContext({ - setPage: () => false, + setPage: () => {}, openChannel: () => {}, closeChannel: () => {}, enterYtSearch: () => {}, @@ -118,53 +119,61 @@ export function NavigationProvider({ children }: { children: ReactNode }) { }); const setPage = useCallback( - (next: Page) => { - // A channel page overlays the content column, so a nav click must close it even when the - // underlying page is unchanged (e.g. "Feed" while a channel opened from the feed). - if (next === pageRef.current && !channelViewRef.current) return false; - markDirty(); - setChannelView(null); // leave any open channel page when navigating via the rail - setPageState(next); - setAccountRaw(PAGE_KEY, next); - // Push an in-app history entry so browser/mouse Back steps through pages instead of leaving the - // app. A fresh { sfPage } (no carried-over _yt/_chan markers) so each page starts at its root, and - // the address bar reflects the page (feed is implicit) while clearing the view-specific params - // (chan/v/pl and the plex content keys pscope/psort/pshow/pf) — a page switch resets to that - // module's root. Feed filters (owned by FeedFiltersProvider) are left intact; the plex/channels - // keys re-mirror from PlexProvider / ChannelsProvider on return to their page. - window.history.pushState( - { sfPage: next, sfScroll: nextNavId() }, - "", - urlWithParams({ - page: next === "feed" ? null : next, - chan: null, - v: null, - pl: null, - pscope: null, - psort: null, - pshow: null, - pf: null, - ctab: null, - csort: null, - cf: null, - }), - ); - notifyNav(); - return true; - }, + (next: Page) => + // Never under an open overlay — see the interface doc. The refs are read when the push + // actually happens, which over an open player is a moment after this call. + closeOverlaysThenNavigate(() => { + // A channel page overlays the content column, so a nav click must close it even when the + // underlying page is unchanged (e.g. "Feed" while a channel opened from the feed). + if (next === pageRef.current && !channelViewRef.current) return; + markDirty(); + setChannelView(null); // leave any open channel page when navigating via the rail + setPageState(next); + setAccountRaw(PAGE_KEY, next); + // Push an in-app history entry so browser/mouse Back steps through pages instead of leaving + // the app. A fresh { sfPage } (no carried-over _yt/_chan markers) so each page starts at its + // root, and the address bar reflects the page (feed is implicit) while clearing the + // view-specific params (chan/v/pl and the plex content keys pscope/psort/pshow/pf) — a page + // switch resets to that module's root. Feed filters (owned by FeedFiltersProvider) are left + // intact; the plex/channels keys re-mirror from PlexProvider / ChannelsProvider on return to + // their page. + window.history.pushState( + { sfPage: next, sfScroll: nextNavId() }, + "", + urlWithParams({ + page: next === "feed" ? null : next, + chan: null, + v: null, + pl: null, + pscope: null, + psort: null, + pshow: null, + pf: null, + ctab: null, + csort: null, + cf: null, + }), + ); + notifyNav(); + }), [markDirty], ); // A sub-view (channel / YT search) is an OVERLAY, not a page change, so it does NOT markDirty: a // pre-pin account re-hydrate should still restore the stored page underneath — the re-hydrate's // replaceState spreads the _chan/_yt markers, so the overlay survives on top of the restored page. + // + // Like setPage it never lands under an open overlay: the player's channel link and the palette's + // channel rows call it from inside one. const openChannel = useCallback((id: string, name?: string) => { - setChannelView({ id, name }); - const st = window.history.state || {}; - const url = urlWithParams({ chan: id }); - if (st._chan) - window.history.replaceState({ ...st, _chan: id, _chanName: name ?? null }, "", url); - else window.history.pushState({ ...st, _chan: id, _chanName: name ?? null }, "", url); + closeOverlaysThenNavigate(() => { + setChannelView({ id, name }); + const st = window.history.state || {}; + const url = urlWithParams({ chan: id }); + if (st._chan) + window.history.replaceState({ ...st, _chan: id, _chanName: name ?? null }, "", url); + else window.history.pushState({ ...st, _chan: id, _chanName: name ?? null }, "", url); + }); }, []); const closeChannel = useCallback(() => { diff --git a/frontend/src/components/PlayerModal.tsx b/frontend/src/components/PlayerModal.tsx index 6b71e37f..15ccd35e 100644 --- a/frontend/src/components/PlayerModal.tsx +++ b/frontend/src/components/PlayerModal.tsx @@ -46,7 +46,7 @@ import { import { renderDescription } from "../lib/descriptionLinks"; import { fullscreenElement } from "../lib/fullscreen"; import { dispatch, isActivationTarget, useKeyScope } from "../lib/keymap"; -import { closeThenNavigate, useBackToClose } from "../lib/history"; +import { useBackToClose } from "../lib/history"; import ChannelNameLink from "./ChannelNameLink"; import Toaster, { useOverlayOrder } from "./ui/Toaster"; import { afterRemoval } from "../lib/playerQueue"; @@ -822,13 +822,11 @@ export default function PlayerModal({ channelId={active.channel_id} title={active.channel_title} onOpenInApp={() => { - const cid = active.channel_id; - const cname = active.channel_title ?? undefined; - // Close first, open the channel only once this player's own Back-to-close - // entry has been reclaimed — otherwise that reclaim undoes the push and - // clicking the name merely closes the player. The rule lives in history.ts - // now; the command palette needs exactly the same one. - closeThenNavigate(onClose, () => openChannel(cid, cname)); + // openChannel closes this player first and pushes only once its + // Back-to-close entry is reclaimed — otherwise that reclaim undoes the push + // and clicking the name merely closes the player + // (lib/history.closeOverlaysThenNavigate). + openChannel(active.channel_id, active.channel_title ?? undefined); }} className="font-medium hover:text-accent truncate text-left" /> diff --git a/frontend/src/components/ui/CommandPalette.dom.test.tsx b/frontend/src/components/ui/CommandPalette.dom.test.tsx index c73afbcb..70bd113b 100644 --- a/frontend/src/components/ui/CommandPalette.dom.test.tsx +++ b/frontend/src/components/ui/CommandPalette.dom.test.tsx @@ -21,7 +21,7 @@ beforeAll(() => { afterEach(cleanup); // `choose` closes and waits for the dialog's own Back-to-close history entry to be reclaimed before -// running the command (see closeThenNavigate), so the command lands a tick later — hence the waits. +// running the command (see closeTopOverlayThenRun), so the command lands a tick later — hence the waits. const ran: string[] = []; const groups = (): CommandGroup[] => [ { diff --git a/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx b/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx index 81cf801e..3191b6d1 100644 --- a/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx +++ b/frontend/src/components/ui/CommandPalette.ordering.dom.test.tsx @@ -1,10 +1,9 @@ // @vitest-environment jsdom // // ONE test, ONE file, on purpose. This is the close→reclaim→run ordering contract of the palette -// (see closeThenNavigate), and it ASSERTS on a real useBackToClose entry driven through a real -// programmatic history.back() — which is why it cannot share module state with anything. (It is not -// the only place in the suite where an unstubbed back() fires; history.burial.dom.test.ts is the -// other file that asserts on one, and any test rendering a real Modal can let one go.) +// (see closeTopOverlayThenRun), and it ASSERTS on a real useBackToClose entry driven through a real +// programmatic history.back() — which is why it cannot share module state with anything. (Any test +// rendering a real Modal can let an unstubbed back() go, too.) // // It cannot live in CommandPalette.dom.test.tsx. jsdom gives a test FILE a single session history, // and history.ts's overlay bookkeeping is module-global, so the sibling tests there leave an @@ -50,7 +49,7 @@ function Harness() { ); } -describe("CommandPalette · closeThenNavigate ordering", () => { +describe("CommandPalette · close-then-run ordering", () => { it("closes, and runs the command only once the close has landed", async () => { render(); // Load-bearing: useBackToClose pushes its entry from a coalesced microtask, so a click in the @@ -62,8 +61,9 @@ describe("CommandPalette · closeThenNavigate ordering", () => { fireEvent.click(screen.getByText("TheVR Podcast")); expect(ran).toEqual(["closed"]); - // Inside closeThenNavigate's 250ms no-popstate fallback, so a pass means the RECLAIM ran the - // command — not the safety net that exists for overlays which never got an entry at all. - await waitFor(() => expect(ran).toEqual(["closed", "ch:1"]), { timeout: 200 }); + // Well inside closeTopOverlayThenRun's 1500 ms give-up timer, so a pass means the RECLAIM ran the + // command — not the late path for a back() that never reports. (The margin used to be 200 ms + // under a 250 ms fallback, which made this load-sensitive by construction.) + await waitFor(() => expect(ran).toEqual(["closed", "ch:1"]), { timeout: 1000 }); }); }); diff --git a/frontend/src/components/ui/CommandPalette.tsx b/frontend/src/components/ui/CommandPalette.tsx index d0f00fca..aff1833a 100644 --- a/frontend/src/components/ui/CommandPalette.tsx +++ b/frontend/src/components/ui/CommandPalette.tsx @@ -2,7 +2,7 @@ import { useEffect, useId, useMemo, useRef, useState, type ReactNode } from "rea import { useTranslation } from "react-i18next"; import { Search } from "lucide-react"; import Modal from "./Modal"; -import { closeThenNavigate } from "../../lib/history"; +import { closeTopOverlayThenRun } from "../../lib/history"; // The app's jump-to bar: one text field over every place you can go and every app-wide thing you can // do, opened with Ctrl/Cmd+K. It is the "one-layer operation" half of E10 — reaching a channel three @@ -92,9 +92,11 @@ export default function CommandPalette({ if (!open) return null; - // Close, wait for this dialog's own Back-to-close entry to be reclaimed, THEN run. Most commands - // navigate, and a pushState issued before that reclaim is undone by it — see closeThenNavigate. - const choose = (item: CommandItem) => closeThenNavigate(onClose, item.run); + // Close, wait for this dialog's own Back-to-close entry to be reclaimed, THEN run: most commands + // navigate, and a pushState issued before that reclaim is undone by it. Only THIS dialog closes — + // a command that navigates closes whatever is underneath itself, one that does not (a theme) must + // leave it standing. See closeTopOverlayThenRun. + const choose = (item: CommandItem) => closeTopOverlayThenRun(onClose, item.run); // These live on the input rather than in the shared keymap: they are the field's own editing keys // (like a