From 84db57095196855a09533a6276fce0024edb1c26 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 25 Oct 2026 04:01:43 +0100 Subject: [PATCH] fix(player): marks give way to newer lists, Watch later items mark, and auto-advance has its own glyphs A session's marks are weighed per field against when each page of a never-refetched list was read: a page read before the mark (a Show more / Load more on its way) takes it, one read after keeps the server's word. The live search lays them over its rows too, not only the related list. Items of the built-in Watch later list ARE the bookmark, so adding, removing and moving them go through features/video/videoWrites and mark the video; the contract test covers those writes and catches a write that is named without being called on the spot. The player's watched toggle follows a mark confirmed elsewhere since it opened (the related row's menu, a notice's Undo); Restart on the related row after a description link goes back to it. The file viewer tells the help sheet its keys wait under a layer; refusalReason replaces the last two open-coded reads. Auto-advance wears list glyphs of its own (the skip arrows are Previous / Next beside it), and the auto-advance and loop cycles are `lit`, not toggles: no aria-pressed, the name says the mode. Both places render from one list of controls, so the docked tooltips name the mode too. The English cap message and admin hint quote "Show more"; the hint says refunds are limited. --- docs/DESIGN.md | 12 +- .../src/components/ui/Button.dom.test.tsx | 12 ++ frontend/src/components/ui/Button.tsx | 7 +- .../src/components/ui/IconButton.dom.test.tsx | 19 ++ frontend/src/components/ui/IconButton.tsx | 7 +- frontend/src/components/ui/buttonStyles.ts | 12 ++ .../features/player/PlayerModal.dom.test.tsx | 76 ++++++- frontend/src/features/player/PlayerModal.tsx | 39 ++-- .../player/RelatedVideos.dom.test.tsx | 15 +- .../src/features/player/RelatedVideos.tsx | 11 +- .../player/playerModalControls.dom.test.tsx | 74 +++++++ .../features/player/playerModalControls.tsx | 204 +++++++++--------- .../features/plexCards/PlexCollectionAdd.tsx | 5 +- frontend/src/features/video/AddToPlaylist.tsx | 7 +- .../video/videoWrites.contract.test.ts | 28 ++- .../features/video/videoWrites.dom.test.ts | 52 ++++- frontend/src/features/video/videoWrites.ts | 51 ++++- frontend/src/i18n/locales/en/config.json | 2 +- frontend/src/i18n/locales/en/errors.json | 2 +- frontend/src/i18n/locales/hu/config.json | 2 +- .../src/lib/accountSwitchNotice.dom.test.ts | 5 +- frontend/src/lib/entryUrl.ts | 6 +- frontend/src/lib/keymap.ts | 3 +- frontend/src/lib/playerModalHooks.ts | 11 +- frontend/src/lib/progressCache.test.ts | 48 ++++- frontend/src/lib/progressCache.ts | 60 +++++- frontend/src/modules/feed/Feed.tsx | 16 +- .../modules/feed/Feed.ytLoadMore.dom.test.tsx | 52 ++++- .../modules/files/FilePreview.dom.test.tsx | 22 ++ frontend/src/modules/files/FilePreview.tsx | 8 +- frontend/src/modules/playlists/Playlists.tsx | 20 +- 31 files changed, 699 insertions(+), 189 deletions(-) create mode 100644 frontend/src/features/player/playerModalControls.dom.test.tsx diff --git a/docs/DESIGN.md b/docs/DESIGN.md index e430a58c..e531c9e7 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -225,7 +225,9 @@ Also in force: tag chip is `Chip`; a sortable row's grip is `DragHandle`. On the always-dark player chrome both `Button` and `IconButton` use the `overlay` variant (white, a white layer), whatever the theme; the Plex control row is IconButton's `player` rung (36, a 20px glyph, round, 44 on touch). A pressed toggle is - `pressed`: the solid accent on a `Button`, the accent glyph on an `IconButton`. A raw `, + ); + const btn = screen.getByRole("button", { name: "Loop: All" }); + expect(btn.hasAttribute("aria-pressed")).toBe(false); + expect(btn.hasAttribute("data-lit")).toBe(true); + expect(btn.className).toContain("data-lit:bg-accent"); + }); + it("a toggle announces aria-pressed and paints the pressed state", () => { const { rerender } = render( + ) : ( + + {c.glyph} + ); return ( <> - - {count} - + {render(step[0]!)} + + {index + 1} / {queueLength} + + {render(step[1]!)} - {/* Persistent playback settings — cycle on click, saved to your account. */} - - + {modes.map(render)} ); } diff --git a/frontend/src/features/plexCards/PlexCollectionAdd.tsx b/frontend/src/features/plexCards/PlexCollectionAdd.tsx index 992a527f..f63b90a7 100644 --- a/frontend/src/features/plexCards/PlexCollectionAdd.tsx +++ b/frontend/src/features/plexCards/PlexCollectionAdd.tsx @@ -2,7 +2,7 @@ import { useLayoutEffect, useState } from "react"; import { useTranslation } from "react-i18next"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { qk } from "../../lib/queryKeys"; -import { api, HttpError, type PlexCollection } from "../../lib/api"; +import { api, refusalReason, type PlexCollection } from "../../lib/api"; import { notifyActionError } from "../../lib/actionErrors"; import { PopoverSurface } from "../../components/ui/AnchoredPopover"; import { useAnchoredPopover } from "../../components/ui/anchoredPopoverHooks"; @@ -123,8 +123,7 @@ export default function PlexCollectionAddAt({ /** A create refused only because the read after it failed: the collection is on Plex, with this * title in it, and its row is kept — the name is taken now, so the field clears as on success. */ function madeOnPlex(e: unknown): boolean { - const d = e instanceof HttpError ? (e.detailData as { reason?: string } | undefined) : undefined; - return d?.reason === "collection_pending_sync"; + return refusalReason(e) === "collection_pending_sync"; } function flipContains( diff --git a/frontend/src/features/video/AddToPlaylist.tsx b/frontend/src/features/video/AddToPlaylist.tsx index 7f88b1e0..3e00298a 100644 --- a/frontend/src/features/video/AddToPlaylist.tsx +++ b/frontend/src/features/video/AddToPlaylist.tsx @@ -5,6 +5,7 @@ import { qk } from "../../lib/queryKeys"; import { ListPlus } from "lucide-react"; import { YouTubeIcon } from "../../lib/brandIcons"; import { api, type Playlist } from "../../lib/api"; +import { addPlaylistItem, removePlaylistItem } from "./videoWrites"; import { notifyActionError } from "../../lib/actionErrors"; import { playlistName } from "../../lib/playlistName"; import { PopoverSurface } from "../../components/ui/AnchoredPopover"; @@ -115,8 +116,8 @@ function MembershipPanel({ if (busy) return; setBusy(true); try { - if (pl.has_video) await api.removeFromPlaylist(pl.id, videoId); - else await api.addToPlaylist(pl.id, videoId); + if (pl.has_video) await removePlaylistItem(qc, pl, videoId); + else await addPlaylistItem(qc, pl, videoId); qc.setQueryData(qk.playlistMembership(videoId), (old) => old?.map((p) => (p.id === pl.id ? { ...p, has_video: !p.has_video } : p)), ); @@ -133,7 +134,7 @@ function MembershipPanel({ let made = false; try { const pl = await api.createPlaylist(name); - await api.addToPlaylist(pl.id, videoId); + await addPlaylistItem(qc, pl, videoId); made = true; } catch (e) { notifyActionError(e, t("playlists.addFailed")); diff --git a/frontend/src/features/video/videoWrites.contract.test.ts b/frontend/src/features/video/videoWrites.contract.test.ts index dea3d9c2..0a29000e 100644 --- a/frontend/src/features/video/videoWrites.contract.test.ts +++ b/frontend/src/features/video/videoWrites.contract.test.ts @@ -16,19 +16,29 @@ const ALLOWED = new Set([ "features/video/videoWrites.ts", ]); -// A related list and a live search are never refetched, so a state or Watch-later write that does -// not reach them (`markVideo`) leaves them showing the old mark for the session. videoWrites makes -// those writes and the mark together; a direct api call skips the mark. -describe("a video's state and Watch-later writes", () => { +// A related list and a live search are never refetched, so a state, Watch-later or playlist-item +// write that does not reach them (`markVideo`) leaves them showing the old mark for the session — +// the built-in Watch later list's items ARE the mark. videoWrites makes those writes and the mark +// together; a direct api call skips the mark. +const WRITES = + /\bapi\s*\.\s*(setState|clearState|watchLaterAdd|watchLaterRemove|addToPlaylist|removeFromPlaylist|moveToPlaylist)\b/; + +describe("a video's state, Watch-later and playlist-item writes", () => { it("go through features/video/videoWrites, never the api directly", () => { const offenders = sources() .filter(({ f }) => !ALLOWED.has(f)) - // Only a call on the api object counts (React's own `setState` is not ours); the dot may sit - // on the next line of a chain. - .filter(({ src }) => - /\bapi\s*\.\s*(setState|clearState|watchLaterAdd|watchLaterRemove)\(/.test(src), - ) + // Only the api object's members count (React's own `setState` is not ours) — named, not only + // called: `(saved ? api.watchLaterAdd : api.watchLaterRemove)(id)` makes the write too. The + // dot may sit on the next line of a chain. + .filter(({ src }) => WRITES.test(src)) .map(({ f }) => f); expect(offenders).toEqual([]); }); + + it("catches a write named without being called on the spot, and a playlist item's", () => { + expect(WRITES.test("(saved ? api.watchLaterAdd : api.watchLaterRemove)(id)")).toBe(true); + expect(WRITES.test("await api\n .removeFromPlaylist(listId, id)")).toBe(true); + expect(WRITES.test("api.moveToPlaylist(a, id, b)")).toBe(true); + expect(WRITES.test("this.setState(next); api.playlists()")).toBe(false); + }); }); diff --git a/frontend/src/features/video/videoWrites.dom.test.ts b/frontend/src/features/video/videoWrites.dom.test.ts index da983463..5aa52125 100644 --- a/frontend/src/features/video/videoWrites.dom.test.ts +++ b/frontend/src/features/video/videoWrites.dom.test.ts @@ -7,11 +7,31 @@ import { QueryClient } from "@tanstack/react-query"; import { api } from "../../lib/api"; import { qk } from "../../lib/queryKeys"; -import { resetVideoState, writeVideoState, writeWatchLater } from "./videoWrites"; +import { + addPlaylistItem, + movePlaylistItem, + removePlaylistItem, + resetVideoState, + writeVideoState, + writeWatchLater, +} from "./videoWrites"; afterEach(() => vi.restoreAllMocks()); -const marks = (qc: QueryClient) => qc.getQueryData(qk.videoMarks()); +// The marks' values, without when each was confirmed. +const marks = (qc: QueryClient) => { + const m = qc.getQueryData>>(qk.videoMarks()); + return m + ? Object.fromEntries( + Object.entries(m).map(([id, f]) => [ + id, + Object.fromEntries(Object.entries(f).map(([k, x]) => [k, x.value])), + ]), + ) + : undefined; +}; +const WATCH_LATER = { id: 1, kind: "watch_later" as const }; +const MINE = { id: 2, kind: "user" as const }; describe("videoWrites", () => { it("marks a state written", async () => { @@ -38,6 +58,34 @@ describe("videoWrites", () => { expect(marks(qc)).toEqual({ X: { saved: false } }); }); + it("marks an item of the Watch later list as the bookmark it is", async () => { + vi.spyOn(api, "addToPlaylist").mockResolvedValue({ ok: true } as never); + vi.spyOn(api, "removeFromPlaylist").mockResolvedValue({ ok: true } as never); + const qc = new QueryClient(); + await addPlaylistItem(qc, WATCH_LATER, "X"); + expect(marks(qc)).toEqual({ X: { saved: true } }); + await removePlaylistItem(qc, WATCH_LATER, "X"); + expect(marks(qc)).toEqual({ X: { saved: false } }); + }); + + it("marks nothing for an item of any other list", async () => { + vi.spyOn(api, "addToPlaylist").mockResolvedValue({ ok: true } as never); + vi.spyOn(api, "removeFromPlaylist").mockResolvedValue({ ok: true } as never); + const qc = new QueryClient(); + await addPlaylistItem(qc, MINE, "X"); + await removePlaylistItem(qc, { id: 3, kind: undefined }, "X"); + expect(marks(qc)).toBeUndefined(); + }); + + it("marks a move into and out of the Watch later list", async () => { + vi.spyOn(api, "moveToPlaylist").mockResolvedValue({ ok: true } as never); + const qc = new QueryClient(); + await movePlaylistItem(qc, MINE, "X", WATCH_LATER); + expect(marks(qc)).toEqual({ X: { saved: true } }); + await movePlaylistItem(qc, WATCH_LATER, "X", MINE); + expect(marks(qc)).toEqual({ X: { saved: false } }); + }); + it("marks nothing the server refused", async () => { vi.spyOn(api, "setState").mockRejectedValue(new Error("offline")); const qc = new QueryClient(); diff --git a/frontend/src/features/video/videoWrites.ts b/frontend/src/features/video/videoWrites.ts index dd8b679b..38fa22f0 100644 --- a/frontend/src/features/video/videoWrites.ts +++ b/frontend/src/features/video/videoWrites.ts @@ -1,15 +1,17 @@ import type { QueryClient } from "@tanstack/react-query"; -import { api, type VideoStatus } from "../../lib/api"; +import { api, type Playlist, type VideoStatus } from "../../lib/api"; import { markVideo } from "../../lib/progressCache"; -// A video's user state and its Watch-later mark, written to the server and — once it has the -// change — into the lists that are never refetched (`markVideo`: every related list, every live -// search). These are the ONE way to make those writes, and `videoWrites.contract.test.ts` holds it: -// four writes that called the api directly (the Notification Center's undo, the feed's reset, the -// feed's and Home's Watch-later toggles) left a related list hiding a video the user had un-hidden, -// or showing it unsaved after a save. Each returns the server's answer, so a caller keeps its own -// optimistic layer, notices and rollback. +// A video's user state, its Watch-later mark and its playlist items, written to the server and — +// once it has the change — into the lists that are never refetched (`markVideo`: every related +// list, every live search). These are the ONE way to make those writes, and +// `videoWrites.contract.test.ts` holds it: four writes that called the api directly (the +// Notification Center's undo, the feed's reset, the feed's and Home's Watch-later toggles) left a +// related list hiding a video the user had un-hidden, or showing it unsaved after a save. A +// playlist item is one of them because the built-in Watch later list IS the mark (`saved`): ticked +// in Add to playlist or taken out on the Playlists page, it left the bookmark as it was. Each +// returns the server's answer, so a caller keeps its own optimistic layer, notices and rollback. export function writeVideoState(qc: QueryClient, id: string, status: VideoStatus) { return api.setState(id, status).then((r) => { @@ -32,3 +34,36 @@ export function writeWatchLater(qc: QueryClient, id: string, saved: boolean) { return r; }); } + +/** The list a playlist item write goes to: on the built-in Watch later list (`kind`) the item IS + * the video's Watch-later mark. A kind the caller cannot tell (undefined) marks nothing. */ +type ListRef = { id: number; kind: Playlist["kind"] | undefined }; + +const markIfWatchLater = (qc: QueryClient, list: ListRef, id: string, saved: boolean) => { + if (list.kind === "watch_later") markVideo(qc, id, { saved }); +}; + +/** Add a video to a playlist; `position` (an index in the stored order) puts it back where a + * removal took it from. */ +export function addPlaylistItem(qc: QueryClient, list: ListRef, id: string, position?: number) { + return api.addToPlaylist(list.id, id, position).then((r) => { + markIfWatchLater(qc, list, id, true); + return r; + }); +} + +export function removePlaylistItem(qc: QueryClient, list: ListRef, id: string) { + return api.removeFromPlaylist(list.id, id).then((r) => { + markIfWatchLater(qc, list, id, false); + return r; + }); +} + +/** Move a video from one playlist to another, in one request (see `api.moveToPlaylist`). */ +export function movePlaylistItem(qc: QueryClient, from: ListRef, id: string, to: ListRef) { + return api.moveToPlaylist(from.id, id, to.id).then((r) => { + markIfWatchLater(qc, from, id, false); + markIfWatchLater(qc, to, id, true); + return r; + }); +} diff --git a/frontend/src/i18n/locales/en/config.json b/frontend/src/i18n/locales/en/config.json index 71fc61e2..40b0e9e9 100644 --- a/frontend/src/i18n/locales/en/config.json +++ b/frontend/src/i18n/locales/en/config.json @@ -56,7 +56,7 @@ }, "related_daily_limit_per_user": { "label": "Related videos — daily limit per user", - "hint": "Max related-video requests a user can make per day — opening a list and each Show more count one. They come from YouTube's website and spend no search quota; looking up the videos in one costs up to about 8 units (up to 4 pages, each a video and a channel lookup). A failed one doesn't count. Set 0 to turn related videos off." + "hint": "Max related-video requests a user can make per day — opening a list counts one, and so does every “Show more”. They come from YouTube's website and spend no search quota; looking up the videos in one costs up to about 8 units (up to 4 pages, each a video and a channel lookup). A failed one doesn't count (up to 5 an hour). Set 0 to turn related videos off." }, "search_daily_limit_per_user": { "label": "Live search — daily limit per user", diff --git a/frontend/src/i18n/locales/en/errors.json b/frontend/src/i18n/locales/en/errors.json index 10d574d8..1b6c9341 100644 --- a/frontend/src/i18n/locales/en/errors.json +++ b/frontend/src/i18n/locales/en/errors.json @@ -189,7 +189,7 @@ "recipient_required": "A recipient email is required", "registration_closed": "Registration is currently closed", "related_disabled": "Related videos are turned off on this instance.", - "related_daily_limit": "You've reached today's related videos limit ({{limit}}). Opening a list and each Show more count as one request. Try again tomorrow.", + "related_daily_limit": "You've reached today's related videos limit ({{limit}}). Opening a list counts as one request, and so does every “Show more”. Try again tomorrow.", "search_disabled": "Live YouTube search is turned off on this instance.", "search_daily_limit": "You've reached today's YouTube search limit ({{limit}}). Try again tomorrow.", "search_term_required": "Enter something to search for", diff --git a/frontend/src/i18n/locales/hu/config.json b/frontend/src/i18n/locales/hu/config.json index b1bc3eb1..3fdb90e4 100644 --- a/frontend/src/i18n/locales/hu/config.json +++ b/frontend/src/i18n/locales/hu/config.json @@ -56,7 +56,7 @@ }, "related_daily_limit_per_user": { "label": "Kapcsolódó videók — napi limit / felhasználó", - "hint": "Hányszor kérhet kapcsolódó videókat egy felhasználó naponta — egy lista megnyitása és minden „Mutass többet” egynek számít. A YouTube weboldaláról jönnek, keresési kvótát nem fogyasztanak; a bennük lévő videók lekérése kérésenként legfeljebb kb. 8 egység (legfeljebb 4 oldal, mindegyik egy videó- és egy csatornalekérés). A sikertelen nem számít. 0 = a kapcsolódó videók kikapcsolva." + "hint": "Hányszor kérhet kapcsolódó videókat egy felhasználó naponta — egy lista megnyitása és minden „Mutass többet” egynek számít. A YouTube weboldaláról jönnek, keresési kvótát nem fogyasztanak; a bennük lévő videók lekérése kérésenként legfeljebb kb. 8 egység (legfeljebb 4 oldal, mindegyik egy videó- és egy csatornalekérés). A sikertelen nem számít (óránként legfeljebb 5). 0 = a kapcsolódó videók kikapcsolva." }, "search_daily_limit_per_user": { "label": "Élő keresés — napi limit / felhasználó", diff --git a/frontend/src/lib/accountSwitchNotice.dom.test.ts b/frontend/src/lib/accountSwitchNotice.dom.test.ts index f38d91ef..974f84e5 100644 --- a/frontend/src/lib/accountSwitchNotice.dom.test.ts +++ b/frontend/src/lib/accountSwitchNotice.dom.test.ts @@ -12,6 +12,7 @@ import { clearEntryUrl, reloadAfterRefusal, } from "./entryUrl"; +import { HttpError } from "./api/core"; const notice = (search: string, hash = "") => accountSwitchNotice(new URLSearchParams(search), new URLSearchParams(hash)); @@ -112,14 +113,14 @@ describe("a tab whose session the server refused", () => { it("reloads as the account signed in now, saying why, when it showed another", () => { const loc = stubLocation(); - reloadAfterRefusal({ detailData: { code: "refused", reason: "account_changed" } }); + reloadAfterRefusal(new HttpError(401, "x", { code: "refused", reason: "account_changed" })); expect(loc.replace).toHaveBeenCalledWith(`/?page=stats&${ACCOUNT_CHANGED_FLAG}`); expect(loc.reload).not.toHaveBeenCalled(); }); it("reloads plainly when the session ended", () => { const loc = stubLocation(); - reloadAfterRefusal({ detailData: { code: "refused", reason: "not_authenticated" } }); + reloadAfterRefusal(new HttpError(401, "x", { code: "refused", reason: "not_authenticated" })); expect(loc.reload).toHaveBeenCalled(); expect(loc.replace).not.toHaveBeenCalled(); }); diff --git a/frontend/src/lib/entryUrl.ts b/frontend/src/lib/entryUrl.ts index 48ae0fb7..0a148ffa 100644 --- a/frontend/src/lib/entryUrl.ts +++ b/frontend/src/lib/entryUrl.ts @@ -15,6 +15,7 @@ // with nothing personal in them (login, deleted, account) ride the query string. import type { NotifLevel } from "./notifications"; +import { refusalReason } from "./api/core"; import { hrefHere } from "./urlState"; export const entrySearch = new URLSearchParams(window.location.search); @@ -45,9 +46,8 @@ export function accountChangedUrl(): string { * signed in now (`account_changed`: a sign-in as someone else in another tab), it boots as that one * and says why — a Sign out that silently left someone else signed in would read as broken. Any * other refusal means the session ended: a plain reload lands on the sign-in page. */ -export function reloadAfterRefusal(err: { detailData?: unknown }): void { - if ((err.detailData as { reason?: string } | undefined)?.reason === "account_changed") - window.location.replace(accountChangedUrl()); +export function reloadAfterRefusal(err: unknown): void { + if (refusalReason(err) === "account_changed") window.location.replace(accountChangedUrl()); else window.location.reload(); } diff --git a/frontend/src/lib/keymap.ts b/frontend/src/lib/keymap.ts index 9cc1b3ed..66d49083 100644 --- a/frontend/src/lib/keymap.ts +++ b/frontend/src/lib/keymap.ts @@ -489,7 +489,8 @@ export const setListInTopLayerProbe = (probe: () => boolean): void => { }; // A surface whose keys wait while a layer sits above it (the YouTube player under its related -// drawer or a dialog: every key but Escape is the layer's then). Its rows would promise keys that do +// drawer or a dialog: every key but Escape is the layer's then; the file viewer under its menu or +// Share dialog). Its rows would promise keys that do // nothing, so the sheet leaves the scope out — asked at the same moment as `helpOverLayer`. const coveredProbes = new Map boolean>(); export const setScopeCoveredProbe = (scope: KeyScope, probe: (() => boolean) | null): void => { diff --git a/frontend/src/lib/playerModalHooks.ts b/frontend/src/lib/playerModalHooks.ts index 8a0a3ae7..dfe5de55 100644 --- a/frontend/src/lib/playerModalHooks.ts +++ b/frontend/src/lib/playerModalHooks.ts @@ -563,10 +563,11 @@ export function useYtProgress({ // `reclaimFocus`, `qc`, … — are a PER-ITEM SNAPSHOT, taken once through `itemSnapshot` when this // player is built: they describe the video it was built for, and re-reading them mid-session // would be the bug, not the fix. `setWatched` is the clearest case: bound to the item, it lets - // this cleanup's late auto-mark land on the item being LEFT, which the modal then ignores. What - // genuinely has to stay live mid-session is behind refs (currentIdRef, modeRef, navRef, - // sessionPosRef, autoMarkedRef, errorStreakRef, playerRef) — and THAT split is the enforcement: - // a value that is not a snapshot does not belong in this closure. (This replaced an - // exhaustive-deps suppression, which made the React Compiler skip the whole hook.) + // this cleanup's late auto-mark land on the item being LEFT, which the modal records for that + // item and never paints on the next one. What genuinely has to stay live mid-session is behind + // refs (currentIdRef, modeRef, navRef, sessionPosRef, autoMarkedRef, errorStreakRef, playerRef) + // — and THAT split is the enforcement: a value that is not a snapshot does not belong in this + // closure. (This replaced an exhaustive-deps suppression, which made the React Compiler skip + // the whole hook.) }, [activeId, mountRef, playerRef, currentIdRef, navRef, modeRef, sessionPosRef]); } diff --git a/frontend/src/lib/progressCache.test.ts b/frontend/src/lib/progressCache.test.ts index e14e9a18..2c4f4fd9 100644 --- a/frontend/src/lib/progressCache.test.ts +++ b/frontend/src/lib/progressCache.test.ts @@ -1,10 +1,11 @@ // What a confirmed mark reaches. A related list and a live YouTube search are never refetched (a // scrape and a slot of the day's cap, or 100 quota units), so the mark is written into them; and it is -// kept apart from them too, where a page landing later cannot take it back. -import { describe, expect, it } from "vitest"; +// kept apart from them too, laid over each page read before it (a page landing later, from a fetch +// that started earlier, cannot take it back) and over none read after (those carry the server's word). +import { afterEach, describe, expect, it, vi } from "vitest"; import { QueryClient } from "@tanstack/react-query"; -import { markVideo, patchCachedProgress } from "./progressCache"; +import { markedRows, markVideo, patchCachedProgress } from "./progressCache"; import { qk } from "./queryKeys"; import type { Video } from "./api"; @@ -30,9 +31,48 @@ describe("markVideo", () => { markVideo(qc, "X", { saved: true }); patchCachedProgress(qc, new Map([["X", 90]])); expect(qc.getQueryData(qk.videoMarks())).toEqual({ - X: { status: "hidden", saved: true, position_seconds: 90 }, + X: { + status: { value: "hidden", at: expect.any(Number) }, + saved: { value: true, at: expect.any(Number) }, + position_seconds: { value: 90, at: expect.any(Number) }, + }, }); // Nothing observes it before a related list opens: it must not be collected meanwhile. expect(qc.getQueryCache().find({ queryKey: qk.videoMarks() })?.options.gcTime).toBe(Infinity); }); }); + +describe("markedRows", () => { + afterEach(() => vi.useRealTimers()); + const marked = (qc: QueryClient) => + qc.getQueryData(qk.videoMarks()) as Parameters[1]; + + it("lays a mark over a page whose fetch started before it", () => { + vi.useFakeTimers({ now: 2000 }); + const qc = new QueryClient(); + markVideo(qc, "X", { saved: true }); + // A Show more that started at 1000 and landed after the mark: built from the pages it started from. + const rows = markedRows([{ items: [v("X")], fetchedAt: 1000 }], marked(qc)); + expect(rows[0]!.saved).toBe(true); + }); + + it("leaves a page read after the mark with the server's word", () => { + vi.useFakeTimers({ now: 2000 }); + const qc = new QueryClient(); + markVideo(qc, "X", { saved: true, position_seconds: 40 }); + // Taken off Watch later on the Playlists page, played on another device: the server says so. + const fresh = { ...v("X"), saved: false, position_seconds: 300 }; + const rows = markedRows([{ items: [fresh], fetchedAt: 3000 }], marked(qc)); + expect(rows[0]).toEqual(fresh); + }); + + it("weighs each field by when it was marked", () => { + const qc = new QueryClient(); + vi.useFakeTimers({ now: 1000 }); + markVideo(qc, "X", { saved: true }); + vi.setSystemTime(3000); + markVideo(qc, "X", { status: "hidden" }); + const rows = markedRows([{ items: [v("X")], fetchedAt: 2000 }], marked(qc)); + expect(rows[0]).toMatchObject({ saved: false, status: "hidden" }); + }); +}); diff --git a/frontend/src/lib/progressCache.ts b/frontend/src/lib/progressCache.ts index 08116ba4..f8dc49ad 100644 --- a/frontend/src/lib/progressCache.ts +++ b/frontend/src/lib/progressCache.ts @@ -63,18 +63,26 @@ export function markVideo(qc: QueryClient, id: string, change: Partial