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.
This commit is contained in:
2026-10-25 04:01:43 +01:00
parent b25179c06a
commit 84db570951
31 changed files with 699 additions and 189 deletions
@@ -42,6 +42,7 @@ import { useConfirm } from "../../components/ui/confirmContext";
import { api, type Me, type Video } from "../../lib/api";
import { clearAll, notify } from "../../lib/notifications";
import { qk } from "../../lib/queryKeys";
import { writeVideoState } from "../video/videoWrites";
beforeAll(() => {
class Stub {
@@ -104,7 +105,7 @@ const vid = (id: string, position_seconds: number): Video =>
function renderModal(ui: ReactNode) {
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
qc.setQueryData(qk.me(), me);
return render(<QueryClientProvider client={qc}>{ui}</QueryClientProvider>);
return Object.assign(render(<QueryClientProvider client={qc}>{ui}</QueryClientProvider>), { qc });
}
afterEach(() => {
@@ -962,6 +963,40 @@ describe("PlayerModal — the side panel on a wide window", () => {
expect(started.length).toBe(before);
});
// A description link put another video in the player: Restart on the related row brings the
// related video back from 0 — the seek moved the linked one.
it("wide: Restart on the related row after a description link goes back to it from 0", async () => {
wideWindow(true);
vi.spyOn(api, "videoDetail").mockResolvedValue({
description: "More: https://www.youtube.com/watch?v=linkedVid01",
} as Awaited<ReturnType<typeof api.videoDetail>>);
vi.spyOn(api, "related").mockResolvedValue({
items: [related("rel1"), related("rel2", 120)],
has_more: false,
next_cursor: null,
});
const A = vid("A", 40);
renderModal(
<PlayerModal video={A} queue={[A]} startIndex={0} onClose={() => {}} onState={() => {}} />,
);
act(() => playerEvents.onReady?.());
fireEvent.click(screen.getByRole("tab", { name: "player.related.title" }));
fireEvent.click(titleOf(await screen.findAllByRole("link", { name: "Video rel2" })));
await waitFor(() => expect(lastStart()?.videoId).toBe("rel2"));
act(() => playerEvents.onReady?.());
fireEvent.click(screen.getByRole("tab", { name: "player.description" }));
fireEvent.click((await screen.findAllByTitle("player.playInApp"))[0]!);
expect(loaded[loaded.length - 1]?.videoId).toBe("linkedVid01");
fireEvent.click(screen.getByRole("tab", { name: "player.related.title" }));
const list = await screen.findByTestId("player-related");
const row = within(list)
.getAllByRole("link", { name: "Video rel2" })[0]!
.closest("[data-nav-index]")!;
fireEvent.click(within(row as HTMLElement).getAllByTitle("card.restartTitle")[0]!);
expect(loaded[loaded.length - 1]).toEqual({ videoId: "rel2", startSeconds: 0 });
expect(seeks).not.toContain(0);
});
// The toggle remembers each video's mark: a related one's did not overwrite the opened one's.
it("wide: Back shows the opened video's own mark, not the one it was opened with", async () => {
const onState = vi.fn();
@@ -979,6 +1014,45 @@ describe("PlayerModal — the side panel on a wide window", () => {
expect(screen.queryAllByTitle("player.markWatched")).toHaveLength(0);
});
// A mark the session confirmed elsewhere while the video plays (the related row's own menu, a
// notice's Undo) reaches the toggle; alone, its first click re-sent the same status.
// A mark from before the player opened is older than the item it was opened with (a list read
// since): the item's own status stands.
it("goes by the opened item, not a mark made before it opened", async () => {
vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {}));
vi.spyOn(api, "setState").mockResolvedValue({ ok: true } as never);
const A = vid("A", 0);
wideWindow(true);
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
qc.setQueryData(qk.me(), me);
await writeVideoState(qc, "A", "watched");
await new Promise((r) => setTimeout(r, 5));
render(
<QueryClientProvider client={qc}>
<PlayerModal video={A} queue={[A]} startIndex={0} onClose={() => {}} onState={() => {}} />
</QueryClientProvider>,
);
await settle();
expect(screen.getAllByTitle("player.markWatched").length).toBeGreaterThan(0);
expect(screen.queryAllByTitle("player.watchedUnmark")).toHaveLength(0);
});
it("follows the playing video's mark written elsewhere, and its own click after that", async () => {
vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {}));
vi.spyOn(api, "setState").mockResolvedValue({ ok: true } as never);
const onState = vi.fn();
const A = vid("A", 0);
wideWindow(true);
const { qc } = renderModal(
<PlayerModal video={A} queue={[A]} startIndex={0} onClose={() => {}} onState={onState} />,
);
expect(screen.getAllByTitle("player.markWatched").length).toBeGreaterThan(0);
await act(async () => void (await writeVideoState(qc, "A", "watched")));
fireEvent.click(await screen.findByTitle("player.watchedUnmark"));
expect(onState.mock.calls.map((c) => c.slice(0, 2))).toEqual([["A", "new"]]);
expect(screen.getAllByTitle("player.markWatched").length).toBeGreaterThan(0);
});
it("wide: a queue step leaves the related video for the queue's next item", async () => {
await playRel2(() => {}, [vid("A", 40), vid("B", 0)]);
next();
+27 -12
View File
@@ -56,6 +56,7 @@ import DownloadButton from "../downloads/DownloadButton";
import ClipPanel, { ClipCropFrame, type ClipCrop } from "./ClipPanel";
import { markInAt, markOutAt, openAt, type ClipRange } from "../../lib/clipRange";
import { api, type Video, type VideoStatus } from "../../lib/api";
import { markedValue, useVideoMarks } from "../../lib/progressCache";
import type { YTPlayer } from "../../lib/youtube";
import {
channelYouTubeUrl,
@@ -543,19 +544,30 @@ export default function PlayerModal({
});
// Local mirror of watch status so the toggle reacts instantly; changes are propagated to the feed
// (and server) via onState. The mirror carries the id it DESCRIBES, and a write for any other id
// is ignored rather than displayed — that is the enforcement, and it has to be in code because
// one writer arrives late by construction: useYtProgress's cleanup for the OUTGOING item runs
// after the commit that already advanced `active`, and near the end of a clip it auto-marks that
// item watched (playerModalHooks.ts `maybeAutoWatch`). Keyed, that stray write is simply not this
// item's, so the toggle falls back to `subject.status` and no ordering has to be argued about.
// Unkeyed it landed last and painted the NEXT video as watched; a corrective click then sent
// onState(next, "new") for an item that was already new.
const [marks, setMarks] = useState<Record<string, VideoStatus>>({});
const watched = (marks[subject.id] ?? subject.status) === "watched";
// (and server) via onState. Each mark is kept under the id it was MADE for, and that has to be in
// code because one writer arrives late by construction: useYtProgress's cleanup for the OUTGOING
// item runs after the commit that already advanced `active`, and near the end of a clip it
// auto-marks that item watched (playerModalHooks.ts `maybeAutoWatch`). Keyed, that late write is
// recorded for the item it belongs to (shown again if the user steps back to it) and never
// painted on the next one; unkeyed it landed last and painted the NEXT video as watched, and a
// corrective click then sent onState(next, "new") for an item that was already new.
// The newest word wins: this toggle's own mark, or one the session confirmed elsewhere since the
// player opened (the related row's own menu, a notice's Undo — `useVideoMarks`), else the item as
// it was opened. The toggle alone missed those, and its first click re-sent the same status.
const [marks, setMarks] = useState<Record<string, { status: VideoStatus; at: number }>>({});
const [openedAt] = useState(() => Date.now());
const sessionMarks = useVideoMarks();
const own = marks[subject.id];
const confirmed = markedValue(sessionMarks, subject.id, "status");
const elsewhere = confirmed && confirmed.at >= openedAt ? confirmed : undefined;
const status =
own && (!elsewhere || own.at >= elsewhere.at)
? own.status
: (elsewhere?.value ?? subject.status);
const watched = status === "watched";
const setWatched = (on: boolean) => {
const next = on ? "watched" : "new";
setMarks((m) => ({ ...m, [subject.id]: next }));
setMarks((m) => ({ ...m, [subject.id]: { status: next, at: Date.now() } }));
onState(subject.id, next, subject);
};
@@ -727,8 +739,11 @@ export default function PlayerModal({
setRelatedOpen(false);
// The video already playing (its row stays in the list): a new player for the same id would
// never be built (one per video), so a Restart is a seek, and a plain pick just keeps playing.
// Unless a description link put another video in the player meanwhile (`navigated`): then the
// pick goes back to this one — the seek moved the linked video, and the pick did nothing.
if (v.id === subject.id) {
if (startAt != null) seekTo(startAt);
if (navigated) loadVideo(subject.id, startAt);
else if (startAt != null) seekTo(startAt);
return;
}
setRelated({ video: v, startAt });
@@ -69,11 +69,11 @@ const page = (items: RelatedVideo[], next: string | null = null) => ({
function setup(
onPlay: (v: Video, startAt: number | null) => void = () => {},
onWrite?: (write: Promise<unknown>) => void,
qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }),
) {
vi.spyOn(api, "comfortPins").mockResolvedValue({ items: [] } as never);
vi.spyOn(api, "downloadIndex").mockResolvedValue({});
const saved = vi.spyOn(api, "savePrefs").mockResolvedValue({} as never);
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
render(
<QueryClientProvider client={qc}>
<FloatingTree>
@@ -343,6 +343,19 @@ describe("RelatedVideos", () => {
expect(titles()).toEqual(["Video C", "Video B"]);
});
// Hidden in this session, then un-hidden on another device: a list read after the mark carries
// the server's word, and the mark does not paint over it.
it("shows a list read after a mark as the server has it", async () => {
vi.spyOn(api, "setState").mockResolvedValue({ ok: true } as never);
vi.spyOn(api, "related").mockResolvedValue(page([row("A"), row("B")]));
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
await writeVideoState(qc, "A", "hidden");
await new Promise((r) => setTimeout(r, 5)); // the list is read a moment after the mark
setup(undefined, undefined, qc);
await title("Video B");
expect(titles()).toEqual(["Video A", "Video B"]);
});
// The Notification Center's undo, the feed's reset, a card elsewhere: every write reaches it.
it("follows a video's state written anywhere else", async () => {
vi.spyOn(api, "related").mockResolvedValue(page([row("A"), row("B")]));
@@ -14,7 +14,7 @@ import {
import { qk } from "../../lib/queryKeys";
import { nextPageOnClick } from "../../lib/nextPage";
import { usePref, type UsePrefOptions } from "../../lib/usePref";
import { useVideoMarks } from "../../lib/progressCache";
import { markedRows, stampFetch, useVideoMarks } from "../../lib/progressCache";
import { notifyActionError } from "../../lib/actionErrors";
import { Button } from "../../components/ui/Button";
import SegmentedToggle from "../../components/ui/SegmentedToggle";
@@ -68,7 +68,7 @@ export function RelatedVideos({
// another slot of the day's cap.
const query = useInfiniteQuery({
queryKey: qk.related(videoId, region),
queryFn: ({ pageParam }) => api.related(videoId, region, pageParam),
queryFn: ({ pageParam }) => stampFetch(() => api.related(videoId, region, pageParam)),
initialPageParam: null as string | null,
getNextPageParam: (last) => (last.has_more ? last.next_cursor : undefined),
staleTime: Infinity,
@@ -128,11 +128,10 @@ export function RelatedVideos({
const menu = useContextMenu<Video>();
const openId = menu.open ? menu.target?.id : undefined;
// What this session confirmed about a row (here or anywhere else), then what is on its way.
// What this session confirmed about a row since its page was read (here or anywhere else), then
// what is on its way.
const marks = useVideoMarks();
const rows = (query.data?.pages ?? [])
.flatMap((p) => p.items)
.map((v) => (marks[v.id] ? { ...v, ...marks[v.id] } : v))
const rows = markedRows(query.data?.pages, marks)
.map((v) => pending.reduce((acc, w) => (w.id === v.id ? { ...acc, ...w.change } : acc), v))
.filter((v) => !isSeen(v));
const followed = new Set(
@@ -0,0 +1,74 @@
// @vitest-environment jsdom
//
// The queue transport cluster, in both of its places. The auto-advance mode wears glyphs of its own:
// on the side panel's glyph-only row the skip arrows are Previous / Next, and a mode that wore them
// too put two identical ⏭ side by side, the second one silently changing a saved setting. The modes
// are cycles, not switches (lit, no aria-pressed), and every control's tooltip names it and its mode
// in both places. i18n is key-passthrough.
import { afterEach, describe, expect, it, vi } from "vitest";
import { cleanup, render, screen } from "@testing-library/react";
vi.mock("react-i18next", async (importOriginal) => ({
...(await importOriginal<typeof import("react-i18next")>()),
useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }),
}));
import { PlayerTransportControls } from "./playerModalControls";
import { AUTO_MODES, LOOP_MODES, type AutoMode, type LoopMode } from "../../lib/playerModes";
afterEach(cleanup);
function mount(variant: "modal" | "docked", autoMode: AutoMode, loopMode: LoopMode = "off") {
render(
<PlayerTransportControls
variant={variant}
index={1}
queueLength={3}
autoMode={autoMode}
loopMode={loopMode}
onPrev={() => {}}
onNext={() => {}}
onCycleAuto={() => {}}
onCycleLoop={() => {}}
/>,
);
}
const glyph = (el: HTMLElement) => el.querySelector("svg")!.getAttribute("class");
const button = (name: string | RegExp) => screen.getByRole("button", { name });
describe("PlayerTransportControls", () => {
for (const variant of ["modal", "docked"] as const)
for (const mode of AUTO_MODES)
it(`${variant}: auto-advance ${mode} wears neither Previous's glyph nor Next's`, () => {
mount(variant, mode);
const auto = glyph(button(/^player\.autoAdvance\.label: /));
expect(auto).not.toBe(glyph(button("player.previous")));
expect(auto).not.toBe(glyph(button("player.next")));
});
for (const variant of ["modal", "docked"] as const)
it(`${variant}: the modes are lit when on, and announce no toggle`, () => {
mount(variant, "next", "off");
const auto = button(/^player\.autoAdvance\.label: /);
const loop = button(/^player\.loop\.label: /);
expect(auto.hasAttribute("aria-pressed")).toBe(false);
expect(loop.hasAttribute("aria-pressed")).toBe(false);
expect(auto.hasAttribute("data-lit")).toBe(true);
expect(loop.hasAttribute("data-lit")).toBe(false);
});
for (const variant of ["modal", "docked"] as const)
it(`${variant}: each mode's tooltip names it and the mode it is in`, () => {
for (const loopMode of LOOP_MODES) {
mount(variant, "random", loopMode);
expect(button(/^player\.autoAdvance\.label: /).title).toBe(
"player.autoAdvance.label: player.autoAdvance.random · player.autoAdvance.hint",
);
expect(button(/^player\.loop\.label: /).title).toBe(
`player.loop.label: player.loop.${loopMode} · player.loop.hint`,
);
cleanup();
}
});
});
@@ -1,6 +1,6 @@
import type { CSSProperties, Ref } from "react";
import type { CSSProperties, ReactNode, Ref } from "react";
import { useTranslation } from "react-i18next";
import { Repeat, Repeat1, Shuffle, SkipBack, SkipForward } from "lucide-react";
import { ListEnd, ListStart, Repeat, Repeat1, Shuffle, SkipBack, SkipForward } from "lucide-react";
import type { VideoDetail } from "../../lib/api";
import type { AutoMode, LoopMode } from "../../lib/playerModes";
import { Button } from "../../components/ui/Button";
@@ -13,13 +13,16 @@ import { renderDescription } from "./descriptionLinks";
// the sibling of plexPlayerControls.tsx. No state or effects here; all behaviour comes in via props.
// The auto-advance / loop modes it renders are defined in `lib/playerModes`.
/** The queue transport cluster — prev / counter / next, then the auto-advance and loop mode toggles.
* Rendered in two places with identical logic: the modal's queue bar (`variant:"modal"`, normal glass
* theme, glyphs only so the whole cluster fits one line of the side panel) and the big-view docked
* control bar (`variant:"docked"`, white-on-dark, labels shown from a breakpoint up). Only the theme +
* label visibility differ; the behaviour is one, so a mode/icon change lands once. Every control keeps
* its full name (with the shortcut) as its tooltip and accessible name, so a glyph-only one still says
* what it does and which mode it is in. The wrapper (fragment vs. bordered row) stays at the call site. */
/** The queue transport cluster — prev / counter / next, then the auto-advance and loop modes.
* Rendered in two places from ONE list of controls: the modal's queue bar (`variant:"modal"`, normal
* glass theme, glyphs only so the whole cluster fits one line of the side panel) and the big-view
* docked control bar (`variant:"docked"`, white-on-dark, labels shown from a breakpoint up). What a
* control does, its glyph and its tooltip are said once; only the primitive and the label's
* visibility differ. Every control's tooltip carries its full name (and the shortcut, where it has
* one), so a glyph-only one still says what it does and which mode it is in. The modes are cycles,
* not switches: `lit` when not off, and the name says which. Auto-advance wears list glyphs of its
* own — the skip arrows are Previous / Next, beside it (DESIGN.md "A glyph means one thing"). The
* wrapper (fragment vs. bordered row) stays at the call site. */
export function PlayerTransportControls({
variant,
index,
@@ -47,102 +50,101 @@ export function PlayerTransportControls({
const divider = docked ? "h-4 w-px bg-white/20" : "w-px h-4 bg-border";
const autoName = `${t("player.autoAdvance.label")}: ${t(`player.autoAdvance.${autoMode}`)}`;
const loopName = `${t("player.loop.label")}: ${t(`player.loop.${loopMode}`)}`;
const autoIcon =
autoMode === "prev" ? <SkipBack /> : autoMode === "random" ? <Shuffle /> : <SkipForward />;
const loopIcon = loopMode === "one" ? <Repeat1 /> : <Repeat />;
const count = (
<span className={counter}>
{index + 1} / {queueLength}
</span>
);
if (!docked)
return (
<>
<IconButton
onClick={onPrev}
disabled={index === 0}
label={t("player.previous")}
title={`${t("player.previous")} · Shift+←`}
>
<SkipBack />
</IconButton>
{count}
<IconButton
onClick={onNext}
disabled={index === queueLength - 1}
label={t("player.next")}
title={`${t("player.next")} · Shift+→`}
>
<SkipForward />
</IconButton>
<span className={divider} />
<IconButton
pressed={autoMode !== "off"}
onClick={onCycleAuto}
label={autoName}
title={`${autoName} · ${t("player.autoAdvance.hint")}`}
>
{autoIcon}
</IconButton>
<IconButton
pressed={loopMode !== "off"}
onClick={onCycleLoop}
label={loopName}
title={`${loopName} · ${t("player.loop.hint")}`}
>
{loopIcon}
</IconButton>
</>
type Control = {
key: string;
glyph: ReactNode;
name: string;
title: string;
onClick: () => void;
disabled?: boolean;
lit?: boolean;
/** From which width the docked bar shows the name beside the glyph. */
labelFrom: "sm" | "md";
/** The glyph after the name (Next points the way it goes). */
trailing?: boolean;
};
const step: Control[] = [
{
key: "prev",
glyph: <SkipBack />,
name: t("player.previous"),
title: `${t("player.previous")} · Shift+←`,
onClick: onPrev,
disabled: index === 0,
labelFrom: "sm",
},
{
key: "next",
glyph: <SkipForward />,
name: t("player.next"),
title: `${t("player.next")} · Shift+→`,
onClick: onNext,
disabled: index === queueLength - 1,
labelFrom: "sm",
trailing: true,
},
];
// Persistent playback settings — cycle on click, saved to your account.
const modes: Control[] = [
{
key: "auto",
glyph:
autoMode === "prev" ? <ListStart /> : autoMode === "random" ? <Shuffle /> : <ListEnd />,
name: autoName,
title: `${autoName} · ${t("player.autoAdvance.hint")}`,
onClick: onCycleAuto,
lit: autoMode !== "off",
labelFrom: "md",
},
{
key: "loop",
glyph: loopMode === "one" ? <Repeat1 /> : <Repeat />,
name: loopName,
title: `${loopName} · ${t("player.loop.hint")}`,
onClick: onCycleLoop,
lit: loopMode !== "off",
labelFrom: "md",
},
];
const render = (c: Control) =>
docked ? (
<Button
key={c.key}
variant="overlay"
size="sm"
icon={c.trailing ? undefined : c.glyph}
onClick={c.onClick}
disabled={c.disabled}
lit={c.lit}
label={c.name}
title={c.title}
>
<span className={c.labelFrom === "sm" ? "hidden sm:inline" : "hidden md:inline"}>
{c.name}
</span>
{c.trailing && c.glyph}
</Button>
) : (
<IconButton
key={c.key}
onClick={c.onClick}
disabled={c.disabled}
lit={c.lit}
label={c.name}
title={c.title}
>
{c.glyph}
</IconButton>
);
return (
<>
<Button
variant="overlay"
size="sm"
icon={<SkipBack />}
onClick={onPrev}
disabled={index === 0}
label={t("player.previous")}
title={`${t("player.previous")} · Shift+←`}
>
<span className="hidden sm:inline">{t("player.previous")}</span>
</Button>
{count}
<Button
variant="overlay"
size="sm"
onClick={onNext}
disabled={index === queueLength - 1}
label={t("player.next")}
title={`${t("player.next")} · Shift+→`}
>
<span className="hidden sm:inline">{t("player.next")}</span>
<SkipForward />
</Button>
{render(step[0]!)}
<span className={counter}>
{index + 1} / {queueLength}
</span>
{render(step[1]!)}
<span className={divider} />
{/* Persistent playback settings — cycle on click, saved to your account. */}
<Button
variant="overlay"
size="sm"
pressed={autoMode !== "off"}
icon={autoIcon}
onClick={onCycleAuto}
label={autoName}
title={t("player.autoAdvance.hint")}
>
<span className="hidden md:inline">{autoName}</span>
</Button>
<Button
variant="overlay"
size="sm"
pressed={loopMode !== "off"}
icon={loopIcon}
onClick={onCycleLoop}
label={loopName}
title={t("player.loop.hint")}
>
<span className="hidden md:inline">{loopName}</span>
</Button>
{modes.map(render)}
</>
);
}