diff --git a/frontend/eslint.config.js b/frontend/eslint.config.js index c54c2b96..7bc0790d 100644 --- a/frontend/eslint.config.js +++ b/frontend/eslint.config.js @@ -898,7 +898,8 @@ export default tseslint.config( plugins: { playwright }, rules: { "playwright/no-focused-test": "error", - "playwright/no-skipped-test": "error", + // fixme too: a test Playwright never runs, which the bookmark net would still count. + "playwright/no-skipped-test": ["error", { disallowFixme: true }], }, }, { diff --git a/frontend/src/features/navigation/NavigationProvider.tsx b/frontend/src/features/navigation/NavigationProvider.tsx index 25d6ea88..269eb1e1 100644 --- a/frontend/src/features/navigation/NavigationProvider.tsx +++ b/frontend/src/features/navigation/NavigationProvider.tsx @@ -64,6 +64,13 @@ function pageRootParams(page: Page): Record { }; } +/** The address of the page under a live search, the search ended: no search, player or channel. */ +const searchEndParams = (): Record => ({ + ...closedChannelParams(), + ...nulls(PLAYER_KEYS), + ...ytSearchParams(null, DEFAULT_YT_COUNT), +}); + const nulls = (keys: readonly string[]) => Object.fromEntries(keys.map((k) => [k, null])); /** No channel page: neither `chan` nor its view's keys, which the page itself writes (ChannelPage) — @@ -138,7 +145,8 @@ export function NavigationProvider({ children }: { children: ReactNode }) { // A channel page overlays the content column, so a nav click must close it even when the // underlying page is unchanged (e.g. "Feed" while a channel opened from the feed). // A live search on it is one too: the press opens the page's root, as over a channel. - if (next === pageRef.current && !channelViewRef.current && ytSearchRef.current == null) { + const samePage = next === pageRef.current && !channelViewRef.current; + if (samePage && ytSearchRef.current == null) { then?.(); return; } @@ -154,11 +162,13 @@ export function NavigationProvider({ children }: { children: ReactNode }) { // every module's own keys (pageRootParams) — a page switch resets to that module's root, and // each module's store mirrors its state back in on its page. The item it was opened from is // still focused: the state setters above have not re-rendered anything yet. + // The press on a search over its own page ends only the search: the page's view is the one + // on screen and stays named — its store, unchanged, writes nothing back in. rememberNavPosition(); window.history.pushState( { sfPage: next, sfScroll: nextNavId() }, "", - urlWithParams(pageRootParams(next)), + urlWithParams(samePage ? searchEndParams() : pageRootParams(next)), ); notifyNav(); then?.(); @@ -215,11 +225,11 @@ export function NavigationProvider({ children }: { children: ReactNode }) { // is the first entry of this document, so its new `_yt` mark has nothing of ours below it). replaceView({ ...st, _yt: q }, urlWithParams(ytSearchParams(q, ytCountFrom(here)))); } else { - // New sub-view entry, at the default count. + // New sub-view entry, at the default count — ours (`_ytOwn`): leaving it steps back off it. setYtCountState(DEFAULT_YT_COUNT); const left = rememberNavPosition(); window.history.pushState( - { ...left, sfPage: "feed", _yt: q }, + { ...left, sfPage: "feed", _yt: q, _ytOwn: 1 }, "", urlWithParams(ytSearchParams(q, DEFAULT_YT_COUNT)), ); @@ -242,11 +252,13 @@ export function NavigationProvider({ children }: { children: ReactNode }) { const exitYtSearch = useCallback(() => { // Pop our search entry (so Forward still works); fall back to a clear in place if we don't own - // 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(); + // one — a search a link or a reload opened is marked (`_yt`) but was pushed by nobody: below it is + // another site, or nothing — or there is no entry of this document below it (a full tab, see + // backStaysInDocument). + if (window.history.state?._ytOwn && backStaysInDocument()) window.history.back(); else { endYtSearch(); - const { _yt: _q, ...st } = window.history.state ?? {}; + const { _yt: _q, _ytOwn: _own, ...st } = window.history.state ?? {}; replaceView(st, urlWithParams(ytSearchParams(null, DEFAULT_YT_COUNT))); } }, [endYtSearch]); @@ -322,7 +334,7 @@ export function NavigationProvider({ children }: { children: ReactNode }) { if (named != null) { window.history.replaceState({ ...st, _yt: named, sfPage: mountPage }, "", url); } else { - const { _yt: _staleYt, _ytScrape: _staleScrape, ...rest } = st; + const { _yt: _staleYt, _ytScrape: _staleScrape, _ytOwn: _staleOwn, ...rest } = st; window.history.replaceState({ ...rest, sfPage: mountPage }, "", url); } function onPop(e: PopStateEvent) { diff --git a/frontend/src/features/navigation/NavigationProvider.ytSearch.dom.test.tsx b/frontend/src/features/navigation/NavigationProvider.ytSearch.dom.test.tsx index cc41603b..17922af2 100644 --- a/frontend/src/features/navigation/NavigationProvider.ytSearch.dom.test.tsx +++ b/frontend/src/features/navigation/NavigationProvider.ytSearch.dom.test.tsx @@ -243,6 +243,33 @@ describe("NavigationProvider · the YouTube search in the address", () => { expect(window.history.length).toBe(depth + 1); // its own entry: Back returns to the search }); + it("the Feed press on the search keeps the feed's own view in the address", () => { + window.history.replaceState({ sfPage: "feed" }, "", "/?page=feed&show=all&sort=oldest"); + const { result } = render(); + act(() => result.current.enterYtSearch("lofi")); + act(() => result.current.setPage("feed")); + expect(search().get("yt")).toBeNull(); + expect([search().get("show"), search().get("sort")]).toEqual(["all", "oldest"]); + }); + + it("a search sent from the box is left by a step back off its own entry", () => { + const { result } = render(); + act(() => result.current.enterYtSearch("lofi")); + const back = vi.spyOn(window.history, "back").mockImplementation(() => {}); + act(() => result.current.exitYtSearch()); + expect(back).toHaveBeenCalledTimes(1); + }); + + it("a search a link opened is left in place, not by a step back off the document", () => { + window.history.replaceState(null, "", "/?page=feed&yt=lofi"); + const { result } = render(); + const back = vi.spyOn(window.history, "back"); + act(() => result.current.exitYtSearch()); + expect(back).not.toHaveBeenCalled(); + expect(result.current.ytSearch).toBeNull(); + expect(search().get("yt")).toBeNull(); + }); + it("a landing on a channel opened from the search (an overlay over it closing) keeps the search", () => { const { result } = render(); act(() => result.current.enterYtSearch("lofi")); diff --git a/frontend/src/i18n/locales/en/files.json b/frontend/src/i18n/locales/en/files.json index ff10fb41..404c1681 100644 --- a/frontend/src/i18n/locales/en/files.json +++ b/frontend/src/i18n/locales/en/files.json @@ -1,6 +1,6 @@ { "linkedGone": "The file in the address is no longer in this list", - "linkedNotListed": "The file in the link isn't among the files listed here — it may be further down the list", + "linkedNotListed": "The file in the link isn't among the files listed here — search for it by name", "navLabel": "Files", "searchPlaceholder": "Search files…", "count_one": "{{count}} item", diff --git a/frontend/src/i18n/locales/hu/files.json b/frontend/src/i18n/locales/hu/files.json index 88354b9a..b45aa571 100644 --- a/frontend/src/i18n/locales/hu/files.json +++ b/frontend/src/i18n/locales/hu/files.json @@ -1,6 +1,6 @@ { "linkedGone": "A linkben szereplő fájl már nincs ebben a listában", - "linkedNotListed": "A linkben szereplő fájl nincs a listázottak között — lehet, hogy lejjebb van", + "linkedNotListed": "A linkben szereplő fájl nincs a listázottak között — keress rá a nevére", "navLabel": "Fájlok", "searchPlaceholder": "Keresés a fájlok között…", "count_one": "{{count}} elem", diff --git a/frontend/src/lib/gates.snapshot.json b/frontend/src/lib/gates.snapshot.json index 7d42f338..088762a0 100644 --- a/frontend/src/lib/gates.snapshot.json +++ b/frontend/src/lib/gates.snapshot.json @@ -78,7 +78,7 @@ { "allowShortCircuit": false, "allowTaggedTemplates": false, "allowTernary": false } ], "playwright/no-focused-test": [2], - "playwright/no-skipped-test": [2] + "playwright/no-skipped-test": [2, { "allowConditional": false, "disallowFixme": true }] }, "settings": null }, diff --git a/frontend/src/lib/useUrlMirror.reloadedFirstWrite.dom.test.tsx b/frontend/src/lib/useUrlMirror.reloadedFirstWrite.dom.test.tsx new file mode 100644 index 00000000..8633b55d --- /dev/null +++ b/frontend/src/lib/useUrlMirror.reloadedFirstWrite.dom.test.tsx @@ -0,0 +1,27 @@ +// @vitest-environment jsdom +// +// A reload under an overlay that reopens (the player) loads on the overlay's entry, its mark on it, +// and the overlay takes that entry over as it opens again. A page's first mirror write on that load +// (the defaults its address did not name yet) is the same view, not another: the mark stays, and the +// reopened overlay still takes its entry over rather than pushing one more. Its own file: the entry +// is read as the history 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 page's first mirror write on the entry a reload under an overlay started from", () => { + it("leaves the entry to the overlay that reopens", async () => { + window.history.replaceState({ sfPage: "activity" }, "", "/?page=activity"); + window.history.pushState({ sfPage: "activity", _ov: 1, _ovd: 1 }, "", "/?page=activity"); + const { useUrlMirror } = await import("./useUrlMirror"); + const { useBackToClose } = await import("./history"); + renderHook(() => useUrlMirror({ at: "runs" })); + expect(new URLSearchParams(window.location.search).get("at")).toBe("runs"); + + const depth = window.history.length; + renderHook(() => useBackToClose(() => {})); + await act(async () => { + await Promise.resolve(); + }); + expect(window.history.length).toBe(depth); + }); +}); diff --git a/frontend/src/lib/useUrlMirror.reloadedRewrite.dom.test.tsx b/frontend/src/lib/useUrlMirror.reloadedRewrite.dom.test.tsx new file mode 100644 index 00000000..5db8f44d --- /dev/null +++ b/frontend/src/lib/useUrlMirror.reloadedRewrite.dom.test.tsx @@ -0,0 +1,39 @@ +// @vitest-environment jsdom +// +// A reload under a dialog that does not reopen (About) loads on the dialog's entry, its overlay mark +// still on it. A page's view mirrored onto that entry (another tab) is another view: the next overlay +// pushes its own entry over it rather than taking this one over — taken over, closing it stepped back +// onto the entry below, and the page read its old view back from that address. Its own file: the +// entry is read as the history 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 mirrored onto the entry a reload under a dialog started from", () => { + it("is where an overlay over it closes to", async () => { + window.history.replaceState({ sfPage: "activity" }, "", "/?page=activity&at=runs"); + window.history.pushState( + { sfPage: "activity", _ov: 1, _ovd: 1 }, + "", + "/?page=activity&at=runs", + ); + const { useUrlMirror } = await import("./useUrlMirror"); + const { useBackToClose } = await import("./history"); + const mirror = renderHook(({ at }) => useUrlMirror({ at }), { initialProps: { at: "runs" } }); + mirror.rerender({ at: "admin" }); + expect(new URLSearchParams(window.location.search).get("at")).toBe("admin"); + + const depth = window.history.length; + const overlay = renderHook(() => useBackToClose(() => {})); + 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 })); + overlay.unmount(); + await landed; + }); + expect(new URLSearchParams(window.location.search).get("at")).toBe("admin"); + }); +}); diff --git a/frontend/src/lib/useUrlMirror.ts b/frontend/src/lib/useUrlMirror.ts index 21d26d14..751e1fc3 100644 --- a/frontend/src/lib/useUrlMirror.ts +++ b/frontend/src/lib/useUrlMirror.ts @@ -1,4 +1,5 @@ -import { useEffect, useEffectEvent, useState } from "react"; +import { useEffect, useEffectEvent, useRef, useState } from "react"; +import { replaceView } from "./history"; import { urlWithParams } from "./urlState"; // Mirror a set of nav params onto the CURRENT history entry (replaceState — mirroring is not a new @@ -19,11 +20,20 @@ export function useUrlMirror(updates: Record): void { // 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. + // A change of what this mirror writes, after its first write, is another view on the same entry + // (replaceView: it drops the marks a reload under an overlay left). Its first write on a mount and + // its write on a landing name the view the entry already shows, so the marks stay: an overlay that + // reopens after a reload still finds its entry. + const written = useRef(null); const mirror = useEffectEvent(() => { const href = urlWithParams(updates); + const changed = written.current != null && written.current !== dep; + written.current = dep; // 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); + if (href === urlWithParams({})) return; + if (changed) replaceView(window.history.state ?? {}, href); + else 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 diff --git a/frontend/src/moduleSearch.url.dom.test.tsx b/frontend/src/moduleSearch.url.dom.test.tsx index 0e4472b0..baae3e4e 100644 --- a/frontend/src/moduleSearch.url.dom.test.tsx +++ b/frontend/src/moduleSearch.url.dom.test.tsx @@ -20,13 +20,14 @@ import { } from "./features/navigation/navigationContext"; import { ModuleStores } from "./features/navigation/ModuleStores"; import { useChannels } from "./modules/channels/channelsContext"; -import { useFiles } from "./modules/files/filesContext"; -import { usePlaylists } from "./modules/playlists/playlistsContext"; +import { useFiles, useFilesActions } from "./modules/files/filesContext"; +import { usePlaylists, usePlaylistsActions } from "./modules/playlists/playlistsContext"; 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 }; +const typeIn: { current: Record void> } = { current: {} }; function Probe() { const channels = useChannels(); @@ -34,8 +35,11 @@ function Probe() { const playlists = usePlaylists(); const plex = usePlex(); const actions = useNavigationActions(); + const filesActions = useFilesActions(); + const playlistsActions = usePlaylistsActions(); useEffect(() => { nav.current = actions; + typeIn.current = { files: filesActions.setSearch, playlists: playlistsActions.setSearch }; seen.current = { channels: channels.search, files: files.search, @@ -115,5 +119,8 @@ describe.each([ await waitFor(() => expect(seen.current[page]).toBe("")); // At once, not after the typing pause: a reload right after the return opens it unsearched. expect(search().get(key)).toBeNull(); + // Typing again within the pause names the new text or nothing, never the ended search. + act(() => typeIn.current[page]!("x")); + expect(search().get(key)).not.toBe(value); }); }); diff --git a/frontend/src/moduleUrl.contract.test.ts b/frontend/src/moduleUrl.contract.test.ts index 01dc3773..b2d5a931 100644 --- a/frontend/src/moduleUrl.contract.test.ts +++ b/frontend/src/moduleUrl.contract.test.ts @@ -86,19 +86,21 @@ function sources(dir: string): string[] { } // The object literals handed to an address writer: `urlWithParams` / `useUrlMirror`, an overlay's -// own keys (ui/Modal's `url`, `useBackToClose`'s). +// own keys (ui/Modal's `url`, `useBackToClose`'s). Each pattern ends at the literal's opening brace; +// the literal runs to its matching close (`writerObjects`), nested arguments and all. const WRITER_OBJECTS = [ - /(?:urlWithParams|useUrlMirror)\(\{([^}]*)\}/g, - /\burl=\{\{([^}]*)\}/g, + /(?:urlWithParams|useUrlMirror)\(\{/g, + /\burl=\{\{/g, // Its first argument a name, a call or an arrow (`() => setOpen(false)`). - /useBackToClose\((?:[^,()]|\([^()]*\))+,\s*\{([^}]*)\}/g, + /useBackToClose\((?:[^,()]|\([^()]*\))+,\s*\{/g, ]; // One of each form: a regex that matched nothing in the tree would pass every scan. const WRITER_FIXTURE = ` - useUrlMirror({ fxa: x }); + useUrlMirror({ fxa: x, fxf }); useBackToClose(onClose, { fxc: "1" }); useBackToClose(() => setOpen(false), { fxd: "1" }); + urlWithParams({ ...fxCodec({ ...fxState, job }), fxe: "1" }); `; // Where a writer outside lib/ is handed its value: the address writers' argument, and an @@ -132,6 +134,18 @@ function topLevel(text: string, sep: string): string[] { out.push(text.slice(from)); return out.map((s) => s.trim()); } +/** The top-level pieces of every writer's object literal in `text` (`a: x`, `...f(y)`, `[K]: v`, `b`). */ +function writerObjects(text: string): string[][] { + return WRITER_OBJECTS.flatMap((re) => + [...text.matchAll(re)].map((m) => { + const open = m.index + m[0].length - 1; + return topLevel(text.slice(open + 1, closing(text, open)), ",").filter(Boolean); + }), + ); +} +/** The key a piece names as a literal: `key: value`, or the shorthand `key`. */ +const keyOf = (piece: string) => /^(\w+)\s*:/.exec(piece)?.[1] ?? /^(\w+)$/.exec(piece)?.[1]; + const LIB_EXPORTS = new Set( sources(join(SRC, "lib")).flatMap((f) => [...readFileSync(f, "utf8").matchAll(/export (?:function|const) (\w+)/g)].map((m) => m[1]!), @@ -143,10 +157,13 @@ function writerValueOk(value: string): boolean { const choice = topLevel(v, "?"); if (choice.length === 2) return topLevel(choice[1]!, ":").every(writerValueOk); if (v.startsWith("{")) { - // Its spreads are a lib/ function's result too. + // Its spreads are a lib/ function's result too; its other pieces a literal key and its value — + // a computed or a shorthand one is a key no literal scan reads. return topLevel(v.slice(1, closing(v, 0)), ",") - .filter((piece) => piece.startsWith("...")) - .every((piece) => writerValueOk(piece.slice(3))); + .filter(Boolean) + .every((piece) => + piece.startsWith("...") ? writerValueOk(piece.slice(3)) : /^\w+\s*:/.test(piece), + ); } const call = /^(\w+)\(/.exec(v); return ( @@ -189,9 +206,10 @@ function queryKeyLiterals(): { key: string; where: string }[] { for (const file of sources(SRC)) { const text = readFileSync(file, "utf8"); const where = relative(SRC, file).split("\\").join("/"); - 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 pieces of writerObjects(text)) { + for (const piece of pieces) { + const key = keyOf(piece); + if (key) out.push({ key, where }); } } if (!text.includes("location.search")) continue; @@ -211,10 +229,9 @@ function computedWriterKeys(inLib = false): string[] { 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)}`); - } + for (const pieces of writerObjects(text)) { + const computed = pieces.find((piece) => piece.startsWith("[")); + if (computed) out.push(`${where}: ${computed.slice(0, 60)}`); } } return out; @@ -352,11 +369,11 @@ describe("module URL keys", () => { }); it("the writer scans see each form they are meant to", () => { - const keys = WRITER_OBJECTS.flatMap((re) => - [...WRITER_FIXTURE.matchAll(re)].map((m) => /(\w+)\s*:/.exec(m[1]!)![1]), + const keys = writerObjects(WRITER_FIXTURE).flatMap((pieces) => + pieces.flatMap((piece) => (piece.startsWith("...") ? [] : (keyOf(piece) ?? []))), ); - expect(keys.sort()).toEqual(["fxa", "fxb", "fxc", "fxd"]); - expect(writerValues(WRITER_FIXTURE)).toHaveLength(4); + expect(keys.sort()).toEqual(["fxa", "fxb", "fxc", "fxd", "fxe", "fxf"]); + expect(writerValues(WRITER_FIXTURE)).toHaveLength(5); }); it("are written outside lib/ only as a literal or a lib/ function's result", () => { @@ -368,6 +385,9 @@ describe("module URL keys", () => { expect(writerValueOk("upd")).toBe(false); expect(writerValueOk("myOwnParams(v)")).toBe(false); expect(writerValueOk("{ ...st, zz: 1 }")).toBe(false); + // A key no scan reads as a literal: computed, or shorthand. + expect(writerValueOk("{ page: p, [K]: v }")).toBe(false); + expect(writerValueOk("{ page: p, zz }")).toBe(false); expect(writerValueOk("Object.fromEntries(rows)")).toBe(false); }); @@ -380,6 +400,30 @@ describe("module URL keys", () => { expect(defined.sort()).toEqual(["MODE_URL", "PLEX_SUB_URL"]); }); + // …and every sub-view is handed one of those, by name: a definition written any other way (inline, + // `satisfies`) is one the census above does not count. + it("hand each sub-view a definition the census names", () => { + const handed = (text: string) => + [...text.matchAll(/\buseHistorySubview(?:<[^>()]*>)?\(/g)].flatMap((m) => { + const open = m.index + m[0].length - 1; + const args = topLevel(text.slice(open + 1, closing(text, open)), ","); + return args.length > 1 ? [args[1]!] : []; + }); + const named = new Set(["MODE_URL", "PLEX_SUB_URL"]); + const unnamed = (text: string) => handed(text).filter((arg) => !named.has(arg)); + const files = sources(SRC).filter( + (f) => !relative(SRC, f).split("\\").join("/").startsWith("lib/history"), + ); + expect(files.flatMap((f) => unnamed(readFileSync(f, "utf8")))).toEqual([]); + // The call sites are seen (a scan of none would pass)… + expect(new Set(files.flatMap((f) => handed(readFileSync(f, "utf8"))))).toEqual(named); + // …and the check tells an inline definition from a named one. + expect(unnamed('useHistorySubview(sub, { key: "zz", encode, decode })')).toEqual([ + '{ key: "zz", encode, decode }', + ]); + expect(unnamed("useHistorySubview(sub, PLEX_SUB_URL)")).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); }); diff --git a/frontend/src/modules/feed/Feed.tsx b/frontend/src/modules/feed/Feed.tsx index 29b53679..36143f8d 100644 --- a/frontend/src/modules/feed/Feed.tsx +++ b/frontend/src/modules/feed/Feed.tsx @@ -24,6 +24,7 @@ import { import { YouTubeIcon } from "../../lib/brandIcons"; import { api, + FEED_DEFAULT_SORT, HttpError, refusalReason, type FeedFilters, @@ -318,7 +319,7 @@ export default function Feed({ const qEmpty = !filters.q.trim(); useEffect(() => { if (qEmpty && parseSort(filters.sort).key === "relevance") { - setFilters({ ...filters, sort: buildSort("date", "desc") }); + setFilters({ ...filters, sort: FEED_DEFAULT_SORT }); } // Naming `filters` is safe precisely because the write below falsifies the condition above: the // sort it sets is "date", so the one re-run it causes does nothing. The old `[qEmpty]` was @@ -624,7 +625,7 @@ export default function Feed({ .catch((e: unknown) => notifyActionError(e, i18n.t("errors.writeFailed.clearSearch")), ); - setFilters({ ...filters, librarySource: "organic", sort: "newest" }); + setFilters({ ...filters, librarySource: "organic", sort: FEED_DEFAULT_SORT }); onExitYtSearch(); dropFeedCaches(); qc.removeQueries({ queryKey: qk.ytSearch() }); diff --git a/frontend/src/modules/feed/FeedHeader.tsx b/frontend/src/modules/feed/FeedHeader.tsx index af2d3109..50f834f3 100644 --- a/frontend/src/modules/feed/FeedHeader.tsx +++ b/frontend/src/modules/feed/FeedHeader.tsx @@ -1,4 +1,5 @@ import { useTranslation } from "react-i18next"; +import { FEED_DEFAULT_SORT } from "../../lib/api"; import { YouTubeIcon } from "../../lib/brandIcons"; import { useMe } from "../../lib/useMe"; import { PageHeader } from "../../components/ui/PageShell"; @@ -25,9 +26,9 @@ export default function FeedHeader() { placeholder={t("header.searchPlaceholder")} onChange={(q) => { // When a search first appears, rank the feed by relevance — set atomically with the - // query (race-free vs. per-keystroke updates). Only overrides the default "newest" + // query (race-free vs. per-keystroke updates). Only overrides the default // sort; a custom sort the user chose is left alone. Cleared in Feed. - const startSearch = !filters.q.trim() && !!q.trim() && filters.sort === "newest"; + const startSearch = !filters.q.trim() && !!q.trim() && filters.sort === FEED_DEFAULT_SORT; setFilters({ ...filters, q, ...(startSearch ? { sort: "relevance" } : {}) }); }} // Turned off on this instance, the box only filters: no Go that could only be refused. diff --git a/frontend/src/modules/files/FilesProvider.tsx b/frontend/src/modules/files/FilesProvider.tsx index 4c8fb3dc..da980124 100644 --- a/frontend/src/modules/files/FilesProvider.tsx +++ b/frontend/src/modules/files/FilesProvider.tsx @@ -102,10 +102,10 @@ export function FilesProvider({ children }: { children: ReactNode }) { const shown = urlPage(nav) === "files"; // Written once the typing pauses (one address-bar write per keystroke is churn a browser throttles); - // an emptied box at once — an arrival's, or the page left: back on it within the pause, the address - // would name the ended search again. - const debounced = useDebounced(search.trim(), 300, focusRaw?.token); - const typed = search.trim() ? debounced : ""; + // a box emptied — an arrival's, or the page left — and filled again, each at once: the pause would + // name the ended search again, on its way out or before the new text settled. + const q = search.trim(); + const typed = useDebounced(q, 300, `${focusRaw?.token ?? ""}:${q === ""}`); useUrlMirror( filesListParams( shown ? { search: typed, folder: viewParam(view), item: focus ? focus.id : null } : null, diff --git a/frontend/src/modules/playlists/Playlists.dom.test.tsx b/frontend/src/modules/playlists/Playlists.dom.test.tsx index 1b59c041..e48d022d 100644 --- a/frontend/src/modules/playlists/Playlists.dom.test.tsx +++ b/frontend/src/modules/playlists/Playlists.dom.test.tsx @@ -12,6 +12,8 @@ const h = vi.hoisted(() => ({ notify: vi.fn(), confirm: vi.fn(async () => true), selectedId: 1 as number | null, + // The selection the store started with (the address's, on a reload or a link). + linkedId: 1 as number | null, // Who reads the selection, so a test's change re-renders them the way the real store does. selectionReaders: new Set<() => void>(), mounted: false, @@ -49,6 +51,7 @@ vi.mock("./playlistsContext", async () => { return { usePlaylists: () => ({ selectedId: useSyncExternalStore(subscribe, () => h.selectedId), + linkedId: h.linkedId, search: "", }), usePlaylistsActions: () => ({ setSearch: () => {} }), @@ -136,8 +139,17 @@ function mount( { strictOverCache = false, detail, - }: { strictOverCache?: boolean; detail?: (id: number) => Promise } = {}, + linked = h.selectedId, + listIn = false, + }: { + strictOverCache?: boolean; + detail?: (id: number) => Promise; + linked?: number | null; + /** The account's list already in, as the rail that fetched it leaves it. */ + listIn?: boolean; + } = {}, ) { + h.linkedId = linked; vi.spyOn(api, "playlists").mockResolvedValue( Object.entries(lists).map(([id, items]) => list(Number(id), items)) as never, ); @@ -146,6 +158,11 @@ function mount( ); qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); qc.setQueryData(qk.me(), { id: 1, can_write: true, prefs: {}, preferences: prefs }); + if (listIn) + qc.setQueryData( + qk.playlists(), + Object.entries(lists).map(([id, items]) => list(Number(id), items)), + ); // Coming back to the page: the selected list is already cached, and StrictMode (dev) runs its // effects twice on that mount. if (strictOverCache && h.selectedId != null) @@ -248,6 +265,37 @@ describe("Playlists — a player the address names", () => { expect(new URLSearchParams(window.location.search).get("watch")).toBe("Z"); }); + it("plays alone over a playlist this account does not hold, the rail moved on before the page came", async () => { + window.history.replaceState(null, "", "/?page=playlists&pl=1&watch=Z"); + // The rail's keep-valid step ran before the page mounted: the selection is already 1, and 1 even + // holds the video — it is still not the playlist the link named. + h.selectedId = 1; + vi.spyOn(api, "videoRow").mockResolvedValue(vid("Z", 0)); + mount({ 1: [vid("A", 0), vid("Z", 0)] }, {}, { linked: 7, listIn: true }); + await waitFor(() => expect(h.player?.video.id).toBe("Z")); + expect(h.player!.queue?.map((v) => v.id)).toEqual(["Z"]); + }); + + it("gives up over a playlist this account does not hold once another one is picked", async () => { + window.history.replaceState(null, "", "/?page=playlists&pl=7&watch=Z"); + h.selectedId = 7; + let answer: (v: Video) => void = () => {}; + vi.spyOn(api, "videoRow").mockReturnValue(new Promise((r) => (answer = r)) as never); + const { rerender } = mount( + { 1: [vid("A", 0)], 2: [vid("B", 0)] }, + {}, + { detail: async (id) => (id === 7 ? new Promise(() => {}) : list(id, [])) }, + ); + await waitFor(() => expect(qc.getQueryData(qk.playlists())).toBeDefined()); + h.selectedId = 1; // the rail's own move + rerender(); + h.selectedId = 2; // a pick, before the video's row came + rerender(); + act(() => answer(vid("Z", 0))); + await new Promise((r) => setTimeout(r, 20)); + expect(h.player).toBeNull(); + }); + it("plays alone when the linked playlist's read is refused", async () => { window.history.replaceState(null, "", "/?page=playlists&pl=1&watch=Z"); vi.spyOn(api, "videoRow").mockResolvedValue(vid("Z", 0)); diff --git a/frontend/src/modules/playlists/Playlists.tsx b/frontend/src/modules/playlists/Playlists.tsx index 5aad7d0b..aa020e50 100644 --- a/frontend/src/modules/playlists/Playlists.tsx +++ b/frontend/src/modules/playlists/Playlists.tsx @@ -192,7 +192,7 @@ export default function Playlists() { function PlaylistDetail() { // The selected playlist lives in PlaylistsProvider, shared with the PlaylistsRail (which owns the // list + its rail controls). This detail pane reacts to the selection. - const { selectedId } = usePlaylists(); + const { selectedId, linkedId } = usePlaylists(); const { t, i18n } = useTranslation(); const lang = i18n.language; const qc = useQueryClient(); @@ -586,11 +586,14 @@ function PlaylistDetail() { // (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. - // One the account's list does not hold (deleted since, another account's link) is out of reach - // as soon as that list is in: the rail then moves to another one by itself, which is no pick. - const [linkedOver] = useState(selectedId); + // Linked over the playlist the store started with, not the selection this page mounted on: the rail + // may have moved that already. One the account's list does not hold (deleted since, another + // account's link) is out of reach as soon as that list is in: the rail then moves to the first one + // by itself, which is no pick — any other is. + const linkedOver = linkedId; const linkedOutOfReach = listQuery.data != null && !listQuery.data.some((p) => p.id === linkedOver); + const railMovedTo = listQuery.data?.[0]?.id ?? null; const linkedListIn = linkedOutOfReach || (detail != null && (detail.items.length === 0 || items.length > 0)) || @@ -605,7 +608,7 @@ function PlaylistDetail() { setPlaying({ video, queue: [video], index: 0, listId, editable: false }); } }, - linkedOutOfReach ? linkedOver : selectedId, + linkedOutOfReach && selectedId === railMovedTo ? linkedOver : 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 f8ee283c..2a6c1ea3 100644 --- a/frontend/src/modules/playlists/PlaylistsProvider.tsx +++ b/frontend/src/modules/playlists/PlaylistsProvider.tsx @@ -31,6 +31,7 @@ export function PlaylistsProvider({ children }: { children: ReactNode }) { () => new URLSearchParams(window.location.search).get("plq") ?? "", ); const [selectedId, setSelectedIdState] = useState(readSelectedId); + const [linkedId] = useState(readSelectedId); const setSearch = useCallback((q: string) => setSearchState(q), []); const setSelectedId = useCallback((id: number | null) => { @@ -46,12 +47,16 @@ export function PlaylistsProvider({ children }: { children: ReactNode }) { const shown = urlPage(nav) === "playlists"; // Leaving the page ends the search: adjusted during the render that leaves it. if (nav.page !== "playlists" && search) setSearchState(""); - // An emptied box at once: back on the page within the pause, the address would name it again. - const debounced = useDebounced(search.trim(), 300); - const typed = search.trim() ? debounced : ""; + // A box emptied and filled again, each at once: the pause would name the ended search again, on + // its way out or before the new text settled. + const q = search.trim(); + const typed = useDebounced(q, 300, q === ""); useUrlMirror(shown ? playlistsParams(selectedId, typed) : playlistsParams(null, "")); - const state = useMemo(() => ({ search, selectedId }), [search, selectedId]); + const state = useMemo( + () => ({ search, selectedId, linkedId }), + [search, selectedId, linkedId], + ); const actions = useMemo( () => ({ setSearch, setSelectedId }), [setSearch, setSelectedId], diff --git a/frontend/src/modules/playlists/playlistsContext.ts b/frontend/src/modules/playlists/playlistsContext.ts index 89dc9e7a..55d785f8 100644 --- a/frontend/src/modules/playlists/playlistsContext.ts +++ b/frontend/src/modules/playlists/playlistsContext.ts @@ -6,6 +6,9 @@ import { createContext, useContext } from "react"; export type PlaylistsState = { search: string; selectedId: number | null; + /** The selection the store started with — the playlist a reload or a link named. A player the + * address names opens over that one: the rail may have moved the selection before the page came. */ + linkedId: number | null; }; export interface PlaylistsActions { @@ -16,6 +19,7 @@ export interface PlaylistsActions { export const PlaylistsStateContext = createContext({ search: "", selectedId: null, + linkedId: null, }); export const PlaylistsActionsContext = createContext({ setSearch: () => {},