diff --git a/backend/app/routes/search.py b/backend/app/routes/search.py index bc1bf444..3f96cd4b 100644 --- a/backend/app/routes/search.py +++ b/backend/app/routes/search.py @@ -314,7 +314,8 @@ def search_youtube( class ClearSearchIn(BaseModel): - video_ids: list[str] = [] + # Required: a default read a misnamed key as an empty list, a 200 that cleared nothing. + video_ids: list[str] @router.post("/clear") diff --git a/backend/tests/test_route_commits.py b/backend/tests/test_route_commits.py index 6491fec5..8f83c3e0 100644 --- a/backend/tests/test_route_commits.py +++ b/backend/tests/test_route_commits.py @@ -310,6 +310,14 @@ def test_clearing_leaves_a_video_the_caller_never_found(db, make_user, as_user): assert r.json() == {"ok": True, "removed": 0} +def test_clearing_refuses_a_body_without_its_list(db, make_user, as_user): + # A misnamed key read as an empty list: 200, and nothing cleared. + user = make_user() + r = as_user(user).post("/api/search/clear", json={"ids": ["s1"]}) + assert r.status_code == 422 + assert as_user(user).post("/api/search/clear", json={"video_ids": []}).status_code == 200 + + # --- Plex show and season marks --------------------------------------------------------------- diff --git a/frontend/src/i18n/locales/en/playlists.json b/frontend/src/i18n/locales/en/playlists.json index 56bf33a9..619e565a 100644 --- a/frontend/src/i18n/locales/en/playlists.json +++ b/frontend/src/i18n/locales/en/playlists.json @@ -120,7 +120,8 @@ "removeNoneLeft_one": "Nothing removed: it isn’t in this playlist any more", "removeNoneLeft_other": "Nothing removed: they aren’t in this playlist any more", "selectionCleared": "Selection cleared", - "removeAllHidden": "The selected rows are hidden by the filter", + "removeAllHidden_one": "The selected row is hidden by the filter", + "removeAllHidden_other": "The selected rows are hidden by the filter", "undoNoneBack_one": "Nothing came back: that video isn’t where it was any more", "undoNoneBack_other": "Nothing came back: those videos aren’t where they were any more", "undoSomeBack_one": "Only {{count}} video came back: the rest aren’t where they were any more", diff --git a/frontend/src/i18n/locales/hu/playlists.json b/frontend/src/i18n/locales/hu/playlists.json index ee10141f..6c7c4aab 100644 --- a/frontend/src/i18n/locales/hu/playlists.json +++ b/frontend/src/i18n/locales/hu/playlists.json @@ -120,7 +120,8 @@ "removeNoneLeft_one": "Nem történt eltávolítás: ez már nincs ebben a listában", "removeNoneLeft_other": "Nem történt eltávolítás: ezek már nincsenek ebben a listában", "selectionCleared": "Kijelölés törölve", - "removeAllHidden": "A kijelölt sorokat a szűrő elrejti", + "removeAllHidden_one": "A kijelölt sort a szűrő elrejti", + "removeAllHidden_other": "A kijelölt sorokat a szűrő elrejti", "undoNoneBack_one": "Semmi nem került vissza: ez a videó már nincs ott, ahol volt", "undoNoneBack_other": "Semmi nem került vissza: ezek a videók már nincsenek ott, ahol voltak", "undoSomeBack_one": "Csak {{count}} videó került vissza: a többi már nincs ott, ahol volt", diff --git a/frontend/src/modules/playlists/Playlists.dom.test.tsx b/frontend/src/modules/playlists/Playlists.dom.test.tsx index 7d3ec084..deb5e9b1 100644 --- a/frontend/src/modules/playlists/Playlists.dom.test.tsx +++ b/frontend/src/modules/playlists/Playlists.dom.test.tsx @@ -1167,8 +1167,12 @@ describe("Playlists — selecting rows", () => { }); it("drops a row removed from the list out of the selection", async () => { - vi.spyOn(api, "removeManyFromPlaylist").mockResolvedValue(wentOut("A") as never); - mount(three); + const lists = { 1: three[1]! }; + vi.spyOn(api, "removeManyFromPlaylist").mockImplementation((async () => { + lists[1] = lists[1].filter((v) => v.id !== "A"); + return wentOut("A"); + }) as never); + mount(lists); fireEvent.click(await box("Video A")); fireEvent.click(await box("Video B")); await rowMenu("Video A", "playlists.removeItem"); @@ -2034,7 +2038,7 @@ describe("Playlists — acting on the selected rows", () => { await screen.findByText("playlists.selectedHidden"); (await screen.findByRole("button", { name: "card.play — Video B" })).focus(); await act(async () => press("Delete")); - expect(toast().message).toEqual(msg("playlists.removeAllHidden")); + expect(toast().message).toEqual(msg("playlists.removeAllHidden", { count: 1 })); expect(remove).not.toHaveBeenCalled(); expect(rowsOnScreen()).toEqual(["Video B"]); }); @@ -2269,6 +2273,11 @@ describe("Playlists — writes that overlap", () => { const inPage = () => screen.findByRole("button", { name: "card.play — Video A" }); const reads = () => vi.mocked(api.playlist).mock.calls.length; const undoOrder = () => screen.getByRole("button", { name: "common.undo" }) as HTMLButtonElement; + // The detail the server sends holds no count of its rows, so a refresh can match the cache exactly. + const sameShape = (lists: Record) => async (id: number) => ({ + ...list(id, lists[id] ?? []), + item_count: 0, + }); const lastUndo = () => h.notify.mock.calls .map(([n]) => n as { action?: { onClick: () => void } }) @@ -2420,6 +2429,191 @@ describe("Playlists — writes that overlap", () => { expect(undoOrder().disabled).toBe(false); }); + it("shows the server's order when a removal fails and then a reorder on its way is accepted", async () => { + const put = later(); + const post = later(); + vi.spyOn(api, "reorderPlaylist").mockReturnValue(put.promise as never); + vi.spyOn(api, "removeManyFromPlaylist").mockReturnValue(post.promise as never); + const lists = abc(); + mount(lists); + await inPage(); + await landed(); + await rowMenu("Video B", "common.order.up"); + await waitFor(() => expect(api.reorderPlaylist).toHaveBeenCalled()); + await removeRow("Video C", "Video B", "Video A"); + await act(async () => post.no(new TypeError("Failed to fetch"))); + await landed(); + lists[1] = [vid("B", 0), vid("A", 0), vid("C", 0)]; + await act(async () => put.ok(undefined)); + await waitFor(() => expect(rowsOnScreen()).toEqual(["Video B", "Video A", "Video C"])); + }); + + it("puts a row back by the order on screen when a reorder was accepted before its removal failed", async () => { + const put = later(); + const post = later(); + vi.spyOn(api, "reorderPlaylist").mockReturnValue(put.promise as never); + vi.spyOn(api, "removeManyFromPlaylist").mockReturnValue(post.promise as never); + const lists = abc(); + mount(lists); + await inPage(); + await landed(); + await rowMenu("Video B", "common.order.up"); + await waitFor(() => expect(api.reorderPlaylist).toHaveBeenCalled()); + await removeRow("Video C", "Video B", "Video A"); + lists[1] = [vid("B", 0), vid("A", 0), vid("C", 0)]; + await act(async () => put.ok(undefined)); + await landed(); + vi.mocked(api.playlist).mockReturnValue(new Promise(() => {}) as never); + await act(async () => post.no(new TypeError("Failed to fetch"))); + await landed(); + expect(rowsOnScreen()).toEqual(["Video B", "Video A", "Video C"]); + }); + + it("shows the server's order when a reorder fails with the next one on its way", async () => { + const first = later(); + const second = later(); + vi.spyOn(api, "reorderPlaylist") + .mockReturnValueOnce(first.promise as never) + .mockReturnValueOnce(second.promise as never); + const lists = abc(); + mount(lists); + await inPage(); + await landed(); + await rowMenu("Video C", "common.order.up"); + await waitFor(() => expect(rowsOnScreen()).toEqual(["Video A", "Video C", "Video B"])); + await rowMenu("Video C", "common.order.up"); + await waitFor(() => expect(rowsOnScreen()).toEqual(["Video C", "Video A", "Video B"])); + await act(async () => first.no(new TypeError("Failed to fetch"))); + await landed(); + lists[1] = [vid("C", 0), vid("A", 0), vid("B", 0)]; + await act(async () => second.ok(undefined)); + await waitFor(() => expect(rowsOnScreen()).toEqual(["Video C", "Video A", "Video B"])); + }); + + it("keeps a row whose removal failed, ticked, when a reorder that left it out is accepted", async () => { + const put = later(); + const post = later(); + vi.spyOn(api, "reorderPlaylist").mockReturnValue(put.promise as never); + vi.spyOn(api, "removeManyFromPlaylist").mockReturnValue(post.promise as never); + mount({ 1: [vid("A", 0), vid("C", 0), vid("B", 0)] }); + await inPage(); + await landed(); + const row = await screen.findByRole("checkbox", { name: "playlists.selectRow:Video C" }); + fireEvent.click(row); + row.focus(); + await act(async () => { + fireEvent.keyDown(row, { key: "Delete", bubbles: true }); + }); + await waitFor(() => expect(rowsOnScreen()).toEqual(["Video A", "Video B"])); + await rowMenu("Video B", "common.order.up"); + await waitFor(() => expect(api.reorderPlaylist).toHaveBeenCalled()); + await act(async () => post.no(new TypeError("Failed to fetch"))); + await landed(); + vi.mocked(api.playlist).mockReturnValue(new Promise(() => {}) as never); + await act(async () => put.ok(undefined)); + await landed(); + expect(rowsOnScreen()).toContain("Video C"); + const box = screen.getByRole("checkbox", { name: "playlists.selectRow:Video C" }); + expect((box as HTMLInputElement).checked).toBe(true); + }); + + it("keeps a reorder made while the read after a failure is on its way", async () => { + const put = later(); + vi.spyOn(api, "reorderPlaylist").mockReturnValue(put.promise as never); + vi.spyOn(api, "removeManyFromPlaylist").mockRejectedValue(new TypeError("Failed to fetch")); + mount(abc()); + await inPage(); + await landed(); + const read = later(); + vi.mocked(api.playlist).mockReturnValue(read.promise as never); + await rowMenu("Video C", "playlists.removeItem"); + await waitFor(() => expect(h.notify).toHaveBeenCalled()); + await landed(); + await rowMenu("Video B", "common.order.up"); + await waitFor(() => expect(api.reorderPlaylist).toHaveBeenCalled()); + await act(async () => read.ok(list(1, abc()[1]))); + await landed(); + expect(rowsOnScreen()).toEqual(["Video B", "Video A", "Video C"]); + }); + + it("gives an Undo the place the row had on screen, with a reorder still on its way", async () => { + const put = later(); + vi.spyOn(api, "reorderPlaylist").mockReturnValue(put.promise as never); + vi.spyOn(api, "removeManyFromPlaylist").mockResolvedValue(wentOut("C") as never); + vi.spyOn(api, "restoreToPlaylist").mockResolvedValue({ ok: true, added: 1 } as never); + const lists: Record = { 1: [...abc()[1], vid("D", 0)] }; + mount(lists); + await inPage(); + await landed(); + await rowMenu("Video D", "common.order.up"); + await waitFor(() => expect(api.reorderPlaylist).toHaveBeenCalled()); + await removeRow("Video C", "Video A", "Video B", "Video D"); + lists[1] = [vid("A", 0), vid("B", 0), vid("D", 0)]; + await act(async () => put.ok(undefined)); + await waitFor(() => expect(lastUndo()).toBeTruthy()); + await act(async () => lastUndo()!.action!.onClick()); + await waitFor(() => expect(api.restoreToPlaylist).toHaveBeenCalledWith(1, ["C"], [3])); + }); + + it("takes a stored position on screen while a write is out, the refresh changing nothing", async () => { + const write = later(); + vi.spyOn(api, "removeManyFromPlaylist").mockReturnValue(write.promise as never); + const lists: Record = { 1: [vid("U", 0), vid("V", 0)] }; + mount(lists, {}, { detail: sameShape(lists) }); + await screen.findByRole("button", { name: "card.play — Video U" }); + await landed(); + await removeRow("Video V", "Video U"); + act(() => patchCachedProgress(qc, new Map([["U", 830]]))); + await landed(); + lists[1] = [vid("U", 830)]; + await act(async () => write.ok(wentOut("V"))); + await landed(); + await play("Video U"); + await waitFor(() => expect(h.player?.video.position_seconds).toBe(830)); + }); + + it("shows a row a read brought during a write, the refresh changing nothing", async () => { + const write = later(); + vi.spyOn(api, "removeManyFromPlaylist").mockReturnValue(write.promise as never); + const lists: Record = abc(); + mount(lists, {}, { detail: sameShape(lists) }); + await inPage(); + await landed(); + await removeRow("Video A", "Video B", "Video C"); + lists[1] = [vid("B", 0), vid("C", 0), vid("X", 0)]; + await act(async () => { + await qc.refetchQueries({ queryKey: qk.playlist(1) }); + }); + await landed(); + expect(rowsOnScreen()).toEqual(["Video B", "Video C"]); + await act(async () => write.ok(wentOut("A"))); + await waitFor(() => expect(rowsOnScreen()).toEqual(["Video B", "Video C", "Video X"])); + }); + + it("keeps a row out, and the history, when a read from before its removal landed and a patch comes after", async () => { + vi.spyOn(api, "reorderPlaylist").mockResolvedValue(undefined as never); + const write = later(); + vi.spyOn(api, "removeManyFromPlaylist").mockReturnValue(write.promise as never); + const lists = abc(); + mount(lists); + await inPage(); + await landed(); + await rowMenu("Video C", "common.order.up"); + await waitFor(() => expect(undoOrder().disabled).toBe(false)); + lists[1] = [vid("A", 0), vid("C", 0), vid("B", 0)]; + await landed(); + await removeRow("Video A", "Video C", "Video B"); + await act(async () => { + await qc.refetchQueries({ queryKey: qk.playlist(1) }); + }); + vi.mocked(api.playlist).mockReturnValue(new Promise(() => {}) as never); + await act(async () => write.ok(wentOut("A"))); + act(() => patchCachedProgress(qc, new Map([["B", 30]]))); + await landed(); + expect(rowsOnScreen()).toEqual(["Video C", "Video B"]); + expect(undoOrder().disabled).toBe(false); + }); + describe("from the player, on a list the cache has dropped", () => { const dropped = async () => { const lists: Record = { @@ -2579,4 +2773,143 @@ describe("Playlists — the focus when what holds it goes", () => { await waitFor(() => expect((group as HTMLInputElement).checked).toBe(false)); expect(document.activeElement).toBe(group); }); + + it("takes the header's box when the page menu's grouping row goes with the view", async () => { + let open: PageMenuOpener | null = null; + window.history.replaceState(null, "", "/?page=playlists&pl=1&plgroup=1"); + const items = [vid("A", 0)]; + vi.spyOn(api, "playlists").mockResolvedValue([list(1, items)] as never); + vi.spyOn(api, "playlist").mockImplementation(async () => list(1, items) as never); + qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + qc.setQueryData(qk.me(), { id: 1, can_write: true, prefs: {}, preferences: {} }); + const slot = document.createElement("div"); + document.body.appendChild(slot); + render( + + + { + open = o; + return () => { + if (open === o) open = null; + }; + }} + > + + + + , + ); + await screen.findByRole("button", { name: "card.play — Video A" }); + act(() => open!(5, 5, document.body)); + const row = await waitFor(() => { + const found = [...document.querySelectorAll("[role^=menuitem]")].find( + (b) => b.textContent === "playlists.groupByChannel", + ); + expect(found).toBeTruthy(); + return found!; + }); + row.focus(); + fireEvent.click(row); + await waitFor(() => expect(document.activeElement).toBe(headerBox())); + await waitFor(() => expect(document.querySelector("[role^=menuitem]")).toBeNull()); + expect(document.activeElement).toBe(headerBox()); + }); + + describe("when the player closes", () => { + let card: HTMLElement; + const openFrom = async (title: string) => { + const opener = await screen.findByRole("button", { name: `card.play — ${title}` }); + opener.focus(); + fireEvent.click(opener); + await waitFor(() => expect(h.mounted).toBe(true)); + // The player holds the focus on a card of its own, outside the page, and takes it along. + card = document.createElement("div"); + card.tabIndex = -1; + document.body.appendChild(card); + card.focus(); + return opener; + }; + const close = () => + act(() => { + card.remove(); + h.player!.onClose(); + }); + const removeFromPlayer = (v: Video) => + act(async () => { + await h.player!.onRemoveFromList!(v); + }); + // The server takes the row out, so the refresh after it does not bring it back. + const mountList = (rows: Video[], gone: string) => { + const lists = { 1: rows }; + vi.spyOn(api, "removeManyFromPlaylist").mockImplementation((async () => { + lists[1] = lists[1].filter((v) => v.id !== gone); + return wentOut(gone); + }) as never); + mount(lists); + }; + + it("takes the row in the place of the one that opened it, which is gone", async () => { + mountList([vid("A", 0), vid("B", 0), vid("C", 0)], "A"); + await openFrom("Video A"); + await removeFromPlayer(vid("A", 0)); + close(); + await waitFor(() => + expect(document.activeElement?.closest("tr")?.textContent).toContain("Video B"), + ); + }); + + it("takes the new last row when the one that opened it was the last", async () => { + mountList([vid("A", 0), vid("B", 0)], "B"); + await openFrom("Video B"); + await removeFromPlayer(vid("B", 0)); + close(); + await waitFor(() => + expect(document.activeElement?.closest("tr")?.textContent).toContain("Video A"), + ); + }); + + it("takes the header's box when a filter shows no row any more", async () => { + window.history.replaceState( + null, + "", + `/?page=playlists&pl=1&plf=${encodeURIComponent(JSON.stringify({ title: "Video B" }))}`, + ); + mountList([vid("A", 0), vid("B", 0)], "B"); + await openFrom("Video B"); + await removeFromPlayer(vid("B", 0)); + close(); + await waitFor(() => expect(document.activeElement).toBe(headerBox())); + }); + + it("takes the list's '…' when the list is empty", async () => { + mountList([vid("A", 0)], "A"); + await openFrom("Video A"); + await removeFromPlayer(vid("A", 0)); + close(); + await waitFor(() => expect(document.activeElement).not.toBe(document.body)); + expect(document.activeElement?.closest("table")).toBeNull(); + }); + + it("moves nothing while the one that opened it is still there", async () => { + mount({ 1: [vid("A", 0), vid("B", 0)] }); + await openFrom("Video B"); + close(); + await waitFor(() => expect(h.mounted).toBe(false)); + await act(async () => void (await new Promise((r) => setTimeout(r, 20)))); + expect(document.activeElement).toBe(document.body); + }); + + it("leaves a focus the reader has put elsewhere", async () => { + mountList([vid("A", 0), vid("B", 0)], "A"); + await openFrom("Video A"); + await removeFromPlayer(vid("A", 0)); + const box = headerBox(); + box.focus(); + close(); + await waitFor(() => expect(h.mounted).toBe(false)); + await act(async () => void (await new Promise((r) => setTimeout(r, 20)))); + expect(document.activeElement).toBe(box); + }); + }); }); diff --git a/frontend/src/modules/playlists/Playlists.tsx b/frontend/src/modules/playlists/Playlists.tsx index d02a1e2d..8bc34054 100644 --- a/frontend/src/modules/playlists/Playlists.tsx +++ b/frontend/src/modules/playlists/Playlists.tsx @@ -2,6 +2,7 @@ import { lazy, Suspense, useEffect, + useEffectEvent, useLayoutEffect, useMemo, useRef, @@ -86,7 +87,7 @@ import { usePlaylistActions } from "./usePlaylistActions"; import { usePlaylists } from "./playlistsContext"; import { railDefaultId } from "./playlistsInit"; import { usePref } from "../../lib/usePref"; -import { followRemoval, navItemAttrs } from "../../lib/listNav"; +import { focusNavItem, followRemoval, navItemAttrs } from "../../lib/listNav"; import { ColumnHead } from "../../components/ui/tableHeader"; import { alignClass } from "../../components/ui/tableAlign"; import { @@ -306,18 +307,24 @@ function PlaylistDetail() { const [renaming, setRenaming] = useState(false); const { target: renameTarget, arm: armRenameReturn } = useReturnFocus(renaming); const [renameValue, setRenameValue] = useState(""); - // The rows leaving each list right now, and the list as it stood when the first of them left. A read - // can answer from before a write that is still out, so while one is out the list is not read again - // and what a read brings is not taken on screen. The list as it stood is for all of those writes: - // the later ones' own view of it lacks the rows the earlier ones took, and so their neighbours. + // The writes out on each list right now (removals, moves, reorders), and the list as the reader last + // saw it, with the rows those writes took still in their places: a later write's own view lacks them, + // and so their neighbours. A read can answer from before a write that is still out: a write's own + // refresh waits for the last of them, but others still start meanwhile (a video's state, a return to + // the tab). So while one is out, what a read brings moves no row on screen and takes none in or out; + // only a row's own fields, a stored position, are taken. const writesOut = useRef(new Map()); const refreshDue = useRef(new Set()); + // A rollback sets rows back by what the cache held, which an accepted reorder may have moved on from, + // and the membership guard never corrects an order: the read after the last write decides it. + const orderInDoubt = useRef(false); // The stored order is undoable: drag and "save this order" both go through `order.set`, so each // is reversible (buttons + Ctrl+Z/Y). onApply persists the new order to the server. const order = useUndoable([], { onApply: (next) => { if (selectedId == null) return; const listId = selectedId; + writeStarts(listId); api .reorderPlaylist( listId, @@ -329,11 +336,21 @@ function PlaylistDetail() { // from before this reorder — so a SECOND reorder failing in that window rolled the rows // back to an order the server does not have, permanently (the membership guard cannot // correct an order). One accepted write, one cache entry that matches it. - // A row taken out while this was on its way stays out: `next` is from before that. + // A row taken out while this was on its way stays out: `next` is from before that. A row it + // does not name stays too, after the ones it does, where the server keeps it: one whose + // removal failed meanwhile, or that came in from elsewhere. + writeDone(listId); qc.setQueryData(qk.playlist(listId), (old) => { if (!old) return old; const held = new Set(old.items.map((v) => v.id)); - return { ...old, items: next.filter((v) => held.has(v.id)) }; + const named = new Set(next.map((v) => v.id)); + return { + ...old, + items: [ + ...next.filter((v) => held.has(v.id)), + ...old.items.filter((v) => !named.has(v.id)), + ], + }; }); // Refresh the sidebar (cover may change) and the detail (so the now-dirty state, // and the Reset/Unsynced indicators, show without an F5). The membership guard @@ -345,6 +362,7 @@ function PlaylistDetail() { // rejected, and produced an unhandled rejection. Say so, and put back the order the server // still has — or the rejected one becomes the base for the next drag. notifyYouTubeActionError(e, msg("playlists.reorderFailed")); + writeDone(listId); restoreFromCache(listId); refreshAfterEdit([]); }); @@ -375,6 +393,7 @@ function PlaylistDetail() { if (!cur) return; lastSetRef.current = `${listId}:${idsKey(cur.items)}`; order.reset(cur.items); + orderInDoubt.current = true; } // What the player was LAUNCHED with, snapshotted when it opens: the video, the queue and the index // inside it. `shown` is derived (sort / filter / view), and the player freezes its queue and owns @@ -448,15 +467,13 @@ function PlaylistDetail() { if (!detail) return; // A write out on the list these rows are from: this may be a read from before it landed, and the // refresh after the last write brings what is true. A list only just picked has no rows yet. - if ( + const held = writesOut.current.has(selectedId as number) && - lastSetRef.current.startsWith(`${selectedId}:`) - ) - return; + lastSetRef.current.startsWith(`${selectedId}:`); // Scoped to the playlist: two lists holding the SAME videos would otherwise look identical to // the guard, so the second would render in the first's order and inherit its undo stack. const key = `${selectedId}:${idsKey(detail.items)}`; - if (key !== lastSetRef.current) { + if (!held && key !== lastSetRef.current) { lastSetRef.current = key; resetOrder(detail.items); } else { @@ -504,12 +521,15 @@ function PlaylistDetail() { const shownRef = useRef([]); const removeRef = useRef(async () => false); + // What the player was opened from, for when `Trash` in it has taken that row by the time it closes. + const openedFrom = useRef<{ el: Element | null; index: number } | null>(null); // Stable (the memoized row callbacks depend on it), so it reads `shown` through the ref rather // than closing over it. const openPlayer = useStableCallback((id: string) => { const queue = shownRef.current; const index = queue.findIndex((v) => v.id === id); if (index < 0) return; + openedFrom.current = { el: document.activeElement, index }; setPlaying({ video: queue[index]!, queue, @@ -644,9 +664,10 @@ function PlaylistDetail() { } // The grouping row is only there for a list long enough to group, or a view to take off: with this // the last of the view on a short list, it goes too. + const groupingRowGoes = (on: boolean) => + !on && items.length <= 1 && isStoredSort(viewSort) && !filterActive; function setGrouping(on: boolean) { - if (!on && items.length <= 1 && isStoredSort(viewSort) && !filterActive) - (selectAllRef.current ?? renameTarget.current)?.focus(); + if (groupingRowGoes(on)) (selectAllRef.current ?? renameTarget.current)?.focus(); setGroupBy(on); } @@ -768,9 +789,38 @@ function PlaylistDetail() { linkedOutOfReach && selectedId === railMovedTo ? linkedOver : selectedId, ); - /** Off the screen at once. Returns the cached list as it stood: the place an Undo goes back to, and + // The player has handed the focus back: to a row gone by now it could not, and the next Tab would + // start over at the top of the page. The row now in that place takes it, else the header's box, else + // (no rows, no table) the list's "…". A focus the reader has put elsewhere stays. + const focusAfterPlayer = useEffectEvent(() => { + const from = openedFrom.current; + openedFrom.current = null; + if (!from || from.el?.isConnected || document.activeElement !== document.body) return; + const rows = shownRef.current.length; + if (rows && tableRef.current) focusNavItem(tableRef.current, Math.min(from.index, rows - 1)); + else (selectAllRef.current ?? renameTarget.current)?.focus(); + }); + const playerOpen = playing != null; + useEffect(() => { + if (!playerOpen) focusAfterPlayer(); + }, [playerOpen]); + + /** The list as the reader sees it now, the rows `earlier` writes took back in their places. */ + function listAsSeen(listId: number, earlier: PlaylistDetail | undefined) { + const cached = qc.getQueryData(qk.playlist(listId)); + const onScreen = listId === selectedRef.current && lastSetRef.current.startsWith(`${listId}:`); + if (!cached || !onScreen) return earlier ?? cached; + const seen = { ...cached, items: order.value }; + if (!earlier) return seen; + const shownNow = new Set(order.value.map((v) => v.id)); + const taken = earlier.items.filter((v) => !shownNow.has(v.id)).map((v) => v.id); + return withRowsBack(seen, earlier, taken); + } + /** Off the screen at once. Returns the list as the reader saw it: the place an Undo goes back to, and * what a failure puts back. */ function takeOut(listId: number, ids: readonly string[]) { + const out = writesOut.current.get(listId); + const before = listAsSeen(listId, out?.before); const gone = new Set(ids); // Only the list ON SCREEN has rows to take out optimistically — the open player can be removing // from the list it was launched from while another one is selected. @@ -796,20 +846,32 @@ function PlaylistDetail() { // so the guard reset the order: the removed row came back and the undo history the rebase had // kept went with it. (A failure puts it back: `putBack`.) A read on its way would do the same when // it lands, so it is called off; the write's own refresh replaces it. - const out = writesOut.current.get(listId); - const before = out ? out.before : qc.getQueryData(qk.playlist(listId)); writesOut.current.set(listId, { count: (out?.count ?? 0) + 1, before }); void qc.cancelQueries({ queryKey: qk.playlist(listId) }); + dropFromCache(listId, ids); + return before; + } + function dropFromCache(listId: number, ids: readonly string[]) { + const gone = new Set(ids); qc.setQueryData(qk.playlist(listId), (old) => old ? { ...old, items: old.items.filter((v) => !gone.has(v.id)) } : old, ); - return before; + } + function writeStarts(listId: number) { + const out = writesOut.current.get(listId); + writesOut.current.set(listId, { count: (out?.count ?? 0) + 1, before: out?.before }); } function writeDone(listId: number) { const out = writesOut.current.get(listId); if (out && out.count > 1) out.count--; else writesOut.current.delete(listId); } + // The write went through: a read from before it may have brought its rows back into the cache since, + // and the next thing to rewrite that entry would hand them to the membership guard. + function writeWentThrough(listId: number, ids: readonly string[]) { + dropFromCache(listId, ids); + writeDone(listId); + } // The write failed: the rows come back now (see `restoreFromCache`), ticked again if they were, so // the reader can try once more without picking them all over. function putBack( @@ -832,10 +894,22 @@ function PlaylistDetail() { for (const id of ids) refreshDue.current.add(id); if (writesOut.current.size) return; qc.invalidateQueries({ queryKey: qk.playlists() }); - qc.invalidateQueries({ queryKey: qk.playlist() }); + const read = qc.invalidateQueries({ queryKey: qk.playlist() }); for (const id of refreshDue.current) qc.invalidateQueries({ queryKey: qk.playlistMembership(id) }); refreshDue.current.clear(); + void read.then(settleOrder); + } + // The refresh is in. A read matching the cache exactly runs no effect, so what the cache got while a + // write was out is taken here, and so is the order after a rollback. A write started since has its + // own refresh to wait for. + function settleOrder() { + const listId = selectedRef.current; + if (listId == null || writesOut.current.size) return; + const cur = qc.getQueryData(qk.playlist(listId)); + const changed = cur && `${listId}:${idsKey(cur.items)}` !== lastSetRef.current; + if (orderInDoubt.current || changed) restoreFromCache(listId); + orderInDoubt.current = false; } const titleOf = (rows: Video[], id: string | undefined) => raw(rows.find((v) => v.id === id)?.title ?? ""); @@ -882,7 +956,8 @@ function PlaylistDetail() { // fault `req` already put in its error dialog is not toasted again (`notifyActionError` checks). notifyActionError(e, failTitle); } - writeDone(listId); + if (done) writeWentThrough(listId, ids); + else writeDone(listId); if (!done) { putBack(listId, before, ids, ticked); refreshAfterEdit(ids); @@ -972,7 +1047,8 @@ function PlaylistDetail() { // `notifyActionError` checks. notifyActionError(e, failTitle); } - writeDone(listId); + if (removed) writeWentThrough(listId, ids); + else writeDone(listId); // …and the rows go back NOW, not when the resync lands: a removal that failed because the server // was unreachable has a resync that fails too. if (removed == null) putBack(listId, before, ids, ticked); @@ -1304,7 +1380,11 @@ function PlaylistDetail() { id: "groupBy", label: t("playlists.groupByChannel"), check: { kind: "many", checked: groupBy }, - onSelect: () => setGroupBy(!groupBy), + // A `many` row keeps its menu open, for a row that stays to be pressed again. + onSelect: () => { + if (groupingRowGoes(!groupBy)) pageMenu.close(); + setGrouping(!groupBy); + }, }, ...(viewActive ? [{ id: "clearView", label: t("playlists.clearView"), onSelect: clearView }] @@ -1653,7 +1733,9 @@ function PlaylistDetail() { else notify({ level: "warning", - message: msg("playlists.removeAllHidden"), + message: msg("playlists.removeAllHidden", { + count: selection.hiddenCount, + }), }); return true; },