peter/siftlode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(player): marks are weighed when answered, lit and pressed hold under the pointer, and Restart is one glyph

Browse Source 
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.
This commit is contained in:
peter
2026-10-25 13:51:26 +01:00
parent 13f65b4844
commit 0bc1024287
18 changed files with 629 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/modules/channels/WatchedChannels.dom.test.tsx
+3 -3
View File
@@ -37,14 +37,14 @@ vi.mock("../../features/navigation/navigationContext", () => ({
vi.mock("../../lib/settingsTab", () => ({ focusSettingsTab: h.focusSettingsTab }));
// The open player's mark, for a test to make as the player would.
const playerMark = vi.hoisted(() => ({
onState: null as null | ((id: string, s: string) => void),
onState: null as null | ((id: string, s: string) => Promise<unknown>),
}));
vi.mock("../../features/player/PlayerModal", () => ({
default: (p: {
video: { id: string };
queue: { id: string }[];
startIndex: number;
onState: (id: string, s: string) => void;
onState: (id: string, s: string) => Promise<unknown>;
}) => {
playerMark.onState = p.onState;
h.player(
@@ -187,7 +187,7 @@ describe("WatchedChannels", () => {
const dialog = await screen.findByRole("dialog");
fireEvent.click(await within(dialog).findByRole("button", { name: "card.play — Video b" }));
await waitFor(() => expect(playerMark.onState).not.toBeNull());
act(() => playerMark.onState!("b", "watched"));
act(() => void playerMark.onState!("b", "watched"));
await waitFor(() => {
const cached = qc.getQueryData(qk.related("SRC", "topical")) as {
pages: { items: { status: string }[] }[];
frontend/src/modules/channels/WatchedVideosDialog.tsx
+3 -1
View File
@@ -54,7 +54,8 @@ export default function WatchedVideosDialog({
// The player's watched / in-progress marks: saved, then shown on this list and the feed's.
const onState = useStableCallback((id: string, status: VideoStatus) => {
writeVideoState(qc, id, status)
const write = writeVideoState(qc, id, status);
write
.then(() => {
qc.setQueryData(
key,
@@ -67,6 +68,7 @@ export default function WatchedVideosDialog({
void qc.invalidateQueries({ queryKey: qk.feed() });
})
.catch((e: unknown) => notifyActionError(e, t("errors.writeFailed.videoState")));
return write;
});
const closePlayer = useStableCallback(() => setPlaying(null));
frontend/src/modules/feed/Feed.tsx
+20 -6
View File
@@ -127,10 +127,17 @@ function matchesView(status: VideoStatus, show: string): boolean {
// the writes it ANSWERED before that refetch set out: a mark made while it was in flight is newer
// than its answer, and so is one sent just before it whose answer had not come back (the server may
// read the list first). Dropping either put a card marked watched back into an Unwatched view.
type Stamped<T> = { value: T; seq: number };
// `answered`: the server has taken the write — the live search then reads it from the marks
// (`markedRows`), where a newer word from elsewhere can follow it; the feed holds it until a refetch.
type Stamped<T> = { value: T; seq: number; answered?: boolean };
type Overrides<T> = Record<string, Stamped<T>>;
const unconfirmed = <T,>(o: Overrides<T>, answered: ReadonlySet<number>): Overrides<T> =>
Object.fromEntries(Object.entries(o).filter(([, s]) => !answered.has(s.seq)));
// The server took a write: say so on its own value only — a later write for the same video stands.
const withAnswer = <T,>(o: Overrides<T>, id: string, seq: number): Overrides<T> => {
const own = o[id];
return own?.seq === seq ? { ...o, [id]: { ...own, answered: true } } : o;
};
// A failed write takes back only its OWN value: a later write for the same video stands.
const withoutWrite = <T,>(o: Overrides<T>, id: string, seq: number): Overrides<T> => {
if (o[id]?.seq !== seq) return o;
@@ -400,6 +407,7 @@ export default function Feed({
// every loaded infinite-query page here was pure waste — 10 pages x rapid marks = a refetch
// storm (PERF-2). The next filter change / remount refetches fresh from the server.
answered.current.add(seq);
setOverrides((o) => withAnswer(o, id, seq));
qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" });
// The video the player is on may be none of this list's (a related one): it names itself.
const v = video ?? loadedRef.current.find((x) => x.id === id);
@@ -412,6 +420,7 @@ export default function Feed({
notifyActionError(e, i18n.t("errors.writeFailed.videoState"));
setOverrides((o) => withoutWrite(o, id, seq));
});
return write;
},
[qc, trackSessionWrite],
);
@@ -432,6 +441,7 @@ export default function Feed({
// Stale-mark only (see setState above): the optimistic override already reset the card.
.then(() => {
answered.current.add(seq);
setOverrides((o) => withAnswer(o, id, seq));
qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" });
})
.catch((e: unknown) => {
@@ -452,6 +462,7 @@ export default function Feed({
writeWatchLater(qc, id, saved)
.then(() => {
answered.current.add(seq);
setSavedOverrides((o) => withAnswer(o, id, seq));
qc.invalidateQueries({ queryKey: qk.playlists() });
qc.invalidateQueries({ queryKey: qk.playlist() });
})
@@ -471,22 +482,25 @@ export default function Feed({
const loaded: Video[] = (query.data?.pages ?? []).flatMap((p) => p.items);
// A mark made anywhere else (the player's related list, the Notification Center's undo) since a
// page was read is laid over it: a Load more on its way when the mark landed rebuilt the list
// from the pages it started from, without the mark.
// from the pages it started from, without the mark. The feed's own writes reach the marks too,
// once answered: over these rows only an unanswered one of them is laid (`withOverrides`), or an
// earlier click of the feed's hid a newer mark from elsewhere for the rest of the search.
const ytLoaded: Video[] = markedRows(ytQuery.data?.pages, videoMarks);
// onState's refs follow each commit: itself, and what the cards show (live search results if on).
useLayoutEffect(() => {
onStateRef.current = onState;
loadedRef.current = ytActive ? ytLoaded : loaded;
});
const withOverrides = (list: Video[]): Video[] =>
// `live`: the live search's rows, which carry the marks — there an answered write stands down.
const withOverrides = (list: Video[], live = false): Video[] =>
list
.map((v) => {
const status = overrides[v.id]; // read once so the narrowing sticks
return status ? { ...v, status: status.value } : v;
return status && !(live && status.answered) ? { ...v, status: status.value } : v;
})
.map((v) => {
const saved = savedOverrides[v.id];
return saved ? { ...v, saved: saved.value } : v;
return saved && !(live && saved.answered) ? { ...v, saved: saved.value } : v;
});
const merged: Video[] = withOverrides(loaded);
const items: Video[] = merged.filter((v) => matchesView(v.status, filters.show));
@@ -508,7 +522,7 @@ export default function Feed({
// user explicitly searched, so show everything) and no auto-pagination (each page costs quota).
if (ytActive) {
const ytItems: Video[] = sortYtResults(
withOverrides(ytLoaded),
withOverrides(ytLoaded, true),
ytSort.key as SortKey,
ytSort.dir,
lang,
frontend/src/modules/feed/Feed.ytLoadMore.dom.test.tsx
+70 -9
View File
@@ -4,7 +4,7 @@
// failure is said under them; before, the whole pane turned into the error text. Load more is the
// retry where trying again can help, and is not offered after a refusal it cannot answer (the day's
// cap, the feature turned off, the shared budget). And a mark made elsewhere while a Load more is on
// its way is not undone by the page landing.
// its way is not undone by the page landing, nor by an earlier write of the feed's own.
import { describe, it, expect, vi, beforeAll, afterEach } from "vitest";
import { act, render, screen, fireEvent, waitFor, cleanup } from "@testing-library/react";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
@@ -17,21 +17,38 @@ vi.mock("../../features/wizard/wizardContext", () => ({
useWizard: () => ({ openWizard: vi.fn() }),
}));
vi.mock("./VirtualFeed", () => ({
default: ({ items }: { items: { id: string; title: string; status: string }[] }) => (
<ol aria-label="results">
default: ({
items,
onState,
onToggleSave,
}: {
items: { id: string; title: string; status: string; saved?: boolean }[];
onState: (id: string, status: string) => void;
onToggleSave: (id: string, saved: boolean) => void;
}) => (
<>
<ol aria-label="results">
{items.map((v) => (
<li key={v.id}>
{v.title} ({v.status}
{v.saved ? ", saved" : ""})
</li>
))}
</ol>
{items.map((v) => (
<li key={v.id}>
{v.title} ({v.status})
</li>
<span key={v.id}>
<button onClick={() => onState(v.id, "hidden")}>hide {v.id}</button>
<button onClick={() => onToggleSave(v.id, true)}>save {v.id}</button>
</span>
))}
</ol>
</>
),
}));
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)"));
});
});
frontend/src/modules/files/useFileActions.tsx
+3 -3
View File
@@ -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) }),
frontend/src/modules/home/Home.tsx
+4 -3
View File
@@ -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: <SkipBack className="w-4 h-4" />,
icon: <RotateCcw className="w-4 h-4" />,
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],
);
frontend/src/modules/playlists/Playlists.dom.test.tsx
+13
View File
@@ -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<Record<string, { saved?: { value: boolean } }>>(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)] });
frontend/src/modules/playlists/Playlists.tsx
+7 -2
View File
@@ -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