From 0d5c6273f1ae89db05c7525ddf0c2a81a1236555 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 27 Oct 2026 13:52:42 +0100 Subject: [PATCH] fix(nav): the address follows the view through overlays, reloads and Back (review round 1) - A URL mirror names the view again on every entry Back or Forward lands on, so a change made under a dialog, a drawer or a sub-view is not undone by the next reload. - A view pushed from the entry a reload under a dialog started from no longer inherits its overlay mark; a reload under two overlays closes onto the page in one step. - The linked player reopens over a live search, gives itself up when its list moves first, and plays alone over an emptied playlist; a capped Files list keeps the link it cannot resolve. - The Feed press over a channel ends the search; leaving a search ends its asked mark; Files and Playlists searches end with their page; the feed always names its view and debounces its search. - Search-box keys are declared per module and kept out of the client log below TRACE. - Writers outside lib/ go through codecs; the URL contract scans computed keys, overlay writers and two-line reads, and holds every page to a bookmark round trip (Config, Activity, Playlists and the Files sort and viewer added). --- docs/development.md | 15 +- frontend/e2e/bookmark.spec.ts | 101 ++++++++++++- .../navigation/NavigationProvider.tsx | 53 +++++-- .../NavigationProvider.ytSearch.dom.test.tsx | 51 +++++++ .../src/features/navigation/moduleRegistry.ts | 4 + .../features/player/useOpenLinkedPlayer.ts | 29 +++- frontend/src/i18n/locales/en/feed.json | 4 +- frontend/src/i18n/locales/hu/feed.json | 4 +- frontend/src/i18n/locales/hu/files.json | 2 +- frontend/src/i18n/locales/hu/player.json | 2 +- frontend/src/lib/adminUsersTab.ts | 5 + frontend/src/lib/configTab.ts | 5 + frontend/src/lib/filesUrlState.ts | 24 +++- .../lib/history.reloadedNested.dom.test.tsx | 47 ++++++ .../history.reloadedOverlayNav.dom.test.tsx | 40 ++++++ frontend/src/lib/history.ts | 70 +++++++-- frontend/src/lib/logHooks.dom.test.ts | 18 ++- frontend/src/lib/logHooks.ts | 9 +- frontend/src/lib/moduleDef.ts | 4 + frontend/src/lib/playlistsUrlState.ts | 9 ++ frontend/src/lib/settingsTab.ts | 5 + frontend/src/lib/urlState.dom.test.ts | 4 +- frontend/src/lib/urlState.ts | 28 ++-- frontend/src/lib/useUrlMirror.dom.test.ts | 42 +++++- frontend/src/lib/useUrlMirror.ts | 39 ++++- frontend/src/main.tsx | 3 +- frontend/src/moduleSearch.url.dom.test.tsx | 26 +++- frontend/src/moduleUrl.contract.test.ts | 134 ++++++++++++++---- .../modules/activity/Activity.dom.test.tsx | 6 +- frontend/src/modules/activity/module.ts | 1 + frontend/src/modules/channels/module.ts | 1 + frontend/src/modules/config/ConfigPanel.tsx | 4 +- frontend/src/modules/feed/Feed.tsx | 23 +-- .../modules/feed/Feed.ytReopen.dom.test.tsx | 58 +++++++- .../src/modules/feed/FeedFiltersProvider.tsx | 6 +- .../feed/FeedFiltersProvider.url.dom.test.tsx | 82 +++++++++++ frontend/src/modules/feed/module.ts | 1 + frontend/src/modules/files/FilePreview.tsx | 4 +- .../src/modules/files/Files.menu.dom.test.tsx | 20 +++ frontend/src/modules/files/Files.tsx | 13 +- frontend/src/modules/files/FilesProvider.tsx | 25 ++-- frontend/src/modules/files/LibraryPlayer.tsx | 4 +- frontend/src/modules/files/module.ts | 1 + .../modules/playlists/Playlists.dom.test.tsx | 26 ++++ frontend/src/modules/playlists/Playlists.tsx | 11 +- .../modules/playlists/PlaylistsProvider.tsx | 12 +- frontend/src/modules/playlists/module.ts | 1 + frontend/src/modules/plex/module.ts | 1 + .../src/modules/settings/SettingsPanel.tsx | 4 +- frontend/src/modules/users/AdminUsers.tsx | 4 +- 50 files changed, 928 insertions(+), 157 deletions(-) create mode 100644 frontend/src/lib/history.reloadedNested.dom.test.tsx create mode 100644 frontend/src/lib/history.reloadedOverlayNav.dom.test.tsx create mode 100644 frontend/src/modules/feed/FeedFiltersProvider.url.dom.test.tsx diff --git a/docs/development.md b/docs/development.md index 994a3d87..a5d4be5e 100644 --- a/docs/development.md +++ b/docs/development.md @@ -49,11 +49,13 @@ A frontend page is one folder, `frontend/src/modules//` (the layering is in address-bar key its view writes (the URL is the view: a bookmark reopens it), its list kept in `lib/` beside its codec; the shell clears them on a page switch and opens a page without another page's keys. Every setting the page shows — a tab, a sort, a filter, a search, the item open in it — goes there: a codec - in `lib/UrlState.ts` (read the params into the view, write the view as updates, a default left out), - written with `useUrlMirror` (a sub-view through `useHistorySubview`'s `url`, an overlay through - `useBackToClose`'s), read back first on mount (a link wins over anything remembered). Add its round trip to - `e2e/bookmark.spec.ts`. Only what an address cannot promise to reopen stays out, said in its codec (Home's - draw, `lib/homeUrlState`). + in `lib/UrlState.ts` (read the params into the view, write the view as updates, a default left out — + or always written, when an address naming none falls back to a view remembered elsewhere), written with + `useUrlMirror` (a sub-view through `useHistorySubview`'s `url`, an overlay through `useBackToClose`'s), + always as the codec's result: a writer outside `lib/` computes no key of its own. Read back first on mount + (a link wins over anything remembered). A search box's key is also in `typedUrlKeys` (the client log keeps + what was typed out). Add its round trip to `e2e/bookmark.spec.ts`. Only what an address cannot promise to + reopen stays out, said in its codec (Home's draw, `lib/homeUrlState`). 3. `index.ts`, the module's public face: the definition, and only what other folders must read (its store's hooks). Another folder reaches the module through this file or not at all. 4. The id in `lib/urlState`'s `PAGES`, and the definition in `features/navigation/moduleRegistry.ts`, in @@ -67,7 +69,8 @@ A frontend page is one folder, `frontend/src/modules//` (the layering is in What holds it: `moduleRegistry.contract.test.ts` (every id defined once, from its own folder, lazily), `moduleStores.contract.test.tsx` (no store reads another module), `moduleUrl.contract.test.ts` (each URL key -belongs to one module and matches what its codec writes; no undeclared key literal), the `local/layers` lint rule (imports only go +belongs to one module and matches what its codec writes; a writer outside `lib/` computes no key; no key literal +written or read from the live query undeclared; every page with keys in the bookmark net), the `local/layers` lint rule (imports only go down, a module only through its face) and `chunkGuard` in the build (no lazy page on the boot path), plus the i18n parity test. diff --git a/frontend/e2e/bookmark.spec.ts b/frontend/e2e/bookmark.spec.ts index 85815399..fbf8d45a 100644 --- a/frontend/e2e/bookmark.spec.ts +++ b/frontend/e2e/bookmark.spec.ts @@ -10,8 +10,10 @@ import { // The address is the view: a view set through the UI, its address opened as a bookmark — in a NEW // browser context, so nothing this tab remembered (its history entries, sessionStorage, the view -// storage it wrote) can stand in for what the address says. One round trip per page, plus the -// overlays the shell opens over any page (the channel page, the player) and the live search. +// storage it wrote) can stand in for what the address says. One round trip per page that names its +// view (moduleUrl.contract.test holds the list to the module definitions), plus the overlays the +// shell opens over any page (the channel page, the player) and the live search. Not Plex: what it +// shows is the instance's own library, which a test stack may not have. const STORAGE_STATE = "e2e/.auth/state.json"; @@ -26,6 +28,13 @@ async function bookmarked(browser: Browser, page: Page, prepare?: (p: Page) => P } const tab = (p: Page, name: string) => p.getByRole("tab", { name, exact: true }); +const column = (p: Page, name: string) => + p.getByRole("columnheader", { name: new RegExp(`^${name}`) }); + +async function gotoAdminPage(page: Page, id: string): Promise { + await page.getByTestId("nav-admin").click(); + await page.getByTestId(`nav-admin-${id}`).click(); +} test.describe("a bookmark reopens the view it was taken of", () => { test("the feed's filters", async ({ page, browser }) => { @@ -60,14 +69,58 @@ test.describe("a bookmark reopens the view it was taken of", () => { test("an admin page's tab", async ({ page, browser }) => { await openApp(page, "/?page=feed"); - await page.getByTestId("nav-admin").click(); - await page.getByTestId("nav-admin-users").click(); + await gotoAdminPage(page, "users"); await tab(page, "Access requests").click(); const fresh = await bookmarked(browser, page); await expect(tab(fresh, "Access requests")).toHaveAttribute("aria-selected", "true"); await fresh.context().close(); }); + test("a Configuration group", async ({ page, browser }) => { + await openApp(page, "/?page=feed"); + await gotoAdminPage(page, "config"); + const second = page.getByRole("tab").nth(1); + const name = (await second.textContent()) ?? ""; + await second.click(); + await expect(page).toHaveURL(/[?&]cfgtab=/); + const fresh = await bookmarked(browser, page); + await expect(fresh.getByRole("tab", { name, exact: true })).toHaveAttribute( + "aria-selected", + "true", + ); + await fresh.context().close(); + }); + + test("the Activity tab and its time window", async ({ page, browser }) => { + await openApp(page, "/?page=feed"); + await gotoAdminPage(page, "activity"); + await tab(page, "Server log").click(); + await page.getByRole("button", { name: "30 days", exact: true }).click(); + await expect(page).toHaveURL(/[?&]aw=30d/); + const fresh = await bookmarked(browser, page); + await expect(tab(fresh, "Server log")).toHaveAttribute("aria-selected", "true"); + await expect(fresh.getByRole("button", { name: "30 days", exact: true })).toHaveAttribute( + "aria-pressed", + "true", + ); + await fresh.context().close(); + }); + + test("a playlist and its sort", async ({ page, browser }) => { + await openApp(page, "/?page=feed"); + await gotoPage(page, "playlists"); + await expect(column(page, "Title")).toBeVisible(); + await column(page, "Title").getByRole("button").first().click(); + await expect(page).toHaveURL(/[?&]plsort=/); + const sorted = await column(page, "Title").getAttribute("aria-sort"); + const fresh = await bookmarked(browser, page); + await expect(column(fresh, "Title")).toHaveAttribute("aria-sort", sorted ?? ""); + expect(new URL(fresh.url()).searchParams.get("pl")).toBe( + new URL(page.url()).searchParams.get("pl"), + ); + await fresh.context().close(); + }); + test("the Channels manager's tab and name search", async ({ page, browser }) => { await openApp(page, "/?page=feed"); await gotoPage(page, "channels"); @@ -90,6 +143,38 @@ test.describe("a bookmark reopens the view it was taken of", () => { await fresh.context().close(); }); + // A file of the test's own, uploaded for it and deleted after: the account's library is left as + // it was found. + test("the Files sort, and the file open in the viewer", async ({ page, browser }) => { + await openApp(page, "/?page=feed"); + const NAME = "bookmark-note"; + // One a run cut short left behind goes first. + const left = (await (await page.request.get("/api/files")).json()) as Record[]; + for (const f of left.filter((f) => JSON.stringify(f).includes(`"${NAME}`))) + await page.request.delete(`/api/files/${String(f.id)}`); + const up = await page.request.post("/api/files/upload", { + headers: { "X-Upload-Filename": `${NAME}.txt` }, + data: Buffer.from("a note the bookmark reopens\n"), + }); + expect(up.ok(), await up.text()).toBeTruthy(); + const { id } = (await up.json()) as { id: number }; + try { + await gotoPage(page, "files"); + await column(page, "Name").getByRole("button").first().click(); + await expect(page).toHaveURL(/[?&]fsort=name/); + const sorted = await column(page, "Name").getAttribute("aria-sort"); + await page.getByText(NAME, { exact: true }).first().click(); + await expect(page).toHaveURL(new RegExp(`[?&]fview=${id}`)); + const fresh = await bookmarked(browser, page); + await expect(fresh.getByRole("dialog", { name: NAME })).toBeVisible(); + await fresh.keyboard.press("Escape"); + await expect(column(fresh, "Name")).toHaveAttribute("aria-sort", sorted ?? ""); + await fresh.context().close(); + } finally { + await page.request.delete(`/api/files/${id}`); + } + }); + test("a channel page's own tab, over the page it was opened from", async ({ page, browser }) => { await openApp(page, "/?page=feed"); await expect(page.getByTestId("video-card").first()).toBeVisible(); @@ -114,9 +199,15 @@ test.describe("a bookmark reopens the view it was taken of", () => { }); // The search itself never reaches YouTube here: the answer is the server's shape, served by the - // test, and the request it answers is what the address named. + // test, and the request it answers is what the address named. The instance's search settings are + // pinned too (the free source, switched on): a stack left on the API source reopens the search + // waiting for its press, by design, and one with search off offers none. test("a live YouTube search, its term and its count", async ({ page, browser }) => { const answer = async (p: Page, seen: string[]) => { + await p.route("**/api/me", async (route) => { + const me = (await (await route.fetch()).json()) as Record; + return route.fulfill({ json: { ...me, search_source: "scrape", search_enabled: true } }); + }); await p.route("**/api/search/youtube?**", (route) => { seen.push(route.request().url()); return route.fulfill({ json: { items: [], next_cursor: null, source: "scrape" } }); diff --git a/frontend/src/features/navigation/NavigationProvider.tsx b/frontend/src/features/navigation/NavigationProvider.tsx index a8eded41..fa18c807 100644 --- a/frontend/src/features/navigation/NavigationProvider.tsx +++ b/frontend/src/features/navigation/NavigationProvider.tsx @@ -86,8 +86,9 @@ export function NavigationProvider({ children }: { children: ReactNode }) { // The live YouTube search: named in the address (`yt`, and `ytn` for its count, lib/ytSearchUrlState) // so a reload or a link opens it again; its own history entry, so Back steps player → search → feed. // Whether it RUNS by itself is not the address's to say: `ytAsked` is the term this document was - // asked to search for (sent from the search box, or "Search YouTube again" pressed). Anything else - // — a reload, a link, a Back onto an older search — runs only when it costs no quota (Feed decides). + // asked to search for (sent from the search box, or "Search YouTube again" pressed), on the entries + // it has stayed on since — leaving the search ends it. Anything else — a reload, a link, a Back or + // a Forward onto a search — runs only when it costs no quota (Feed decides). const [ytSearch, setYtSearch] = useState(() => page === "feed" ? ytSearchFrom(new URLSearchParams(window.location.search)) : null, ); @@ -96,10 +97,16 @@ export function NavigationProvider({ children }: { children: ReactNode }) { ); const [ytAsked, setYtAsked] = useState(null); const ytSearchRef = useLatest(ytSearch); + const ytCountRef = useLatest(ytCount); + // The search ends: off the screen, and no longer asked for. + const endYtSearch = useCallback(() => { + setYtSearch(null); + setYtAsked(null); + }, []); // The open channel page rides in history.state (_chan = id, _chanName = a name to show before the // detail loads) AND is mirrored to the address bar as ?chan=, so Back closes it, a reload restores - // it, and a bookmarked ?chan link opens it (unlike ytSearch, which is dropped). Reload of an in-app - // entry reads history.state._chan; a fresh bookmark has only the ?chan param. + // it, and a bookmarked ?chan link opens it. Reload of an in-app entry reads history.state._chan; a + // fresh bookmark has only the ?chan param. const [channelView, setChannelView] = useState(() => channelViewFrom(window.history.state, window.location.search), ); @@ -134,6 +141,9 @@ export function NavigationProvider({ children }: { children: ReactNode }) { return; } setChannelView(null); // leave any open channel page when navigating via the rail + // A live search under that channel ends too: the page opens at its root, which the address + // below names — the Feed press over a channel opened from a search included. + endYtSearch(); setPageState(next); setAccountRaw(PAGE_KEY, next); // Push an in-app history entry so browser/mouse Back steps through pages instead of leaving @@ -151,7 +161,7 @@ export function NavigationProvider({ children }: { children: ReactNode }) { notifyNav(); then?.(); }), - [pageRef, channelViewRef], + [pageRef, channelViewRef, endYtSearch], ); // A sub-view (channel / YT search) is an OVERLAY, not a page change. Like setPage it never lands @@ -179,11 +189,20 @@ export function NavigationProvider({ children }: { children: ReactNode }) { // No owned history entry (e.g. opened straight from a ?chan bookmark), or none of this document // below it to step back to (a full tab, see backStaysInDocument): clear it in place — the // entry's channel marks and the ?chan param. + // A live search under it (the channel opened from its results) is on screen again, and named + // again: the channel's address carried none of it. setChannelView(null); const { _chan: _id, _chanName: _name, ...st } = window.history.state ?? {}; - window.history.replaceState(st, "", urlWithParams(closedChannelParams())); + window.history.replaceState( + st, + "", + urlWithParams({ + ...closedChannelParams(), + ...ytSearchParams(ytSearchRef.current, ytCountRef.current), + }), + ); } - }, []); + }, [ytSearchRef, ytCountRef]); const enterYtSearch = useCallback((q: string) => { setYtSearch(q); @@ -230,11 +249,11 @@ export function NavigationProvider({ children }: { children: ReactNode }) { // one, or there is no entry of this document below it (a full tab, see backStaysInDocument). if (window.history.state?._yt && backStaysInDocument()) window.history.back(); else { - setYtSearch(null); + endYtSearch(); const { _yt: _q, ...st } = window.history.state ?? {}; window.history.replaceState(st, "", urlWithParams(ytSearchParams(null, DEFAULT_YT_COUNT))); } - }, []); + }, [endYtSearch]); // Leaving the feed ends any live YouTube search, so coming back shows the normal feed. Adjusted // DURING the render that changes the page, against the page we were last on: as an effect it ran @@ -244,7 +263,10 @@ export function NavigationProvider({ children }: { children: ReactNode }) { const [pageSearchWasOn, setPageSearchWasOn] = useState(page); if (pageSearchWasOn !== page) { setPageSearchWasOn(page); - if (page !== "feed") setYtSearch(null); + if (page !== "feed") { + setYtSearch(null); + setYtAsked(null); + } } // The browser tab names the place and the object in it (DESIGN § Composition, "Where am I"): @@ -286,8 +308,8 @@ export function NavigationProvider({ children }: { children: ReactNode }) { // page survives F5. const st = window.history.state || {}; // Mirror the resolved initial page to the address bar (the feed too) so a page reached from the - // stored landing pref — not just from the URL — is itself bookmarkable. urlWithParams preserves any - // ?chan / ?v / ?pl / filter params the loaders already read. + // stored landing pref — not just from the URL — is itself bookmarkable. urlWithParams keeps the + // shell's keys (?chan, a player's) and the page's own; other pages' go (below). // A retired page's link — or the remembered last page, when that is what opened — also opens its // part of the page that replaced it (an `at` tab). const linked = new URLSearchParams(window.location.search).get("page"); @@ -325,9 +347,12 @@ export function NavigationProvider({ children }: { children: ReactNode }) { setPageState(p); setAccountRaw(PAGE_KEY, p); // The YouTube search is read from the address the browser restored, so Back/Forward opens or - // clears it to match the entry we landed on — not asked for: an API search waits for a press. + // clears it to match the entry we landed on. One landed on without it has ended: a Forward back + // onto it is not asked for, and an API search waits for a press. const here = new URLSearchParams(window.location.search); - setYtSearch(p === "feed" ? ytSearchFrom(here) : null); + const landed = p === "feed" ? ytSearchFrom(here) : null; + setYtSearch(landed); + if (landed == null) setYtAsked(null); setYtCountState(ytCountFrom(here)); // The channel page rides in history.state._chan, with a ?chan URL fallback for a fresh bookmark // entry (URL-only, no _chan) — reading e.state alone would wrongly close the channel when a diff --git a/frontend/src/features/navigation/NavigationProvider.ytSearch.dom.test.tsx b/frontend/src/features/navigation/NavigationProvider.ytSearch.dom.test.tsx index a610ed4a..353f402e 100644 --- a/frontend/src/features/navigation/NavigationProvider.ytSearch.dom.test.tsx +++ b/frontend/src/features/navigation/NavigationProvider.ytSearch.dom.test.tsx @@ -188,6 +188,57 @@ describe("NavigationProvider · the YouTube search in the address", () => { expect(search().get("ytn")).toBeNull(); }); + it("a search left by Back and reached again by Forward is not asked for", () => { + const { result } = render(); + act(() => result.current.enterYtSearch("lofi")); + expect(result.current.ytSearchAsked).toBe(true); + window.history.replaceState({ sfPage: "feed" }, "", "/?page=feed&q=lofi"); + act(() => { + window.dispatchEvent(new PopStateEvent("popstate", { state: window.history.state })); + }); + window.history.replaceState({ sfPage: "feed", _yt: "lofi" }, "", "/?page=feed&yt=lofi"); + act(() => { + window.dispatchEvent(new PopStateEvent("popstate", { state: window.history.state })); + }); + expect(result.current.ytSearch).toBe("lofi"); + expect(result.current.ytSearchAsked).toBe(false); + }); + + it("a search left in place is not asked for when it is opened again by a link", () => { + window.history.replaceState(null, "", "/?page=feed&yt=lofi"); + const { result } = render(); + act(() => result.current.askYtSearch()); + stubNoEntryBelow(); + act(() => result.current.exitYtSearch()); + window.history.replaceState({ sfPage: "feed" }, "", "/?page=feed&yt=lofi"); + act(() => { + window.dispatchEvent(new PopStateEvent("popstate", { state: window.history.state })); + }); + expect(result.current.ytSearchAsked).toBe(false); + }); + + it("the Feed press over a channel opened from the search opens the plain feed", () => { + const { result } = render(); + act(() => result.current.enterYtSearch("lofi")); + act(() => result.current.openChannel("UC1")); + act(() => result.current.setPage("feed")); + expect(result.current.channelView).toBeNull(); + expect(result.current.ytSearch).toBeNull(); + expect(search().get("yt")).toBeNull(); + }); + + it("a channel closed in place over the search names the search again", () => { + const { result } = render(); + act(() => result.current.enterYtSearch("lofi")); + act(() => result.current.setYtCount(40)); + act(() => result.current.openChannel("UC1")); + expect(search().get("yt")).toBeNull(); + stubNoEntryBelow(); + act(() => result.current.closeChannel()); + expect(result.current.ytSearch).toBe("lofi"); + expect([search().get("yt"), search().get("ytn")]).toEqual(["lofi", "40"]); + }); + it("a page switch takes the search off the address", () => { const { result } = render(); act(() => result.current.enterYtSearch("lofi")); diff --git a/frontend/src/features/navigation/moduleRegistry.ts b/frontend/src/features/navigation/moduleRegistry.ts index 82e55d74..aa84d5ef 100644 --- a/frontend/src/features/navigation/moduleRegistry.ts +++ b/frontend/src/features/navigation/moduleRegistry.ts @@ -55,6 +55,10 @@ const ALL_URL_KEYS: readonly string[] = ALL.flatMap((m) => m.urlKeys); export function moduleUrlKeys(): readonly string[] { return ALL_URL_KEYS; } +/** Every module's address-bar keys that carry typed text — what the client log keeps out (main.tsx). */ +export function typedUrlKeys(): readonly string[] { + return ALL.flatMap((m) => m.typedUrlKeys ?? []); +} export function foreignUrlKeys(page: Page): readonly string[] { return ALL.filter((m) => m.id !== page).flatMap((m) => m.urlKeys); } diff --git a/frontend/src/features/player/useOpenLinkedPlayer.ts b/frontend/src/features/player/useOpenLinkedPlayer.ts index 2f4e7a3e..4731b4a2 100644 --- a/frontend/src/features/player/useOpenLinkedPlayer.ts +++ b/frontend/src/features/player/useOpenLinkedPlayer.ts @@ -4,43 +4,58 @@ import { useQuery } from "@tanstack/react-query"; import { api, type Video } from "../../lib/api"; import { qk } from "../../lib/queryKeys"; import { linkedPlayer, PLAYER_KEYS } from "../../lib/playerUrlState"; -import { dropFromAddress } from "../../lib/urlState"; +import { clearParams } from "../../lib/urlState"; import { notifyActionError } from "../../lib/actionErrors"; /** Opens the player an address names (a reload, a bookmark — lib/playerUrlState) on the page that * hosts it, once: read as the host mounts, opened once its list is in (`ready`). From the list when the * item is in it (`find`), so the player plays on through the list; else alone, its row fetched from - * the catalog. A row the catalog no longer has is said, not swallowed. */ + * the catalog. A row the catalog no longer has is said, not swallowed. + * + * `scope` names the list the address put it over (a playlist, a live search): when the host moves to + * another before it opened, the address under it named something else, and the player is given up — + * opened later, it would start over a list nobody linked, unasked. */ export function useOpenLinkedPlayer( ready: boolean, find: (id: string) => Video | undefined, open: (video: Video, inList: boolean) => void, + scope: string | number | null = null, ): void { const { t } = useTranslation(); const [linked] = useState(() => linkedPlayer()?.watch ?? null); + const [linkedScope] = useState(scope); // Opened (or said to be gone) once; a ref, as nothing on screen reads it. const done = useRef(linked == null); - const listed = ready && linked != null ? find(linked) : undefined; + const moved = scope !== linkedScope; + const listed = ready && !moved && linked != null ? find(linked) : undefined; const row = useQuery({ queryKey: qk.videoRow(linked ?? ""), queryFn: () => api.videoRow(linked!), - enabled: ready && linked != null && !listed, + enabled: ready && !moved && linked != null && !listed, retry: false, }); const openIt = useEffectEvent(open); const failed = useEffectEvent((err: unknown) => { notifyActionError(err, t("player.linkedFailed")); - dropFromAddress(PLAYER_KEYS); + clearParams(...PLAYER_KEYS); }); useEffect(() => { + if (done.current) return; + if (moved) { + done.current = true; + // The address goes on naming it under the new list (a reload would open it there) — unless a + // player opened since names its own. + if (linkedPlayer()?.watch === linked) clearParams(...PLAYER_KEYS); + return; + } // Not before the host is ready: the row's cache entry is shared, so another opener's fetch fills // it here too (a disabled query still reads the cache) — the shell's player over a page that // plays its own would open a second one. - if (done.current || !ready) return; + if (!ready) return; if (listed) openIt(listed, true); else if (row.data) openIt(row.data, false); else if (row.isError) failed(row.error); else return; done.current = true; - }, [ready, listed, row.data, row.isError, row.error]); + }, [linked, moved, ready, listed, row.data, row.isError, row.error]); } diff --git a/frontend/src/i18n/locales/en/feed.json b/frontend/src/i18n/locales/en/feed.json index ce0941ad..1572832c 100644 --- a/frontend/src/i18n/locales/en/feed.json +++ b/frontend/src/i18n/locales/en/feed.json @@ -76,8 +76,8 @@ "cleared_one": "Cleared {{count}} result", "cleared_other": "Cleared {{count}} results", "showHint": "How many results to fetch at a time — “Load more” pulls another batch of this size.", - "notRunTitle": "This search hasn't run again", - "notRunBody": "A YouTube API search spends 100 units of the shared daily quota, so a reload or a link doesn't start one by itself.", + "notRunTitle": "Not searched yet", + "notRunBody": "A YouTube API search spends 100 units of the shared daily quota, so a reload, a link or Back doesn't start one by itself.", "runAgain": "Search YouTube again" } } diff --git a/frontend/src/i18n/locales/hu/feed.json b/frontend/src/i18n/locales/hu/feed.json index 111fe5b2..ca264e61 100644 --- a/frontend/src/i18n/locales/hu/feed.json +++ b/frontend/src/i18n/locales/hu/feed.json @@ -76,8 +76,8 @@ "cleared_one": "{{count}} találat törölve", "cleared_other": "{{count}} találat törölve", "showHint": "Hány találatot szerezzen egyszerre — a „Továbbiak betöltése” ennyivel tölt tovább.", - "notRunTitle": "Ez a keresés nem futott újra", - "notRunBody": "Egy YouTube API-keresés 100 egységet fogyaszt a közös napi kvótából, ezért újratöltés vagy link nem indít el egyet magától.", + "notRunTitle": "Ez a keresés még nem futott le", + "notRunBody": "Egy YouTube API-keresés 100 egységet fogyaszt a közös napi kvótából, ezért újratöltéskor, linkről vagy visszalépéskor nem indul el magától.", "runAgain": "Keresés újra a YouTube-on" } } diff --git a/frontend/src/i18n/locales/hu/files.json b/frontend/src/i18n/locales/hu/files.json index 727225a0..a6deff3d 100644 --- a/frontend/src/i18n/locales/hu/files.json +++ b/frontend/src/i18n/locales/hu/files.json @@ -1,5 +1,5 @@ { - "linkedGone": "A címben szereplő fájl már nincs ebben a listában", + "linkedGone": "A linkben szereplő fájl már nincs ebben a listában", "navLabel": "Fájlok", "searchPlaceholder": "Keresés a fájlok között…", "count_one": "{{count}} elem", diff --git a/frontend/src/i18n/locales/hu/player.json b/frontend/src/i18n/locales/hu/player.json index e10bf28d..c0afd43a 100644 --- a/frontend/src/i18n/locales/hu/player.json +++ b/frontend/src/i18n/locales/hu/player.json @@ -1,6 +1,6 @@ { "loading": "Betöltés…", - "linkedFailed": "A címben szereplő videó nem nyitható meg", + "linkedFailed": "A linkben szereplő videó nem nyitható meg", "backToOriginal": "Vissza az eredeti videóhoz", "back": "Vissza", "previous": "Előző", diff --git a/frontend/src/lib/adminUsersTab.ts b/frontend/src/lib/adminUsersTab.ts index 5c93cd33..8fbc307c 100644 --- a/frontend/src/lib/adminUsersTab.ts +++ b/frontend/src/lib/adminUsersTab.ts @@ -10,6 +10,11 @@ import { LS, setTabRaw } from "./storage"; export const ADMIN_USERS_TAB_KEY = LS.adminUsersTab; // The address-bar key the page mirrors its tab to. export const USERS_TAB_URL_KEY = "utab"; + +/** The address-bar delta for the Users tab shown. */ +export function usersTabParams(tab: string | null): Record { + return { [USERS_TAB_URL_KEY]: tab }; +} const ADMIN_USERS_ACCESS_TAB = "access"; export function focusAccessRequestsTab(): void { diff --git a/frontend/src/lib/configTab.ts b/frontend/src/lib/configTab.ts index 897fc28f..d6342031 100644 --- a/frontend/src/lib/configTab.ts +++ b/frontend/src/lib/configTab.ts @@ -1,3 +1,8 @@ // The Configuration page's address-bar key: the config group its tab rail shows. Here, beside the other // pages' keys, so the page's definition can declare it without importing the lazy page. export const CONFIG_TAB_KEY = "cfgtab"; + +/** The address-bar delta for the config group shown (null: none named). */ +export function configTabParams(tab: string | null): Record { + return { [CONFIG_TAB_KEY]: tab }; +} diff --git a/frontend/src/lib/filesUrlState.ts b/frontend/src/lib/filesUrlState.ts index 073e3efd..df8265e5 100644 --- a/frontend/src/lib/filesUrlState.ts +++ b/frontend/src/lib/filesUrlState.ts @@ -1,7 +1,7 @@ // The Files page's address-bar keys: the folder shown (`?folder=`, its encoding in // modules/files/filesContext), the file it turns to once (`?item=`), the name search (`?fq=`) and the // sort (`?fsort=`, always written, lib/columnSort's `none` for the list's own order). Here, beside the -// other pages' key lists, so the page's definition can declare them without importing its own folder. +// other pages' codecs (lib/UrlState), where the URL contract test reads them all. export const FILES_FOLDER_KEY = "folder"; export const FILES_ITEM_KEY = "item"; export const FILES_SEARCH_KEY = "fq"; @@ -21,6 +21,28 @@ export const FILES_KEYS = [ export type FilesOpenFile = { open: "view" | "play"; id: number }; +/** The address-bar delta for the list (the page's store): its search, folder and the file it turns to + * — each null when not named (`null`: the page is not what the address shows). */ +export function filesListParams( + list: { search: string; folder: string | null; item: number | null } | null, +): Record { + return { + [FILES_SEARCH_KEY]: list?.search || null, + [FILES_FOLDER_KEY]: list?.folder ?? null, + [FILES_ITEM_KEY]: list?.item != null ? String(list.item) : null, + }; +} + +/** The address-bar delta for the list's sort (lib/columnSort's encoding, `none` included). */ +export function filesSortParams(sort: string | null): Record { + return { [FILES_SORT_KEY]: sort }; +} + +/** What a file open over the list names in its own entry: the viewer's, or the player's. */ +export function filesOpenParams(file: FilesOpenFile): Record { + return { [file.open === "play" ? FILES_PLAY_KEY : FILES_VIEW_KEY]: String(file.id) }; +} + /** The file an address opens over the list (a reload, a bookmark), or null. */ export function openFileFrom(params: URLSearchParams): FilesOpenFile | null { for (const [open, key] of [ diff --git a/frontend/src/lib/history.reloadedNested.dom.test.tsx b/frontend/src/lib/history.reloadedNested.dom.test.tsx new file mode 100644 index 00000000..0fe565c9 --- /dev/null +++ b/frontend/src/lib/history.reloadedNested.dom.test.tsx @@ -0,0 +1,47 @@ +// @vitest-environment jsdom +// +// A reload under two overlays (a dialog, or a maximised player, over the player) loads on the upper +// one's entry; only the player reopens, and takes that entry over. The lower one's entry is still +// under it, and nothing reopens there: closing the player steps back over it too, so one close +// lands on the page — not on a dead step that looks like it. Its own file: the entry is read as the +// module loads, so it is set up before the import. +import { describe, expect, it } from "vitest"; +import { act, renderHook } from "@testing-library/react"; + +describe("an overlay reopened on the entry a reload under two started from", () => { + it("closes onto the page under both", async () => { + window.history.replaceState({ sfPage: "feed" }, "", "/?page=feed"); + window.history.pushState({ sfPage: "feed", _ov: 1, _ovd: 1 }, "", "/?page=feed&watch=v1"); + window.history.pushState({ sfPage: "feed", _ov: 2, _ovd: 2 }, "", "/?page=feed&watch=v1"); + const depth = window.history.length; + const { useBackToClose } = await import("./history"); + + const holder: { unmount?: () => void } = {}; + const r = renderHook(() => useBackToClose(() => holder.unmount?.(), { watch: "v1" })); + holder.unmount = r.unmount; + await act(async () => { + await Promise.resolve(); + }); + expect(window.history.length).toBe(depth); + + await act(async () => { + const landed = new Promise((res) => window.addEventListener("popstate", res, { once: true })); + r.unmount(); + await landed; + await Promise.resolve(); + }); + expect(window.history.state?._ov).toBeUndefined(); + expect(window.location.search).toBe("?page=feed"); + }); + + it("a pushed overlay entry records how many it stands on", async () => { + window.history.replaceState({ sfPage: "feed" }, "", "/?page=feed"); + const { useBackToClose } = await import("./history"); + const r = renderHook(() => useBackToClose(() => {})); + await act(async () => { + await Promise.resolve(); + }); + expect(window.history.state?._ovd).toBe(1); + r.unmount(); + }); +}); diff --git a/frontend/src/lib/history.reloadedOverlayNav.dom.test.tsx b/frontend/src/lib/history.reloadedOverlayNav.dom.test.tsx new file mode 100644 index 00000000..bb172201 --- /dev/null +++ b/frontend/src/lib/history.reloadedOverlayNav.dom.test.tsx @@ -0,0 +1,40 @@ +// @vitest-environment jsdom +// +// The entry a reload under an open dialog started from carries the dialog's mark, and a dialog that +// does not reopen (About, the cheat sheet) leaves it there. A view pushed from that entry (a channel +// page, a search, a sub-view) must not inherit the mark: the first overlay over it took the view's +// entry over as its own, and closing it closed the view too. Its own file: the entry is read as the +// module loads, so it is set up before the import. +import { describe, expect, it } from "vitest"; +import { act, renderHook } from "@testing-library/react"; + +describe("a view pushed from the entry a reload under a dialog started from", () => { + it("is not taken over by the first overlay over it", async () => { + window.history.replaceState({ sfPage: "feed" }, "", "/?page=feed"); + window.history.pushState({ sfPage: "feed", _ov: 3 }, "", "/?page=feed"); + const { rememberNavPosition, useBackToClose } = await import("./history"); + + // openChannel's push, as it spreads the entry it leaves. + const left = rememberNavPosition(); + expect(left._ov).toBeUndefined(); + window.history.pushState({ ...left, _chan: "UC1" }, "", "/?chan=UC1"); + const depth = window.history.length; + + const holder: { unmount?: () => void } = {}; + const r = renderHook(() => useBackToClose(() => holder.unmount?.(), { watch: "v1" })); + holder.unmount = r.unmount; + await act(async () => { + await Promise.resolve(); + }); + expect(window.history.length).toBe(depth + 1); + + await act(async () => { + const landed = new Promise((res) => window.addEventListener("popstate", res, { once: true })); + r.unmount(); + await landed; + await Promise.resolve(); + }); + expect(window.history.state?._chan).toBe("UC1"); + expect(window.location.search).toBe("?chan=UC1"); + }); +}); diff --git a/frontend/src/lib/history.ts b/frontend/src/lib/history.ts index 6df78b70..48e7e1b5 100644 --- a/frontend/src/lib/history.ts +++ b/frontend/src/lib/history.ts @@ -87,7 +87,10 @@ if (typeof window !== "undefined") { /** Call right before a pushState that opens a new view over the current entry (a page, a sub-view, a * channel, a tab). Returns the current state WITHOUT the stamp: a push that spreads the current state - * must spread this, so the new entry does not inherit a position it was never left from. */ + * must spread this, so the new entry does not inherit a position it was never left from — nor an + * overlay's marks, which belong to the entry being left (an entry a reload under an open dialog + * started from carries them, and the first overlay over a view pushed from it took that view's entry + * over for its own). */ export function rememberNavPosition(): Record { // A restore still waiting for the last landing's lists belongs to the page being left: left // running, it would focus "list N, item M" of whatever page this push opens. @@ -95,7 +98,8 @@ export function rememberNavPosition(): Record { const { sfNav: _stale, ...st } = (window.history.state ?? {}) as Record; const pos = navPosition(); window.history.replaceState(pos ? { ...st, sfNav: pos } : st, ""); - return st; + const { _ov: _overlay, _ovd: _depth, ...view } = st; + return view; } // --- Is the entry a back() lands on still ours? -------------------------------------------- @@ -111,12 +115,12 @@ interface NavigationLike { entries: () => readonly { sameDocument: boolean }[]; } -/** Would one more history.back() still land in this document? */ -export function backStaysInDocument(): boolean { +/** Would `steps` more steps back (one, a history.back()) still land in this document? */ +export function backStaysInDocument(steps = 1): boolean { const nav = (window as { navigation?: NavigationLike }).navigation; const at = nav?.currentEntry?.index; if (nav == null || at == null) return true; - return nav.entries()[at - pendingProgrammatic - 1]?.sameDocument === true; + return nav.entries()[at - pendingSteps - steps]?.sameDocument === true; } // --- Sub-views (value carried in history.state._sub) --------------------------------------- @@ -292,10 +296,36 @@ const nextOverlayToken = () => ++overlayCounter; let historyDepth = 0; // The entry this document was loaded on, when it was an overlay's (a reload under an open player): // the first overlay to open takes it over rather than pushing another over it — pushed, Back would -// close the player onto that same page again, one dead step per reload. Until the first push or Back. +// close the player onto that same page again, one dead step per reload. Until the first push or Back; +// a view pushed from it does not carry its mark (rememberNavPosition). `_ovd` on an overlay's entry is how many overlay +// entries it stood on, its own included: a reload under two (a dialog, or a maximised player, over +// the player) leaves the lower one's entry under the one taken over, and closing the taken-over +// overlay steps back over those too (`deadBelow`) — one Back closes it, as it opened. let reloadedOverlayEntry: unknown = window.history.state?._ov ?? null; -// Programmatic history.back() calls still awaiting their popstate — each must be ignored. +const reloadedDepth: unknown = window.history.state?._ovd; +let deadBelow = 0; +// Programmatic history steps back still awaiting their popstate — each must be ignored — and how +// many entries they step over in all (a step over dead entries is one traversal of several). let pendingProgrammatic = 0; +let pendingSteps = 0; +const pendingStepCounts: number[] = []; +function stepBack(steps: number): void { + pendingProgrammatic++; + pendingSteps += steps; + pendingStepCounts.push(steps); + if (steps === 1) window.history.back(); + else window.history.go(-steps); +} +function landedStepBack(): void { + pendingProgrammatic--; + pendingSteps -= pendingStepCounts.shift() ?? 1; +} +// The overlay at the bottom of the stack closing: back over its entry and any dead ones under it. +function stepsToClose(): number { + const steps = historyDepth === 0 ? 1 + deadBelow : 1; + if (historyDepth === 0) deadBelow = 0; + return steps; +} let reconcileScheduled = false; let listenerInstalled = false; // Re-checked after every reconcile — see `afterClosing`. @@ -314,19 +344,27 @@ function reconcileHistory(): void { const here = window.history.state?._ov; if (historyDepth === 1 && here != null && here === reloadedOverlayEntry) { reloadedOverlayEntry = null; + if (typeof reloadedDepth === "number" && reloadedDepth > 1) deadBelow = reloadedDepth - 1; continue; } reloadedOverlayEntry = null; - window.history.pushState({ ...window.history.state, _ov: nextOverlayToken() }, ""); + window.history.pushState( + { ...window.history.state, _ov: nextOverlayToken(), _ovd: historyDepth + deadBelow }, + "", + ); } while (historyDepth > want) { historyDepth--; - if (backStaysInDocument()) { - pendingProgrammatic++; - window.history.back(); - } else { + const steps = stepsToClose(); + if (backStaysInDocument(steps)) stepBack(steps); + else if (steps > 1 && backStaysInDocument()) stepBack(1); + else { // Nothing of ours below to step back to (see backStaysInDocument): the entry stays, unmarked. - const { _ov: _drop, ...st } = (window.history.state ?? {}) as Record; + const { + _ov: _drop, + _ovd: _depth, + ...st + } = (window.history.state ?? {}) as Record; window.history.replaceState(st, ""); } } @@ -368,7 +406,7 @@ function ensureOverlayListener(): void { window.addEventListener("popstate", () => { reloadedOverlayEntry = null; if (pendingProgrammatic > 0) { - pendingProgrammatic--; // our own back(): consume and ignore + landedStepBack(); // our own back(): consume and ignore // The last one landed: an overlay that opened while it was in flight still owes its entry. if (pendingProgrammatic === 0 && historyDepth < overlayStack.length) scheduleReconcile(); else nameOverlaysHere(); @@ -376,7 +414,11 @@ function ensureOverlayListener(): void { } // Genuine user Back: the browser already dropped one overlay entry, so close the topmost // overlay to match. Its unmount cleanup finds itself already off the stack and only re-syncs. + // The last one closing over dead entries (see `deadBelow`) steps over them too. if (historyDepth > 0) historyDepth--; + const dead = historyDepth === 0 ? deadBelow : 0; + if (historyDepth === 0) deadBelow = 0; + if (dead > 0 && backStaysInDocument(dead)) stepBack(dead); const top = overlayStack.pop(); if (top) top.cb(); scheduleReconcile(); diff --git a/frontend/src/lib/logHooks.dom.test.ts b/frontend/src/lib/logHooks.dom.test.ts index 93b697b5..57da00f4 100644 --- a/frontend/src/lib/logHooks.dom.test.ts +++ b/frontend/src/lib/logHooks.dom.test.ts @@ -10,9 +10,12 @@ import { installClientLog } from "./logHooks"; const appCopy = (text: string | null) => ["Refresh", "Frissítés"].includes(text?.trim() ?? "") ? "Refresh" : null; +// The pages' search boxes' keys, as main.tsx hands them over from the module definitions. +const TYPED = ["fq", "yt"]; + beforeAll(() => { - installClientLog(appCopy); - installClientLog(appCopy); // a hot reload's second call must not wrap History twice + installClientLog(appCopy, TYPED); + installClientLog(appCopy, TYPED); // a hot reload's second call must not wrap History twice expect(events().map((e) => e.event)).toEqual(["app.start", "nav.view"]); clear(); }); @@ -56,6 +59,17 @@ describe("client log hooks", () => { }); }); + it("keeps a page's own search box out below TRACE too", () => { + setLevel("DEBUG"); + window.history.pushState({ sfPage: "files" }, "", "/?folder=all&fq=private+words&yt=lofi"); + expect(views()[0]!.attrs!.values).toEqual({ folder: "all" }); + expect(JSON.stringify(views())).not.toContain("private"); + + setLevel("TRACE"); + window.history.pushState({ sfPage: "files" }, "", "/?folder=all&fq=private+words"); + expect(views()[1]!.attrs!.values).toEqual({ folder: "all", fq: "private words" }); + }); + it("records an overlay opening as a view of its own, and Back as a pop", () => { window.history.pushState({ sfPage: "feed", _ov: 1 }, ""); // What Back does: the browser restores the previous entry's state (the unwrapped prototype diff --git a/frontend/src/lib/logHooks.ts b/frontend/src/lib/logHooks.ts index 4c0c90c2..c2168511 100644 --- a/frontend/src/lib/logHooks.ts +++ b/frontend/src/lib/logHooks.ts @@ -14,7 +14,8 @@ import { enabled, errorAttrs, log } from "./log"; import { pathTemplate } from "./http"; // URL parameters that carry what the user TYPED — search text. Logged only at TRACE ("Teljes"). -// Every other parameter is filter/sort/view state: its NAME is kept at INFO, its value at DEBUG. +// Every other parameter is filter/sort/view state: its NAME is kept at INFO, its value at DEBUG. The +// shell's own here; each page's search boxes join them at install (ModuleDef.typedUrlKeys). const TYPED_PARAMS = new Set(["q", "keywords", "search"]); type Via = "start" | "push" | "replace" | "pop"; @@ -168,9 +169,13 @@ let installed = false; /** Install the hooks. Idempotent: a second call (a hot reload) must not wrap History twice. * `appCopy` turns on-screen app copy into its English wording (i18n/appCopy.ts) — passed in, so this * module stays free of the i18n runtime. */ -export function installClientLog(appCopy: AppCopy = () => null): void { +export function installClientLog( + appCopy: AppCopy = () => null, + typedParams: readonly string[] = [], +): void { if (installed) return; installed = true; + for (const k of typedParams) TYPED_PARAMS.add(k); log.info("app.start", "app loaded", { version: FRONTEND_VERSION, diff --git a/frontend/src/lib/moduleDef.ts b/frontend/src/lib/moduleDef.ts index d6471f8e..bf1dd813 100644 --- a/frontend/src/lib/moduleDef.ts +++ b/frontend/src/lib/moduleDef.ts @@ -50,6 +50,10 @@ export type ModuleDef = { // along onto another's URL. Empty for a page with nothing of its own in the URL. Each key belongs to // one module and none is a shell key (moduleUrl.contract.test). urlKeys: readonly string[]; + // Those of them that carry what the user TYPED (a search box's text): the client log keeps their + // values out below its most detailed level, as it does the shell's own (lib/logHooks). A new search + // box's key is declared here with it (moduleUrl.contract.test holds them to urlKeys). + typedUrlKeys?: readonly string[]; // Its page plays YouTube videos in the player and reopens it from an address that names one (a // reload, a bookmark — lib/playerUrlState), with its own list as the queue. A page without it gets // the shell's player for such an address: the video alone. diff --git a/frontend/src/lib/playlistsUrlState.ts b/frontend/src/lib/playlistsUrlState.ts index 553cf16f..3ada9db0 100644 --- a/frontend/src/lib/playlistsUrlState.ts +++ b/frontend/src/lib/playlistsUrlState.ts @@ -20,6 +20,15 @@ export function isStoredSort(s: SortState): boolean { return s?.key === PLAYLIST_STORED_SORT?.key && s?.dir === PLAYLIST_STORED_SORT?.dir; } +/** The address-bar delta for the page's store: the playlist shown and the rail's search (each null + * when not named). */ +export function playlistsParams( + selected: number | null, + search: string, +): Record { + return { pl: selected != null ? String(selected) : null, plq: search || null }; +} + /** The address-bar delta for an open playlist's view; `null` clears it (no playlist open). */ export function playlistViewParamUpdates(v: PlaylistView | null): Record { if (!v) return { plsort: null, plf: null, plgroup: null }; diff --git a/frontend/src/lib/settingsTab.ts b/frontend/src/lib/settingsTab.ts index 74045e15..f5fbece8 100644 --- a/frontend/src/lib/settingsTab.ts +++ b/frontend/src/lib/settingsTab.ts @@ -6,6 +6,11 @@ import { LS, setTabRaw } from "./storage"; // address-bar key the page mirrors its tab to is here too, beside the other pages' keys. export const SETTINGS_TAB_KEY = "stab"; +/** The address-bar delta for the Settings tab shown. */ +export function settingsTabParams(tab: string | null): Record { + return { [SETTINGS_TAB_KEY]: tab }; +} + export type SettingsTabId = | "appearance" | "notifications" diff --git a/frontend/src/lib/urlState.dom.test.ts b/frontend/src/lib/urlState.dom.test.ts index ab87bf44..ca7e1e47 100644 --- a/frontend/src/lib/urlState.dom.test.ts +++ b/frontend/src/lib/urlState.dom.test.ts @@ -127,13 +127,13 @@ describe("a page opened at a path that starts with //", () => { }); describe("filterParamUpdates", () => { - it("maps non-default filters to values and defaults to null (delete)", () => { + it("maps non-default filters to values and other defaults to null (delete)", () => { const f: FeedFilters = { ...DEFAULTS, q: "react", tags: [3, 7], show: "watched" }; const upd = filterParamUpdates(f); expect(upd.q).toBe("react"); expect(upd.tags).toBe("3,7"); expect(upd.show).toBe("watched"); - expect(upd.sort).toBeNull(); // default "newest" is not emitted → cleared + expect(upd.sort).toBe("newest"); // the view and the order are always named, defaults too expect(upd.scope).toBeNull(); }); diff --git a/frontend/src/lib/urlState.ts b/frontend/src/lib/urlState.ts index 5c380810..2c45ae38 100644 --- a/frontend/src/lib/urlState.ts +++ b/frontend/src/lib/urlState.ts @@ -80,13 +80,19 @@ export function filtersToParams(f: FeedFilters): URLSearchParams { } /** Build the address-bar delta for the feed filters: each recognized filter key mapped to its emitted - * value, or null to delete it (so switching a filter back to its default clears the key). Pass `null` - * to clear ALL filter keys — used when leaving the feed, where the localStorage copy still holds them. - * Feed the result into urlWithParams(). */ + * value, or null to delete it (so switching a filter back to its default clears the key) — except the + * view and the order, always written (defaults too): a feed address then always names its view, so a + * bookmark of the default feed opens the default feed, not whatever view the account stored since + * (an address naming none falls back to it). Pass `null` to clear ALL filter keys — used when leaving + * the feed, where the localStorage copy still holds them. Feed the result into urlWithParams(). */ export function filterParamUpdates(f: FeedFilters | null): Record { const p = f ? filtersToParams(f) : new URLSearchParams(); const out: Record = {}; for (const k of KEYS) out[k] = p.get(k); + if (f) { + out.show = f.show || "unwatched"; + out.sort = f.sort || "newest"; + } return out; } @@ -233,13 +239,6 @@ export function urlWithParams(updates: Record): string { return hrefHere(params); } -/** Take `keys` off the current entry's address: what a link named and the page could not show (a file - * or a video that is gone, said once) must not stay to be said again on every reload, or shared. */ -export function dropFromAddress(keys: readonly string[]): void { - const updates = Object.fromEntries(keys.map((k) => [k, null])); - window.history.replaceState(window.history.state, "", urlWithParams(updates)); -} - /** The channel page's address, opened over the page on screen: the shell's own keys (the page, an * entry link not yet read) and the channel — none of the page's keys, none of another channel's view * (the channel page writes its own, lib/channelPageUrlState), and no player (opening a channel closes @@ -269,10 +268,11 @@ export function hrefHere(query: URLSearchParams, hash: string = window.location. return url.pathname.startsWith("//") ? url.href : `${url.pathname}${url.search}${url.hash}`; } -/** Remove specific TRANSIENT params (one-time auth/admin links: ?link, ?oauth, ?admin, ?access, ?verify, - * ?reset) from the address bar once they've been read, WITHOUT touching the nav/filter params that now - * live there. Preserves history.state. Replaces the old whole-query stripUrlParams, which is unsafe now - * that the query is the source of truth for navigation. No-op when none of the keys are present. */ +/** Take `keys` off the current entry's address, WITHOUT touching the nav/filter params that live + * there: TRANSIENT params once read (one-time auth/admin links: ?link, ?oauth, ?admin, ?access, ?verify, + * ?reset), and what a link named that the page could not show (a file or a video that is gone, said + * once) — left, it would be said again on every reload, or shared. Preserves history.state. No-op when + * none of the keys are present. */ export function clearParams(...keys: string[]): void { const params = new URLSearchParams(window.location.search); let changed = false; diff --git a/frontend/src/lib/useUrlMirror.dom.test.ts b/frontend/src/lib/useUrlMirror.dom.test.ts index c0c3f567..c75e92ff 100644 --- a/frontend/src/lib/useUrlMirror.dom.test.ts +++ b/frontend/src/lib/useUrlMirror.dom.test.ts @@ -5,7 +5,7 @@ // so an inline object literal (new identity every render) only fires when a param actually changes; a // null value clears a key; and every other param on the URL is preserved. import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -import { renderHook } from "@testing-library/react"; +import { act, cleanup, renderHook } from "@testing-library/react"; import { useUrlMirror } from "./useUrlMirror"; const setUrl = (search: string) => window.history.replaceState(null, "", search || "/"); @@ -15,7 +15,10 @@ beforeEach(() => { setUrl("/"); spy = vi.spyOn(window.history, "replaceState"); }); -afterEach(() => spy.mockRestore()); +afterEach(() => { + cleanup(); + spy.mockRestore(); +}); describe("useUrlMirror", () => { it("writes the param onto the current entry on mount", () => { @@ -49,6 +52,41 @@ describe("useUrlMirror", () => { expect(window.location.search).toBe("?page=playlists&pl=5"); }); + // An entry above the view's own (a dialog's, a drawer's, a sub-view's) took a change the entry + // under it never saw: Back restores that entry's older address, and a reload would bring the old + // view back over the one on screen. + it("names the view again on the entry Back lands on", async () => { + setUrl("?page=plex&pf="); + const { rerender } = renderHook((p: Record) => useUrlMirror(p), { + initialProps: { pf: "" } as Record, + }); + window.history.pushState({ _ov: 1 }, "", window.location.href); + rerender({ pf: "genres=Drama" }); + expect(window.location.search).toBe("?page=plex&pf=genres%3DDrama"); + await act(async () => { + const landed = new Promise((res) => window.addEventListener("popstate", res, { once: true })); + window.history.back(); + await landed; + await new Promise((res) => setTimeout(res, 0)); + }); + expect(window.history.state).toBeNull(); + expect(window.location.search).toBe("?page=plex&pf=genres%3DDrama"); + }); + + it("writes nothing on a landing that already names the view", async () => { + setUrl("?pl=5"); + renderHook(() => useUrlMirror({ pl: "5" })); + window.history.pushState({ _ov: 1 }, "", window.location.href); + spy.mockClear(); + await act(async () => { + const landed = new Promise((res) => window.addEventListener("popstate", res, { once: true })); + window.history.back(); + await landed; + await new Promise((res) => setTimeout(res, 0)); + }); + expect(spy).not.toHaveBeenCalled(); + }); + it("leaves an unconsumed auth token in the fragment alone", () => { // This hook's mount effect is what ran before (and therefore before the auth card) had even // loaded, so whatever it writes to the address bar is what the card later reads. It must not eat diff --git a/frontend/src/lib/useUrlMirror.ts b/frontend/src/lib/useUrlMirror.ts index 451c1194..21d26d14 100644 --- a/frontend/src/lib/useUrlMirror.ts +++ b/frontend/src/lib/useUrlMirror.ts @@ -1,4 +1,4 @@ -import { useEffect, useEffectEvent } from "react"; +import { useEffect, useEffectEvent, useState } from "react"; import { urlWithParams } from "./urlState"; // Mirror a set of nav params onto the CURRENT history entry (replaceState — mirroring is not a new @@ -6,17 +6,44 @@ import { urlWithParams } from "./urlState"; // value clears that key, and urlWithParams preserves every other param (so providers don't clobber each // other). Extracted from the identical replaceState/urlWithParams blocks FeedFiltersProvider and // PlaylistsProvider used to hand-roll, so the mirroring contract lives in one place. +// +// Also whenever Back/Forward makes another entry current: the browser restores that entry's address, +// which says what the view was when it was left — a change made since on an entry above it (a dialog's +// or a drawer's, a sub-view's) is not in it, and a reload would bring the old view back over the one +// on screen. (One landed on by an overlay's step back that another step will leave is written too, +// and harmlessly: the next landing writes the entry that stays.) export function useUrlMirror(updates: Record): void { // The caller typically passes an inline object literal (new identity every render); key on its // serialized value so the effect only fires when a param actually changes, not every render. const dep = JSON.stringify(updates); - // The object itself is read through an effect event, so the effect's one reactive input is the - // serialized value — which is exactly what the dep list says: an effect event for the identity that - // churns, a primitive for the trigger. + // The object itself is read through an effect event, so the effect's reactive inputs are the + // serialized value and the landing count — which is exactly what the dep list says: an effect event + // for the identity that churns, primitives for the triggers. const mirror = useEffectEvent(() => { - window.history.replaceState(window.history.state, "", urlWithParams(updates)); + const href = urlWithParams(updates); + // Every mirror runs on every landing: one with nothing to change writes nothing (a browser + // throttles a page that rewrites its address too often). + if (href !== urlWithParams({})) window.history.replaceState(window.history.state, "", href); }); + // Counted in state, on a task after the popstate dispatch: every listener of it has run by then — + // a store that reads its keys back from the landed address among them (Activity) — and so has the + // render each one caused (React renders a popstate's updates at once, between listeners). A write + // inside the dispatch put the store's old value over the address a later listener was still to + // read. + const [landings, setLandings] = useState(0); + useEffect(() => { + let timer = 0; + const onPop = () => { + window.clearTimeout(timer); + timer = window.setTimeout(() => setLandings((n) => n + 1), 0); + }; + window.addEventListener("popstate", onPop); + return () => { + window.removeEventListener("popstate", onPop); + window.clearTimeout(timer); + }; + }, []); useEffect(() => { mirror(); - }, [dep]); + }, [dep, landings]); } diff --git a/frontend/src/main.tsx b/frontend/src/main.tsx index 8a439655..ec1d0d50 100644 --- a/frontend/src/main.tsx +++ b/frontend/src/main.tsx @@ -8,6 +8,7 @@ import { ConfirmProvider } from "./components/ui/ConfirmProvider"; import BootGate, { BootScreen } from "./shell/BootGate"; import { NavigationProvider } from "./features/navigation/NavigationProvider"; import { ModuleStores } from "./features/navigation/ModuleStores"; +import { typedUrlKeys } from "./features/navigation/moduleRegistry"; import { PrefsProvider } from "./features/prefs/PrefsProvider"; import { WizardProvider } from "./features/wizard/WizardProvider"; import "./i18n"; @@ -19,7 +20,7 @@ import { installStaleBuildReload } from "./lib/staleBuild"; import { appCopyOf } from "./i18n/appCopy"; // First, before any render: a crash or a navigation during boot belongs in the log too. -installClientLog(appCopyOf); +installClientLog(appCopyOf, typedUrlKeys()); // A tab from before the last release reloads onto the new build instead of crashing on a lazy chunk. installStaleBuildReload(); diff --git a/frontend/src/moduleSearch.url.dom.test.tsx b/frontend/src/moduleSearch.url.dom.test.tsx index ba1a4342..7c46c42b 100644 --- a/frontend/src/moduleSearch.url.dom.test.tsx +++ b/frontend/src/moduleSearch.url.dom.test.tsx @@ -4,7 +4,7 @@ // the list searched (Channels, Files and Playlists kept it in memory only, Plex in this browser only). // The real navigation and the real stores, read through their hooks; only the network is absent. import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { cleanup, render, waitFor } from "@testing-library/react"; +import { act, cleanup, render, waitFor } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { useEffect } from "react"; @@ -14,6 +14,10 @@ vi.mock("react-i18next", async (importOriginal) => ({ })); import { NavigationProvider } from "./features/navigation/NavigationProvider"; +import { + useNavigationActions, + type NavigationActions, +} from "./features/navigation/navigationContext"; import { ModuleStores } from "./features/navigation/ModuleStores"; import { useChannels } from "./modules/channels/channelsContext"; import { useFiles } from "./modules/files/filesContext"; @@ -22,13 +26,16 @@ import { usePlex } from "./modules/plex/plexContext"; import { claimLocalData, getAccountRaw, LS, setAccountRaw } from "./lib/storage"; const seen: { current: Record } = { current: {} }; +const nav: { current: NavigationActions | null } = { current: null }; function Probe() { const channels = useChannels(); const files = useFiles(); const playlists = usePlaylists(); const plex = usePlex(); + const actions = useNavigationActions(); useEffect(() => { + nav.current = actions; seen.current = { channels: channels.search, files: files.search, @@ -93,3 +100,20 @@ describe("Plex's search", () => { expect(search().get("pqf")).toBe("people"); }); }); + +// The search is the page's while it is on screen: leaving the page ends it, so a trip back through +// the rail opens the page unsearched (and the address names none). +describe.each([ + ["files", "fq", "notes"], + ["playlists", "plq", "jazz"], +])("%s's name search, once the page is left", (page, key, value) => { + it("is gone when the page is opened again", async () => { + mountAt(`/?page=${page}&${key}=${value}`, page); + await waitFor(() => expect(seen.current[page]).toBe(value)); + act(() => nav.current!.setPage("feed")); + act(() => nav.current!.setPage(page as "files" | "playlists")); + await waitFor(() => expect(seen.current[page]).toBe("")); + await new Promise((r) => setTimeout(r, 350)); // past the address write's typing pause + expect(search().get(key)).toBeNull(); + }); +}); diff --git a/frontend/src/moduleUrl.contract.test.ts b/frontend/src/moduleUrl.contract.test.ts index 8ee94f78..61cc7798 100644 --- a/frontend/src/moduleUrl.contract.test.ts +++ b/frontend/src/moduleUrl.contract.test.ts @@ -3,26 +3,28 @@ // The URL is the view: each module declares the address-bar keys its view writes (`urlKeys`), and the // shell clears them on a page switch and opens a page without another page's keys (NavigationProvider, // derived from these declarations). That only holds while the declarations are true, so this pins -// them: each key belongs to one module and is no shell key, every codec writes only its module's keys, -// and no key literal anywhere in the app reads or writes the address bar undeclared. +// them: each key belongs to one module and is no shell key, the keys a module declares are exactly +// what its encoders write, a writer outside lib/ hands the address an encoder's result (or literal +// keys, checked here) — never a key of its own making — and every key literal read from the live +// query is declared. import { describe, it, expect } from "vitest"; import { readdirSync, readFileSync, statSync } from "node:fs"; import { join, relative } from "node:path"; -import { moduleDef, REGISTERED_PAGES } from "./features/navigation/moduleRegistry"; +import { moduleDef, REGISTERED_PAGES, typedUrlKeys } from "./features/navigation/moduleRegistry"; import { DEFAULT_TAG_MODES, EMPTY_PLEX_FILTERS, type FeedFilters } from "./lib/api"; import { filterParamUpdates, SHELL_KEYS, type Page } from "./lib/urlState"; import { channelsParamUpdates } from "./lib/channelsUrlState"; import { plexParamUpdates, plexSearchParamUpdates } from "./lib/plexUrlState"; import { activityParamUpdates, DEFAULT_ACTIVITY_STATE } from "./lib/activityUrlState"; import { DEFAULT_LOGS_STATE, logsParamUpdates } from "./lib/logsUrlState"; -import { FILES_KEYS } from "./lib/filesUrlState"; +import { filesListParams, filesOpenParams, filesSortParams } from "./lib/filesUrlState"; import { ytSearchParams } from "./lib/ytSearchUrlState"; import { budgetParams } from "./lib/homeUrlState"; import { PLAYER_KEYS, playerParamUpdates } from "./lib/playerUrlState"; -import { playlistViewParamUpdates } from "./lib/playlistsUrlState"; -import { SETTINGS_TAB_KEY } from "./lib/settingsTab"; -import { CONFIG_TAB_KEY } from "./lib/configTab"; -import { USERS_TAB_URL_KEY } from "./lib/adminUsersTab"; +import { playlistsParams, playlistViewParamUpdates } from "./lib/playlistsUrlState"; +import { settingsTabParams } from "./lib/settingsTab"; +import { configTabParams } from "./lib/configTab"; +import { usersTabParams } from "./lib/adminUsersTab"; import { PLEX_SUB_URL } from "./modules/plex/plexSubUrl"; import { MODE_URL } from "./shell/welcomeMode"; import { CHANNEL_PAGE_KEYS, channelPageParamUpdates } from "./lib/channelPageUrlState"; @@ -56,13 +58,17 @@ const WRITES: Partial string[]>> = { ...Object.keys(plexSearchParamUpdates({ q: "x", field: "all" })), PLEX_SUB_URL.key, ], - files: () => [...FILES_KEYS], - settings: () => [SETTINGS_TAB_KEY], - config: () => [CONFIG_TAB_KEY], - users: () => [USERS_TAB_URL_KEY], + files: () => [ + ...Object.keys(filesListParams(null)), + ...Object.keys(filesSortParams(null)), + ...Object.keys(filesOpenParams({ open: "view", id: 1 })), + ...Object.keys(filesOpenParams({ open: "play", id: 1 })), + ], + settings: () => Object.keys(settingsTabParams(null)), + config: () => Object.keys(configTabParams(null)), + users: () => Object.keys(usersTabParams(null)), playlists: () => [ - "pl", - "plq", + ...Object.keys(playlistsParams(null, "")), ...Object.keys(playlistViewParamUpdates({ sort: null, filters: {}, groupBy: false })), ], activity: () => [ @@ -79,25 +85,57 @@ function sources(dir: string): string[] { }); } -// Key literals that reach the address bar's QUERY: the keys of an object literal handed to -// `urlWithParams` / `useUrlMirror`, and a `.get("…")` on the live query. +// The object literals handed to an address writer: `urlWithParams` / `useUrlMirror`, an overlay's +// own keys (ui/Modal's `url`, `useBackToClose`'s). +const WRITER_OBJECTS = [ + /(?:urlWithParams|useUrlMirror)\(\{([^}]*)\}/g, + /\burl=\{\{([^}]*)\}/g, + /useBackToClose\([^,()]+,\s*\{([^}]*)\}/g, +]; + +// Key literals that reach the address bar's QUERY: the literal keys of a writer's object, and a +// `.get("…")` / `.has("…")` in a file that reads the live query (any params object there: the +// two-line form names its params before it reads them). The fragment (`hash`) is not the query. function queryKeyLiterals(): { key: string; where: string }[] { const out: { key: string; where: string }[] = []; for (const file of sources(SRC)) { const text = readFileSync(file, "utf8"); const where = relative(SRC, file).split("\\").join("/"); - for (const m of text.matchAll(/(?:urlWithParams|useUrlMirror)\(\{([^}]*)\}/g)) { - for (const k of `{${m[1]!}`.matchAll(/[{,]\s*(\w+)\s*:/g)) out.push({ key: k[1]!, where }); + for (const re of WRITER_OBJECTS) { + for (const m of text.matchAll(re)) { + for (const k of `{${m[1]!}`.matchAll(/[{,]\s*(\w+)\s*:/g)) out.push({ key: k[1]!, where }); + } } - for (const m of text.matchAll( - /new URLSearchParams\(window\.location\.search\)\.get\("(\w+)"\)/g, - )) { - out.push({ key: m[1]!, where }); + if (!text.includes("location.search")) continue; + for (const m of text.matchAll(/(\b\w+|\))\.(?:get|has)\("(\w+)"\)/g)) { + if (m[1] !== "hash") out.push({ key: m[2]!, where }); } } return out; } +// A key a writer computes itself (`[KEY]: …`) is one no scan can read: outside lib/ (the codecs, and +// the history plumbing they feed) a writer hands the address an encoder's result instead, and the +// encoder is what WRITES calls. +function computedWriterKeys(inLib = false): string[] { + const out: string[] = []; + for (const file of sources(SRC)) { + const where = relative(SRC, file).split("\\").join("/"); + if (where.startsWith("lib/") !== inLib) continue; + const text = readFileSync(file, "utf8"); + for (const re of WRITER_OBJECTS) { + for (const m of text.matchAll(re)) { + if (/[{,]\s*\[/.test(`{${m[1]!}`)) out.push(`${where}: ${m[0]!.slice(0, 60)}`); + } + } + } + return out; +} + +// A retired page's link keys: read once to open the page that replaced it, and cleared (lib/urlState +// RETIRED) — no module writes them any more. +const RETIRED_LINK_KEYS = ["lu", "lq", "lwin"]; + describe("module URL keys", () => { const owner = new Map(); for (const page of REGISTERED_PAGES) { @@ -154,14 +192,62 @@ describe("module URL keys", () => { it("cover every key literal the app reads or writes in the address bar", () => { const shell: readonly string[] = SHELL_KEYS; const undeclared = queryKeyLiterals().filter( - ({ key }) => !owner.has(key) && !shell.includes(key), + ({ key }) => !owner.has(key) && !shell.includes(key) && !RETIRED_LINK_KEYS.includes(key), ); expect(undeclared).toEqual([]); }); it("the literal scan sees the call sites it is meant to (a regex that matched nothing would pass)", () => { const seen = new Set(queryKeyLiterals().map(({ key }) => key)); - for (const key of ["pl", "page", "rjob", "link"]) expect(seen).toContain(key); + // A writer's literal, a one-line read, a two-line read, a retired link's. + for (const key of ["page", "rjob", "link", "plq", "folder", "lq"]) expect(seen).toContain(key); + }); + + it("are written outside lib/ only through the codecs that WRITES calls", () => { + expect(computedWriterKeys()).toEqual([]); + }); + + it("the computed-key scan sees what it is meant to (lib's own writers compute theirs)", () => { + expect(computedWriterKeys(true).some((w) => w.startsWith("lib/history.ts"))).toBe(true); + }); + + // What a search box puts in the address is what the user typed: the client log keeps it out below + // its most detailed level (lib/logHooks), from these declarations. + it("declare their typed-text keys among their own", () => { + for (const page of REGISTERED_PAGES) { + const def = moduleDef(page); + expect( + (def.typedUrlKeys ?? []).filter((k) => !def.urlKeys.includes(k)), + page, + ).toEqual([]); + } + expect([...typedUrlKeys()].sort()).toEqual(["aq", "cq", "fq", "plq", "pq", "q", "yt"]); + // …which the client log is handed as it installs. + expect(readFileSync(join(SRC, "main.tsx"), "utf8")).toContain( + "installClientLog(appCopyOf, typedUrlKeys());", + ); + }); +}); + +// The bookmark net (e2e/bookmark.spec.ts) opens every page that names its view, through the rail or +// by its address — Plex excepted, by name there: its view is the instance's own library. +describe("the bookmark net", () => { + const spec = readFileSync(join(SRC, "..", "e2e", "bookmark.spec.ts"), "utf8"); + const opened = (page: Page) => + spec.includes(`page=${page}`) || + spec.includes(`gotoPage(page, "${page}")`) || + spec.includes(`gotoAdminPage(page, "${page}")`); + + it.each(REGISTERED_PAGES.filter((p) => moduleDef(p).urlKeys.length > 0 && p !== "plex"))( + "takes a round trip through %s", + (page) => { + expect(opened(page)).toBe(true); + }, + ); + + it("names the page it leaves out", () => { + expect(opened("plex")).toBe(false); + expect(spec).toContain("Not Plex:"); }); }); diff --git a/frontend/src/modules/activity/Activity.dom.test.tsx b/frontend/src/modules/activity/Activity.dom.test.tsx index e2883d37..11b2f66c 100644 --- a/frontend/src/modules/activity/Activity.dom.test.tsx +++ b/frontend/src/modules/activity/Activity.dom.test.tsx @@ -297,7 +297,9 @@ describe("Activity", () => { it("Back/Forward on the server log reads only the filters of the entry it lands on", async () => { // A browser runs the microtasks between two popstate listeners, so each listener's update - // renders on its own; a test dispatch does not. Each listener runs in its own act here. + // renders on its own; a test dispatch does not. Each listener runs in its own act here — a + // synchronous one: a browser runs no TASK between them (an async act let a timer set by one + // listener fire before the next). const listeners: EventListener[] = []; const addOther = window.addEventListener.bind(window); const removeOther = window.removeEventListener.bind(window); @@ -316,7 +318,7 @@ describe("Activity", () => { // Forward onto a pivot's entry: one run's lines over the log's whole retention. window.history.pushState(null, "", "/?page=activity&at=logs&aw=90d&lrun=41"); for (const l of [...listeners]) { - await act(async () => l(new PopStateEvent("popstate"))); + act(() => l(new PopStateEvent("popstate"))); } await waitFor(() => expect(logsMock).toHaveBeenLastCalledWith(expect.objectContaining({ run_id: 41 }), null), diff --git a/frontend/src/modules/activity/module.ts b/frontend/src/modules/activity/module.ts index fa95fe6b..2a27cf5c 100644 --- a/frontend/src/modules/activity/module.ts +++ b/frontend/src/modules/activity/module.ts @@ -14,4 +14,5 @@ export const activityModule: ModuleDef = { gate: (me) => me.role === "admin", Page: lazyWithPreload(() => import("./Activity")), urlKeys: ACTIVITY_KEYS, + typedUrlKeys: ["aq"], }; diff --git a/frontend/src/modules/channels/module.ts b/frontend/src/modules/channels/module.ts index 93b63b8e..fd411622 100644 --- a/frontend/src/modules/channels/module.ts +++ b/frontend/src/modules/channels/module.ts @@ -15,4 +15,5 @@ export const channelsModule: ModuleDef = { maxWidth: "96rem", Store: ChannelsProvider, urlKeys: CHANNELS_KEYS, + typedUrlKeys: ["cq"], }; diff --git a/frontend/src/modules/config/ConfigPanel.tsx b/frontend/src/modules/config/ConfigPanel.tsx index 3727db4a..f183596b 100644 --- a/frontend/src/modules/config/ConfigPanel.tsx +++ b/frontend/src/modules/config/ConfigPanel.tsx @@ -16,7 +16,7 @@ import SegmentedToggle from "../../components/ui/SegmentedToggle"; import FilterSelect from "../../components/ui/FilterSelect"; import { DraftSaveBar, type SaveState } from "../../components/ui/DraftSaveBar"; import { LS, useTabPersistedState } from "../../lib/storage"; -import { CONFIG_TAB_KEY } from "../../lib/configTab"; +import { CONFIG_TAB_KEY, configTabParams } from "../../lib/configTab"; import { useUrlMirror } from "../../lib/useUrlMirror"; import { notifyActionError } from "../../lib/actionErrors"; import { Button } from "../../components/ui/Button"; @@ -192,7 +192,7 @@ export default function ConfigPanel() { const activeGroup = groupKeys.includes(tab) ? tab : groupKeys[0]; // The address names the group on screen (always: see SettingsPanel); until the registry is in, the // one it was opened on. - useUrlMirror({ [CONFIG_TAB_KEY]: (data ? activeGroup : tab) || null }); + useUrlMirror(configTabParams((data ? activeGroup : tab) || null)); if (q.isError && !data) { return ( diff --git a/frontend/src/modules/feed/Feed.tsx b/frontend/src/modules/feed/Feed.tsx index baf69e7a..83d85d7b 100644 --- a/frontend/src/modules/feed/Feed.tsx +++ b/frontend/src/modules/feed/Feed.tsx @@ -503,6 +503,10 @@ export default function Feed({ const saved = savedOverrides[v.id]; return saved && !(live && saved.answered) ? { ...v, saved: saved.value } : v; }); + // The live search's cards, in the order picked for them, and the player's queue over them. + const ytShown = (rows: Video[]): Video[] => + sortYtResults(withOverrides(rows, true), ytSort.key as SortKey, ytSort.dir, lang); + const ytQueue = (rows: Video[]): Video[] => ytShown(rows).filter((v) => v.status !== "hidden"); const merged: Video[] = withOverrides(loaded); const items: Video[] = merged.filter((v) => matchesView(v.status, filters.show)); // The in-app player's queue (auto-advance / loop / prev-next). It's the filtered feed, but the @@ -511,16 +515,18 @@ export default function Feed({ // goes hidden still drops out ("visible affects"). const playerQueue: Video[] = merged.filter((v) => v.status !== "hidden"); // A reload or a bookmark of the player reopens it once this list is in, the list as its queue again - // (the feed's filters are in the address under it). Not over a live search: that is not in the - // address, and a reload leaves it. + // (the feed's filters are in the address under it) — over a live search too, its results the + // queue, once they are in (or the search waits for its press, and it plays alone). Scoped to the + // list it was linked over: leaving the search first gives it up. useOpenLinkedPlayer( - !ytActive && !query.isLoading, - (id) => playerQueue.find((v) => v.id === id), + ytActive ? ytQuery.data !== undefined || ytQuery.isError || !ytRuns : !query.isLoading, + (id) => (ytActive ? ytQueue(ytLoaded) : playerQueue).find((v) => v.id === id), (video, inList) => { openVideo(video); // Not in what this list has loaded (further down it, or no longer in it): it plays alone. if (!inList) setActiveVideo((a) => a && { ...a, alone: true }); }, + ytActive ? `yt:${ytSearch}` : "feed", ); // A live search ingests new catalog videos, so the disabled feed queries still hold their stale @@ -534,12 +540,7 @@ export default function Feed({ // A separate data source rendered in the same cards/player. No watch-state view filter (the // user explicitly searched, so show everything) and no auto-pagination (each page costs quota). if (ytActive) { - const ytItems: Video[] = sortYtResults( - withOverrides(ytLoaded, true), - ytSort.key as SortKey, - ytSort.dir, - lang, - ); + const ytItems: Video[] = ytShown(ytLoaded); const ytPlayerQueue: Video[] = ytItems.filter((v) => v.status !== "hidden"); const ytError = ytQuery.error instanceof HttpError @@ -704,7 +705,7 @@ export default function Feed({ ({ ), })); +vi.mock("../../features/player/PlayerModal", () => ({ + default: ({ video, queue }: { video: { id: string }; queue?: { id: string }[] }) => ( +

+ playing {video.id} in {queue ? queue.map((v) => v.id).join(",") : "no list"} +

+ ), +})); + import Feed from "./Feed"; import { api, DEFAULT_TAG_MODES, HttpError, type FeedFilters, type Video } from "../../lib/api"; import { qk } from "../../lib/queryKeys"; @@ -48,6 +56,7 @@ beforeAll(() => { afterEach(() => { cleanup(); vi.restoreAllMocks(); + window.history.replaceState(null, "", "/"); }); const FILTERS: FeedFilters = { @@ -106,14 +115,14 @@ function mount({ search_source: source, search_enabled: enabled, }); - const tree = (count: number) => ( + const tree = (count: number, ytSearch: string | null = "lofi") => ( {}} canRead - ytSearch="lofi" + ytSearch={ytSearch} onYtSearch={() => {}} onExitYtSearch={() => {}} /> @@ -133,7 +142,10 @@ function mount({ ); const view = render(tree(20)); - return { showing: (count: number) => view.rerender(tree(count)) }; + return { + showing: (count: number) => view.rerender(tree(count)), + leave: () => view.rerender(tree(20, null)), + }; } const settle = () => new Promise((r) => setTimeout(r, 30)); @@ -216,3 +228,41 @@ describe("Feed — a live search the address reopened", () => { expect(screen.queryByRole("button", { name: "feed.yt.runAgain" })).toBeNull(); }); }); + +// A player opened over the search names itself in the address too: a reload or a bookmark opens it +// again over the search it was opened from — not later, over a list nobody linked. +describe("Feed — a player the address names over a live search", () => { + it("reopens over the results, them as its queue", async () => { + window.history.replaceState(null, "", "/?page=feed&yt=lofi&watch=b"); + vi.spyOn(api, "searchYoutube").mockResolvedValue({ + items: [hit("a"), hit("b")], + next_cursor: null, + source: "scrape", + } as never); + const row = vi.spyOn(api, "videoRow"); + mount({ source: "scrape", asked: false }); + expect(await screen.findByText("playing b in a,b")).toBeTruthy(); + expect(row).not.toHaveBeenCalled(); + }); + + it("plays alone over a search that waits for its press", async () => { + window.history.replaceState(null, "", "/?page=feed&yt=lofi&watch=b"); + vi.spyOn(api, "videoRow").mockResolvedValue(hit("b")); + mount({ source: "api", asked: false }); + expect(await screen.findByText("playing b in no list")).toBeTruthy(); + }); + + it("is given up when the search is left before it opened, and leaves the address", async () => { + window.history.replaceState(null, "", "/?page=feed&yt=lofi&watch=b"); + vi.spyOn(api, "searchYoutube").mockReturnValue(new Promise(() => {})); + vi.spyOn(api, "feed").mockResolvedValue({ items: [hit("b")], next_cursor: null } as never); + const row = vi.spyOn(api, "videoRow").mockResolvedValue(hit("b")); + const { leave } = mount({ source: "scrape", asked: false }); + leave(); + await settle(); + await settle(); + expect(screen.queryByText(/^playing/)).toBeNull(); + expect(row).not.toHaveBeenCalled(); + expect(new URLSearchParams(window.location.search).get("watch")).toBeNull(); + }); +}); diff --git a/frontend/src/modules/feed/FeedFiltersProvider.tsx b/frontend/src/modules/feed/FeedFiltersProvider.tsx index f2b366db..98e5526f 100644 --- a/frontend/src/modules/feed/FeedFiltersProvider.tsx +++ b/frontend/src/modules/feed/FeedFiltersProvider.tsx @@ -2,6 +2,7 @@ import { useCallback, useEffect, useMemo, useState, type ReactNode } from "react import { type FeedFilters } from "../../lib/api"; import { filterParamUpdates, hasFilterParams, paramsToFilters } from "../../lib/urlState"; import { useUrlMirror } from "../../lib/useUrlMirror"; +import { useDebounced } from "../../lib/useDebounced"; import { ownKey, writeJSON } from "../../lib/storage"; import { logFilterChange } from "../../lib/filterLog"; import { useNavigation } from "../../features/navigation/navigationContext"; @@ -56,8 +57,11 @@ export function FeedFiltersProvider({ children }: { children: ReactNode }) { // keys are cleared from the URL (localStorage still holds them; they re-mirror on return to the // feed). The channel page keeps its own isolated filters, in keys of its own. useUrlMirror preserves the nav params (?page/?chan/?v/?pl) the // other providers own. + // The search text once the typing pauses, as the other pages' searches: one address-bar write per + // keystroke is churn a browser throttles. const nav = useNavigation(); - useUrlMirror(filterParamUpdates(urlPage(nav) === "feed" ? filters : null)); + const typedQ = useDebounced(filters.q ?? "", 300); + useUrlMirror(filterParamUpdates(urlPage(nav) === "feed" ? { ...filters, q: typedQ } : null)); // A share link's filters (captured at mount) win over the account's stored view and are persisted // to it once, so the view the link opened also survives a later plain reload. diff --git a/frontend/src/modules/feed/FeedFiltersProvider.url.dom.test.tsx b/frontend/src/modules/feed/FeedFiltersProvider.url.dom.test.tsx new file mode 100644 index 00000000..277f8663 --- /dev/null +++ b/frontend/src/modules/feed/FeedFiltersProvider.url.dom.test.tsx @@ -0,0 +1,82 @@ +// @vitest-environment jsdom +// +// The feed's address names its view. The view and the order always (defaults too): an address naming +// none falls back to the view the account stored, so a bookmark of the default feed reopened whatever +// view was stored since. The search text once the typing pauses, as the other pages' searches. +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { act, cleanup, render } from "@testing-library/react"; +import { useEffect } from "react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { NavigationProvider } from "../../features/navigation/NavigationProvider"; +import { FeedFiltersProvider } from "./FeedFiltersProvider"; +import { useFeedFilters, useFeedFiltersActions } from "./feedFiltersContext"; +import { qk } from "../../lib/queryKeys"; +import { claimLocalData, LS } from "../../lib/storage"; +import type { FeedFilters } from "../../lib/api"; + +const seen: { filters: FeedFilters | null; set: ((f: FeedFilters) => void) | null } = { + filters: null, + set: null, +}; +function Probe() { + const filters = useFeedFilters(); + const { setFilters } = useFeedFiltersActions(); + useEffect(() => { + seen.filters = filters; + seen.set = setFilters; + }); + return null; +} + +function mount() { + const qc = new QueryClient(); + qc.setQueryData(qk.me(), { id: 5, preferences: {} }); + render( + + + + + + + , + ); +} + +const search = () => new URLSearchParams(window.location.search); + +beforeEach(() => { + localStorage.clear(); + claimLocalData(5); +}); +afterEach(() => { + cleanup(); + window.history.replaceState(null, "", "/"); +}); + +describe("FeedFiltersProvider · the address", () => { + it("names the default view and order", () => { + window.history.replaceState(null, "", "/?page=feed"); + mount(); + expect([search().get("show"), search().get("sort")]).toEqual(["unwatched", "newest"]); + }); + + it("a bookmark of the default feed opens it, not the view the account stored since", () => { + localStorage.setItem(LS.filters, JSON.stringify({ show: "all", includeShorts: true })); + window.history.replaceState(null, "", "/?page=feed&show=unwatched&sort=newest"); + mount(); + expect(seen.filters!.show).toBe("unwatched"); + expect(seen.filters!.includeShorts).toBe(false); + }); + + it("names the search once the typing pauses", async () => { + window.history.replaceState(null, "", "/?page=feed"); + mount(); + act(() => seen.set!({ ...seen.filters!, q: "lo" })); + act(() => seen.set!({ ...seen.filters!, q: "lofi" })); + expect(search().get("q")).toBeNull(); + await act(async () => { + await new Promise((r) => setTimeout(r, 350)); + }); + expect(search().get("q")).toBe("lofi"); + }); +}); diff --git a/frontend/src/modules/feed/module.ts b/frontend/src/modules/feed/module.ts index ff1fb34c..726c7721 100644 --- a/frontend/src/modules/feed/module.ts +++ b/frontend/src/modules/feed/module.ts @@ -19,6 +19,7 @@ export const feedModule: ModuleDef = { Store: FeedStore, // Its filters, and the live YouTube search shown in its place (lib/ytSearchUrlState). urlKeys: [...FEED_KEYS, ...YT_SEARCH_KEYS], + typedUrlKeys: ["q", "yt"], playsVideos: true, }; diff --git a/frontend/src/modules/files/FilePreview.tsx b/frontend/src/modules/files/FilePreview.tsx index 482fd2dc..4de3dace 100644 --- a/frontend/src/modules/files/FilePreview.tsx +++ b/frontend/src/modules/files/FilePreview.tsx @@ -14,7 +14,7 @@ import { formatBytes } from "../../lib/format"; import { dispatch, setScopeCoveredProbe, useKeyScope } from "../../lib/keymap"; import { useLayer } from "../../lib/layerStack"; import { qk } from "../../lib/queryKeys"; -import { FILES_VIEW_KEY } from "../../lib/filesUrlState"; +import { filesOpenParams } from "../../lib/filesUrlState"; import { jobTitle } from "../../features/downloads/downloadTitle"; import { type ActionMenuItem } from "../../components/ui/ActionMenu"; import { ContextMenu } from "../../components/ui/ContextMenu"; @@ -129,7 +129,7 @@ export default function FilePreview({ title={{title}} onClose={onClose} maxWidth="max-w-6xl" - url={{ [FILES_VIEW_KEY]: String(file.id) }} + url={filesOpenParams({ open: "view", id: file.id })} headerActions={ <> { // Said once: the address no longer names it, so a reload or a copied link does not say it again. expect(here().has("fview")).toBe(false); }); + + // A list the cap cut cannot tell a deleted file from one beyond its end: it says the file is not + // among the ones listed, and keeps the link. + it("past the cap with no search or sort to widen it, says it is not listed and keeps the link", async () => { + // The case before closed a viewer: its step back lands a moment later, on whatever is current. + await new Promise((r) => setTimeout(r, 30)); + window.history.replaceState(null, "", "/?page=files&fsort=none&fview=9999"); + mocks.files = Array.from({ length: FILES_LIST_CAP }, (_, i) => file(i + 1)); + mocks.folders = [YT]; + mount(); + await waitFor(() => + expect(mocks.notify).toHaveBeenCalledWith( + expect.objectContaining({ level: "info", message: "files.arrival.notListed" }), + ), + ); + expect(mocks.notify).not.toHaveBeenCalledWith( + expect.objectContaining({ message: "files.linkedGone" }), + ); + expect(here().get("fview")).toBe("9999"); + }, 30_000); // a full page of FILES_LIST_CAP rows }); describe("Files · the view", () => { diff --git a/frontend/src/modules/files/Files.tsx b/frontend/src/modules/files/Files.tsx index caf9e2fc..5ba1dc73 100644 --- a/frontend/src/modules/files/Files.tsx +++ b/frontend/src/modules/files/Files.tsx @@ -25,11 +25,12 @@ import { LS, ownKey } from "../../lib/storage"; import { useCardPager } from "../../lib/useCardPager"; import { useDebounced } from "../../lib/useDebounced"; import { useUrlMirror } from "../../lib/useUrlMirror"; -import { dropFromAddress } from "../../lib/urlState"; +import { clearParams } from "../../lib/urlState"; import { FILES_PLAY_KEY, FILES_SORT_KEY, FILES_VIEW_KEY, + filesSortParams, openFileFrom, } from "../../lib/filesUrlState"; import { useLiveQuery } from "../../lib/useLiveQuery"; @@ -231,7 +232,7 @@ export default function Files() { setLinkedSort(undefined); setPrefSort(next); }; - useUrlMirror({ [FILES_SORT_KEY]: encodeSortParamOrNone(sort) }); + useUrlMirror(filesSortParams(encodeSortParamOrNone(sort))); const params = listParams(shown); // Live at rest: a download finishing elsewhere (the queue, a clip, another tab) lands here on its own. const filesQ = useLiveQuery(qk.files(params), () => api.files(params), { intervalMs: 15000 }); @@ -286,15 +287,17 @@ export default function Files() { const [viewing, setViewing] = useState(null); // A reload or a bookmark reopens the file the address names — in the viewer or the player — once // the list it is in has come (its folder, search and sort are in the address too). Once; a file the - // list no longer holds is said, and taken off the address. + // list no longer holds is said, and taken off the address — unless the cap cut the list, which then + // cannot tell a deleted file from one beyond its end (as an arrival's notice says): the link stays. const [linkedFile] = useState(() => openFileFrom(new URLSearchParams(window.location.search))); const linkedDone = useRef(linkedFile == null); const listIn = wide ? wideIn && wideQ.data !== undefined : filesQ.data !== undefined; const linkedRow = linkedFile && listIn ? rows.find((f) => f.id === linkedFile.id) : undefined; const openLinked = useEffectEvent((file: LibraryFile | undefined) => { - if (!file) { + if (!file && capped && !wide) notify({ level: "info", message: t("files.arrival.notListed") }); + else if (!file) { notify({ level: "warning", message: t("files.linkedGone") }); - dropFromAddress([FILES_VIEW_KEY, FILES_PLAY_KEY]); + clearParams(FILES_VIEW_KEY, FILES_PLAY_KEY); } else if (linkedFile?.open === "play") play(file); else setViewing(file.id); }); diff --git a/frontend/src/modules/files/FilesProvider.tsx b/frontend/src/modules/files/FilesProvider.tsx index 7e9d5b7a..2f65dfcd 100644 --- a/frontend/src/modules/files/FilesProvider.tsx +++ b/frontend/src/modules/files/FilesProvider.tsx @@ -2,7 +2,12 @@ import { useCallback, useMemo, useState, type ReactNode } from "react"; import { LS, getAccountRaw, setAccountRaw } from "../../lib/storage"; import { useUrlMirror } from "../../lib/useUrlMirror"; import { useDebounced } from "../../lib/useDebounced"; -import { FILES_FOLDER_KEY, FILES_ITEM_KEY, FILES_SEARCH_KEY } from "../../lib/filesUrlState"; +import { + FILES_FOLDER_KEY, + FILES_ITEM_KEY, + FILES_SEARCH_KEY, + filesListParams, +} from "../../lib/filesUrlState"; import { useNavigation, useNavigationActions } from "../../features/navigation/navigationContext"; import { urlPage } from "../../features/navigation/urlPage"; import { @@ -24,8 +29,8 @@ import { // read it via hooks. Shaped like PlaylistsProvider: split State + Actions contexts, composed in // main.tsx around . // -// The search lives in the address alone (`?fq=`): a reload or a bookmark opens it searched, and a trip -// back to the page through the rail opens it unsearched. The view is remembered per account and +// The search is named in the address (`?fq=`), so a reload or a bookmark opens it searched, and ends +// when the page is left: a trip back to the page through the rail opens it unsearched. The view is remembered per account and // mirrored to `?folder=` on the Files page, so a reload or a bookmark opens the same folder. A // `?folder=` in the address wins over the remembered one. A folder that no longer exists is the // page's to fall back from (it knows the folder list). @@ -89,19 +94,21 @@ export function FilesProvider({ children }: { children: ReactNode }) { ); // Only ON the Files page, and only until the page has used it. const focus = page === "files" && focusRaw && focusRaw.token !== focusApplied ? focusRaw : null; - // Leaving the page ends the arrival: back on it later, nothing is rung or turned to. + // Leaving the page ends the arrival — back on it later, nothing is rung or turned to — and the + // search: the page opens unsearched again. if (page !== "files" && arrivedRaw) setArrivedRaw(null); + if (page !== "files" && search) setSearchState(""); const arrived = page === "files" ? arrivedRaw : null; const shown = urlPage(nav) === "files"; // Written once the typing pauses (one address-bar write per keystroke is churn a browser throttles); // an arrival's cleared box at once. const typed = useDebounced(search.trim(), 300, focusRaw?.token); - useUrlMirror({ - [FILES_SEARCH_KEY]: shown && typed ? typed : null, - [FILES_FOLDER_KEY]: shown ? viewParam(view) : null, - [FILES_ITEM_KEY]: shown && focus ? String(focus.id) : null, - }); + useUrlMirror( + filesListParams( + shown ? { search: typed, folder: viewParam(view), item: focus ? focus.id : null } : null, + ), + ); const state = useMemo( () => ({ search, view, focus, arrived }), diff --git a/frontend/src/modules/files/LibraryPlayer.tsx b/frontend/src/modules/files/LibraryPlayer.tsx index 379b0fe2..0f3ea7a7 100644 --- a/frontend/src/modules/files/LibraryPlayer.tsx +++ b/frontend/src/modules/files/LibraryPlayer.tsx @@ -4,7 +4,7 @@ import { useQueryClient } from "@tanstack/react-query"; import Modal from "../../components/ui/Modal"; import { api, type DownloadJob } from "../../lib/api"; import { qk } from "../../lib/queryKeys"; -import { FILES_PLAY_KEY } from "../../lib/filesUrlState"; +import { filesOpenParams } from "../../lib/filesUrlState"; import { jobTitle } from "../../features/downloads/downloadTitle"; import { exposeVideoToBackgroundPlay } from "../../lib/browserBackgroundPlay"; import { useMediaLogRef } from "../../lib/mediaLog"; @@ -97,7 +97,7 @@ export default function LibraryPlayer({ title={title} onClose={onClose} maxWidth="max-w-4xl" - url={{ [FILES_PLAY_KEY]: String(job.id) }} + url={filesOpenParams({ open: "play", id: job.id })} > {isAudio ? (
diff --git a/frontend/src/modules/files/module.ts b/frontend/src/modules/files/module.ts index 5cef1c49..e3c2fc06 100644 --- a/frontend/src/modules/files/module.ts +++ b/frontend/src/modules/files/module.ts @@ -18,4 +18,5 @@ export const filesModule: ModuleDef = { maxWidth: "80rem", Store: FilesProvider, urlKeys: FILES_KEYS, + typedUrlKeys: ["fq"], }; diff --git a/frontend/src/modules/playlists/Playlists.dom.test.tsx b/frontend/src/modules/playlists/Playlists.dom.test.tsx index 2fc7683c..4beca9c7 100644 --- a/frontend/src/modules/playlists/Playlists.dom.test.tsx +++ b/frontend/src/modules/playlists/Playlists.dom.test.tsx @@ -211,6 +211,32 @@ describe("Playlists — a player the address names", () => { expect(h.player!.queue?.map((v) => v.id)).toEqual(["Z"]); expect(h.player!.onRemoveFromList).toBeUndefined(); }); + + it("plays alone over a playlist emptied since", async () => { + window.history.replaceState(null, "", "/?page=playlists&pl=1&watch=Z"); + vi.spyOn(api, "videoRow").mockResolvedValue(vid("Z", 0)); + mount({ 1: [] }); + await waitFor(() => expect(h.player?.video.id).toBe("Z")); + expect(h.player!.queue?.map((v) => v.id)).toEqual(["Z"]); + }); + + it("is given up when another playlist is picked before it opened, and leaves the address", async () => { + window.history.replaceState(null, "", "/?page=playlists&pl=1&watch=Z"); + const row = vi.spyOn(api, "videoRow").mockResolvedValue(vid("Z", 0)); + // The linked playlist's read never answers: the opener is still waiting when the pick lands. + const { rerender } = mount({ 1: [vid("A", 0)], 2: [vid("B", 0)] }); + vi.mocked(api.playlist).mockImplementation(async (id: number) => + id === 1 ? new Promise(() => {}) : (list(id, [vid("B", 0)]) as never), + ); + qc.removeQueries({ queryKey: qk.playlist(1) }); + h.selectedId = 2; + rerender(); + await screen.findByText("Video B"); + await new Promise((r) => setTimeout(r, 30)); + expect(h.player).toBeNull(); + expect(row).not.toHaveBeenCalled(); + expect(new URLSearchParams(window.location.search).get("watch")).toBeNull(); + }); }); describe("Playlists — the row the player is launched with", () => { diff --git a/frontend/src/modules/playlists/Playlists.tsx b/frontend/src/modules/playlists/Playlists.tsx index 787d6278..c28e86b9 100644 --- a/frontend/src/modules/playlists/Playlists.tsx +++ b/frontend/src/modules/playlists/Playlists.tsx @@ -583,14 +583,21 @@ function PlaylistDetail() { }); // A reload or a bookmark of the player reopens it once this playlist is in, its view as the queue // again (the playlist and its view are in the address under it). A video the view does not show - // (filtered out, gone from the list) plays alone, and nothing removes it from a list. + // (filtered out, gone from the list, the list emptied or out of reach) plays alone, and nothing + // removes it from a list. In: the order seeded from it (an empty one has none to wait for), or its + // read refused. Scoped to the playlist it was linked over: another one picked first gives it up. + const linkedListIn = + (detail != null && (detail.items.length === 0 || items.length > 0)) || + (selectedId != null && detailQuery.isError) || + (selectedId == null && listQuery.data?.length === 0); useOpenLinkedPlayer( - detail != null && items.length > 0, + linkedListIn, (id) => shown.find((v) => v.id === id), (video, inList) => { if (inList) openPlayer(video.id); else setPlaying({ video, queue: [video], index: 0, listId: selectedId, editable: false }); }, + selectedId, ); async function moveItemTo(video: Video, target: Playlist) { diff --git a/frontend/src/modules/playlists/PlaylistsProvider.tsx b/frontend/src/modules/playlists/PlaylistsProvider.tsx index 5e90fa42..5b9b1d06 100644 --- a/frontend/src/modules/playlists/PlaylistsProvider.tsx +++ b/frontend/src/modules/playlists/PlaylistsProvider.tsx @@ -5,6 +5,7 @@ import { useDebounced } from "../../lib/useDebounced"; import { useNavigation } from "../../features/navigation/navigationContext"; import { urlPage } from "../../features/navigation/urlPage"; import { readSelectedId } from "./playlistsInit"; +import { playlistsParams } from "../../lib/playlistsUrlState"; import { PlaylistsActionsContext, PlaylistsStateContext, @@ -18,8 +19,8 @@ import { // FeedFiltersProvider / NavigationProvider (split State + Actions contexts, one-line hooks; composed // in main.tsx around ). // -// The `search` term lives in the address alone (`?plq=`, a reload or a bookmark keeps it, a trip back -// through the rail starts unsearched); the selected playlist is per-account persisted (LS.playlist) so +// The `search` term is named in the address (`?plq=`, a reload or a bookmark keeps it) and ends when +// the page is left (a trip back through the rail starts unsearched); the selected playlist is per-account persisted (LS.playlist) so // a reload returns you to it, and named in the address too. Neither is server-synced, and an account switch does a full reload, so init-from-storage is all the // hydration these need — no per-account load effect (unlike the server-backed feed filters). // @@ -43,11 +44,10 @@ export function PlaylistsProvider({ children }: { children: ReactNode }) { // The search once the typing pauses: one address-bar write per keystroke is churn a browser throttles. const nav = useNavigation(); const shown = urlPage(nav) === "playlists"; + // Leaving the page ends the search: adjusted during the render that leaves it. + if (nav.page !== "playlists" && search) setSearchState(""); const typed = useDebounced(search.trim(), 300); - useUrlMirror({ - pl: shown && selectedId != null ? String(selectedId) : null, - plq: shown && typed ? typed : null, - }); + useUrlMirror(shown ? playlistsParams(selectedId, typed) : playlistsParams(null, "")); const state = useMemo(() => ({ search, selectedId }), [search, selectedId]); const actions = useMemo( diff --git a/frontend/src/modules/playlists/module.ts b/frontend/src/modules/playlists/module.ts index 9b88fe87..fb773d1c 100644 --- a/frontend/src/modules/playlists/module.ts +++ b/frontend/src/modules/playlists/module.ts @@ -19,5 +19,6 @@ export const playlistsModule: ModuleDef = { Store: PlaylistsProvider, // The playlist shown (PlaylistsProvider, playlistsInit). urlKeys: PLAYLISTS_KEYS, + typedUrlKeys: ["plq"], playsVideos: true, }; diff --git a/frontend/src/modules/plex/module.ts b/frontend/src/modules/plex/module.ts index 697c8126..458d782b 100644 --- a/frontend/src/modules/plex/module.ts +++ b/frontend/src/modules/plex/module.ts @@ -19,4 +19,5 @@ export const plexModule: ModuleDef = { maxWidth: "100rem", Store: PlexStore, urlKeys: [...PLEX_KEYS, ...PLEX_SEARCH_KEYS, PLEX_SUB_KEY], + typedUrlKeys: ["pq"], }; diff --git a/frontend/src/modules/settings/SettingsPanel.tsx b/frontend/src/modules/settings/SettingsPanel.tsx index 564c4f79..dc14ec49 100644 --- a/frontend/src/modules/settings/SettingsPanel.tsx +++ b/frontend/src/modules/settings/SettingsPanel.tsx @@ -26,7 +26,7 @@ import { api, HttpError, type Me } from "../../lib/api"; import { displayName, roleKey } from "../../lib/api/account"; import { notifyActionError } from "../../lib/actionErrors"; import { LS, useTabPersistedState } from "../../lib/storage"; -import { SETTINGS_TAB_KEY } from "../../lib/settingsTab"; +import { SETTINGS_TAB_KEY, settingsTabParams } from "../../lib/settingsTab"; import { useUrlMirror } from "../../lib/useUrlMirror"; import Avatar from "../../components/ui/Avatar"; import { notify, type NotifSettings } from "../../lib/notifications"; @@ -80,7 +80,7 @@ export default function SettingsPanel() { : "appearance"; // The address names the tab on screen, so a bookmark opens it (always written: without it, the page // opens on the tab this browser tab last showed). - useUrlMirror({ [SETTINGS_TAB_KEY]: tab }); + useUrlMirror(settingsTabParams(tab)); return (
diff --git a/frontend/src/modules/users/AdminUsers.tsx b/frontend/src/modules/users/AdminUsers.tsx index 811ffc19..8c5a39e5 100644 --- a/frontend/src/modules/users/AdminUsers.tsx +++ b/frontend/src/modules/users/AdminUsers.tsx @@ -27,7 +27,7 @@ import { displayName } from "../../lib/api/account"; import { notify } from "../../lib/notifications"; import { formatDateTime, relativeTime } from "../../lib/format"; import { invalidateUserAccess } from "../../lib/adminUsers"; -import { ADMIN_USERS_TAB_KEY, USERS_TAB_URL_KEY } from "../../lib/adminUsersTab"; +import { ADMIN_USERS_TAB_KEY, USERS_TAB_URL_KEY, usersTabParams } from "../../lib/adminUsersTab"; import { Badge } from "../../components/ui/Badge"; import { Button } from "../../components/ui/Button"; import { IconButton } from "../../components/ui/IconButton"; @@ -89,7 +89,7 @@ export default function AdminUsers() { ]; const active = tabs.some((x) => x.id === tab) ? tab : "roles"; // The address names the tab on screen (always: see SettingsPanel). - useUrlMirror({ [USERS_TAB_URL_KEY]: active }); + useUrlMirror(usersTabParams(active)); return ( <> {/* The tab bar is fixed chrome; the tab content scrolls under it. */}