From 0bc10242874014b84a3e9d378cfb05f0649fccc5 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 25 Oct 2026 13:51:26 +0100 Subject: [PATCH] fix(player): marks are weighed when answered, lit and pressed hold under the pointer, and Restart is one glyph The watched toggle weighed its own click at click time against marks stamped when the server answered, so the late answer of its previous click overruled the newer one; it now holds its word while any of its writes is on the way, is stamped at the answer, and drops a refused write. The hosts return the write. In a live search the feed's own answered writes stand down over the rows, which carry the marks, so a newer mark from the related list reaches a card the feed touched earlier. A lit mode cycle got the hover layer, and a variant's hover colour outranked the pressed and lit colours: the state layer stands down on [data-lit] and those colours are important. The menus' Play from the beginning wears RotateCcw like the card's Restart, Reset wears Eraser and Redownload the retry arrow (DESIGN decision log). Tests pin the toggle's orders, the Watch later mark at its call sites, the live-search overrides, the hover exemptions and the menu glyphs. --- docs/DESIGN.md | 14 +- .../src/PlaylistEditFailures.dom.test.tsx | 47 +++ frontend/src/components/ui/buttonStyles.ts | 17 +- .../features/player/PlayerModal.dom.test.tsx | 373 +++++++++++++++--- frontend/src/features/player/PlayerModal.tsx | 42 +- .../features/plexCards/usePlexCardActions.tsx | 4 +- .../src/features/video/useVideoActions.tsx | 6 +- frontend/src/index.css | 20 +- frontend/src/lib/menuGlyphs.contract.test.ts | 34 ++ frontend/src/lib/theme.contract.test.ts | 25 +- .../channels/WatchedChannels.dom.test.tsx | 6 +- .../modules/channels/WatchedVideosDialog.tsx | 4 +- frontend/src/modules/feed/Feed.tsx | 26 +- .../modules/feed/Feed.ytLoadMore.dom.test.tsx | 79 +++- frontend/src/modules/files/useFileActions.tsx | 6 +- frontend/src/modules/home/Home.tsx | 7 +- .../modules/playlists/Playlists.dom.test.tsx | 13 + frontend/src/modules/playlists/Playlists.tsx | 9 +- 18 files changed, 629 insertions(+), 103 deletions(-) create mode 100644 frontend/src/lib/menuGlyphs.contract.test.ts diff --git a/docs/DESIGN.md b/docs/DESIGN.md index e531c9e7..e81a0579 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -225,9 +225,10 @@ 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 mode cycle (the player's - auto-advance and loop) is no two-state switch: `lit` gives it the same look when it is not off, without - `aria-pressed`, and its name says the mode. A raw ` + + ))} - + ), })); import Feed from "./Feed"; import { api, DEFAULT_TAG_MODES, HttpError, type FeedFilters, type Video } from "../../lib/api"; import { qk } from "../../lib/queryKeys"; -import { writeVideoState } from "../../features/video/videoWrites"; +import { writeVideoState, writeWatchLater } from "../../features/video/videoWrites"; beforeAll(() => { class Stub { @@ -159,6 +176,50 @@ describe("Feed — a live search's Load more on its way", () => { await act(async () => void (await writeVideoState(qc, "a", "hidden"))); await act(async () => land({ items: [hit("b")], next_cursor: null, source: "scrape" })); await screen.findByText(/hit b/); - expect(screen.getByRole("list", { name: "results" }).textContent).not.toContain("hit a (new)"); + expect(screen.getByRole("list", { name: "results" }).textContent).toBe( + "hit a (hidden)hit b (new)", + ); + }); +}); + +describe("Feed — a live search's own writes and a newer mark", () => { + const results = () => screen.getByRole("list", { name: "results" }).textContent; + const searched = () => { + vi.spyOn(api, "searchYoutube").mockResolvedValue({ + items: [hit("a")], + next_cursor: null, + source: "scrape", + } as never); + return mount(); + }; + const tick = () => act(async () => void (await new Promise((r) => setTimeout(r, 5)))); + + // Hidden from its card, then put back from the player's related row (which writes past the feed). + it("shows a status marked elsewhere after the feed's own answered write", async () => { + vi.spyOn(api, "setState").mockResolvedValue({ ok: true } as never); + const qc = searched(); + fireEvent.click(await screen.findByRole("button", { name: "hide a" })); + await waitFor(() => expect(results()).toBe("hit a (hidden)")); + await tick(); + await act(async () => void (await writeVideoState(qc, "a", "new"))); + await waitFor(() => expect(results()).toBe("hit a (new)")); + }); + + it("shows a bookmark taken off elsewhere after the feed's own answered save", async () => { + vi.spyOn(api, "watchLaterAdd").mockResolvedValue({ ok: true } as never); + vi.spyOn(api, "watchLaterRemove").mockResolvedValue({ ok: true } as never); + const qc = searched(); + fireEvent.click(await screen.findByRole("button", { name: "save a" })); + await waitFor(() => expect(results()).toBe("hit a (new, saved)")); + await tick(); + await act(async () => void (await writeWatchLater(qc, "a", false))); + await waitFor(() => expect(results()).toBe("hit a (new)")); + }); + + it("keeps its own write over the rows until the server has answered it", async () => { + vi.spyOn(api, "setState").mockReturnValue(new Promise(() => {}) as never); + searched(); + fireEvent.click(await screen.findByRole("button", { name: "hide a" })); + await waitFor(() => expect(results()).toBe("hit a (hidden)")); }); }); diff --git a/frontend/src/modules/files/useFileActions.tsx b/frontend/src/modules/files/useFileActions.tsx index dddb3446..ca16009d 100644 --- a/frontend/src/modules/files/useFileActions.tsx +++ b/frontend/src/modules/files/useFileActions.tsx @@ -11,10 +11,10 @@ import { Pin, PinOff, PlayCircle, + RefreshCw, RotateCcw, Scissors, Share2, - SkipBack, Trash2, } from "lucide-react"; import { api, type DownloadJobKind, type FileFolder, type LibraryFile } from "../../lib/api"; @@ -126,7 +126,7 @@ export function useFileActions({ { id: "restart", label: t("card.restartTitle"), - icon: icon(SkipBack), + icon: icon(RotateCcw), onSelect: () => onPlay(file, true), }, ] @@ -269,7 +269,7 @@ export function useFileActions({ { id: "redownload", label: t("files.actions.redownload"), - icon: icon(RotateCcw), + icon: icon(RefreshCw), onSelect: () => void confirmThen( t("downloads.confirm.redownloadTitle", { name: jobTitle(file) }), diff --git a/frontend/src/modules/home/Home.tsx b/frontend/src/modules/home/Home.tsx index 176d2c91..242262b2 100644 --- a/frontend/src/modules/home/Home.tsx +++ b/frontend/src/modules/home/Home.tsx @@ -10,7 +10,6 @@ import { PinOff, Play, RotateCcw, - SkipBack, Star, } from "lucide-react"; import { qk } from "../../lib/queryKeys"; @@ -195,7 +194,7 @@ function useHomeCardMenu({ { id: "restart", label: t("card.restartTitle"), - icon: , + icon: , onSelect: () => onPlay(target, 0), }, ] @@ -837,7 +836,8 @@ export default function Home() { // A full refetch, unlike the feed's stale-mark: these rails are two dozen rows, and marking // something watched genuinely changes what the page should answer (it can retire the hero). // The feed avoids this because it would refetch ten infinite-query pages per click. - void writeVideoState(qc, id, status) + const write = writeVideoState(qc, id, status); + void write .then(() => { return qc.invalidateQueries({ queryKey: qk.discover() }); }) @@ -849,6 +849,7 @@ export default function Home() { // would describe a state the user can see is untrue, over a fully populated page whose // invalidations never even ran (they sit inside `.then`). .catch((e: unknown) => notifyActionError(e, t("home.actionFailed"))); + return write; }, [qc, t], ); diff --git a/frontend/src/modules/playlists/Playlists.dom.test.tsx b/frontend/src/modules/playlists/Playlists.dom.test.tsx index 5a44be53..8a3e2730 100644 --- a/frontend/src/modules/playlists/Playlists.dom.test.tsx +++ b/frontend/src/modules/playlists/Playlists.dom.test.tsx @@ -419,6 +419,19 @@ describe("Playlists — a change the server refused is taken back on screen", () expect(rowsOnScreen()).toEqual(["Video V", "Video W"]); }); + // Watch later's items ARE the bookmark: a video moved into it is saved wherever it is shown. + it("marks a video moved into Watch later as saved", async () => { + vi.spyOn(api, "moveToPlaylist").mockResolvedValue(undefined as never); + mount({ 1: [vid("V", 0)], [WATCH_LATER]: [] }, { lastMovePlaylistId: WATCH_LATER }); + await rowMenu("Video V", "playlists.moveToRecent"); + await waitFor(() => + expect( + qc.getQueryData>(qk.videoMarks())?.V?.saved + ?.value, + ).toBe(true), + ); + }); + it("puts back the order when saving a reorder failed", async () => { vi.spyOn(api, "reorderPlaylist").mockRejectedValue(new HttpError(500, "boom")); mount({ 1: [vid("V", 0), vid("W", 0)] }); diff --git a/frontend/src/modules/playlists/Playlists.tsx b/frontend/src/modules/playlists/Playlists.tsx index 2befa4d1..97bb3a58 100644 --- a/frontend/src/modules/playlists/Playlists.tsx +++ b/frontend/src/modules/playlists/Playlists.tsx @@ -307,7 +307,10 @@ function PlaylistDetail() { }); const detail = detailQuery.data; // Which kind of list an item write goes to (the Watch later list's items are the bookmark): - // the rail's list knows every one, the open detail the one on screen. + // the rail's list knows every one, the open detail the one on screen. Only a user list's rows are + // removed or moved from here today (Watch later offers neither), so it reads "user" — looked up + // all the same, so no write here takes the kind for granted. A move INTO Watch later passes the + // rail's own row as its target. const listRef = (id: number) => ({ id, kind: playlists.find((p) => p.id === id)?.kind ?? (detail?.id === id ? detail.kind : undefined), @@ -623,7 +626,8 @@ function PlaylistDetail() { } function playerState(id: string, status: VideoStatus) { - writeVideoState(qc, id, status) + const write = writeVideoState(qc, id, status); + write .then(() => { // Every cached list, not `qk.playlist(selectedId)`: the player may have been launched from a // list that is no longer the selected one (see `playing`), and that list holds the row whose @@ -633,6 +637,7 @@ function PlaylistDetail() { }) // No catch used to mean an unhandled rejection and nothing said. .catch((e: unknown) => notifyActionError(e, t("errors.writeFailed.videoState"))); + return write; } // Stable identity: PlayerModal's keydown effect depends on it, so a fresh closure