diff --git a/backend/app/downloads/backfill.py b/backend/app/downloads/backfill.py index ac2293b4..a79ed4e2 100644 --- a/backend/app/downloads/backfill.py +++ b/backend/app/downloads/backfill.py @@ -89,14 +89,17 @@ def _retitle(db: Session, a: MediaAsset, adapter: adapters.SourceAdapter, url: s named = normalize_title(adapter.title(info)) if not named or adapter.is_placeholder_title(named): return False - # One conditional write, not a read and a compare: a name its owner gives while this runs (the - # poster step can take seconds before the commit) is not overwritten by what was read before it. + # One conditional write, not a read and a compare: a name its owner gives while the page is read + # is not overwritten by what was read before it. The asset first, as every other writer of the + # two locks them (`service.lock_own_job`): the session does not flush by itself. + placeholder = a.title + a.title = named + db.flush() db.execute( update(DownloadJob) - .where(DownloadJob.asset_id == a.id, DownloadJob.display_name == a.title) + .where(DownloadJob.asset_id == a.id, DownloadJob.display_name == placeholder) .values(display_name=named[:DISPLAY_TEXT_MAX]) ) - a.title = named return True @@ -127,6 +130,9 @@ def run() -> dict: if adapter.wants_metadata_backfill(): url = a.source_webpage_url or service.source_url(a.source_kind, a.source_ref) if _retitle(db, a, adapter, url): + # Before the poster step: it can cut a frame for 30 s, and these locks held + # across it block a rename and deadlock a delete. + db.commit() retitled += 1 log.info("asset %s retitled", a.id) # the title names what they downloaded if a.poster_path is None: diff --git a/backend/app/reader/service.py b/backend/app/reader/service.py index fa651005..1041b822 100644 --- a/backend/app/reader/service.py +++ b/backend/app/reader/service.py @@ -255,17 +255,6 @@ def mark(db: Session, user: User, video_id: str, status: str) -> None: ) -def clear_state(db: Session, user: User, video_id: str) -> None: - """The video back to pristine for this user: status, resume point and `watched_at` all go. - Playlist membership lives elsewhere and stays.""" - lock_takeout(db, user) # as `unmark` - row = queries.video_state(db, user, video_id) - if row is not None: - db.delete(row) - _remember_unmark(db, user, video_id) - db.flush() - - def set_progress(db: Session, user: User, video_id: str, position: int) -> None: """The player's resume point. 0 clears it: a row holding nothing else goes, one with a status (watched, hidden) keeps it with the position zeroed. The status itself is never changed here. diff --git a/backend/app/routes/feed.py b/backend/app/routes/feed.py index f36294dd..43581896 100644 --- a/backend/app/routes/feed.py +++ b/backend/app/routes/feed.py @@ -204,22 +204,21 @@ def set_video_state( return {"ok": True, "video_id": video_id, "status": status} -@router.delete("/videos/{video_id}/state") -def clear_video_state( +@router.delete("/videos/{video_id}/progress") +def clear_video_progress( video_id: str, user: User = Depends(current_user), db: Session = Depends(get_db), ) -> dict: - """Reset a video to pristine for this user — drop the whole VideoState row (status, - watch position and watched_at), as if it had never been opened. Saved/playlist membership - lives elsewhere and is untouched.""" + """Forget where the user left a video: its resume point goes, its status (watched, hidden) stays. + Saved/playlist membership lives elsewhere and is untouched.""" try: - service.clear_state(db, user, video_id) + service.set_progress(db, user, video_id, 0) db.commit() - # silent-ok: the row went under the delete, which is the outcome the delete wanted. + # silent-ok: the row went under the write, which is the outcome the write wanted. except StaleDataError: db.rollback() - return {"ok": True, "video_id": video_id, "status": "new", "position_seconds": 0} + return {"ok": True, "video_id": video_id, "position_seconds": 0} @router.post("/videos/{video_id}/progress") diff --git a/backend/tests/test_facebook_titles.py b/backend/tests/test_facebook_titles.py index eb92db1f..25372183 100644 --- a/backend/tests/test_facebook_titles.py +++ b/backend/tests/test_facebook_titles.py @@ -6,9 +6,7 @@ no post text to fall back on, and the download name copied the placeholder too. the same reel (yt-dlp 2026.08.19, as on prod) still carries the post's text in its description. """ -import threading - -from sqlalchemy import update +from sqlalchemy import select, update from app.config import settings from app.db import SessionLocal @@ -149,8 +147,8 @@ def test_the_backfill_retitles_a_placeholder_download_and_the_names_that_repeate def test_a_name_given_while_the_backfill_runs_stays(db, make_user, monkeypatch): - # The owner renames the download between the backfill's retitle and its commit (the poster step - # sits between them): the owner's name wins, it is not overwritten by what was read before it. + # The owner renames the download while the backfill reads the page: the owner's name wins, it is + # not overwritten by what was read before it. _fake_ydl(monkeypatch, _PUBLIC) owner = make_user() asset = MediaAsset( @@ -178,7 +176,9 @@ def test_a_name_given_while_the_backfill_runs_stays(db, make_user, monkeypatch): db.add(job) db.commit() - def rename(): + read_page = backfill.public_title_info + + def renamed_while_reading(*args): with SessionLocal() as other: other.execute( update(DownloadJob) @@ -186,27 +186,64 @@ def test_a_name_given_while_the_backfill_runs_stays(db, make_user, monkeypatch): .values(display_name="Office jokes") ) other.commit() + return read_page(*args) - def poster_while_renamed(a, root): - # A rename that must wait for the backfill's write waits here, and lands after its commit. - t = threading.Thread(target=rename) - t.start() - t.join(timeout=0.5) - renamers.append(t) - return None - - renamers: list[threading.Thread] = [] - monkeypatch.setattr(backfill.editmod, "ensure_poster", poster_while_renamed) + monkeypatch.setattr(backfill, "public_title_info", renamed_while_reading) + monkeypatch.setattr(backfill.editmod, "ensure_poster", lambda a, root: None) assert backfill.run()["retitled"] == 1 - for t in renamers: - t.join(timeout=10) - assert not t.is_alive() db.expire_all() assert db.get(DownloadJob, job.id).display_name == "Office jokes" +def test_the_poster_step_holds_no_row_lock(db, make_user, monkeypatch): + # It can cut a frame for 30 s. A delete locks the asset and then the job: with the job held here + # and the asset still to write, that is a deadlock, and a rename waits out the whole cut. + _fake_ydl(monkeypatch, _PUBLIC) + owner = make_user() + asset = MediaAsset( + source_kind="url", + source_ref=_URL, + format_sig="fb", + status="ready", + rel_path="facebook/x.mp4", + uploader_url="https://www.facebook.com/1", + title="Facebook", + ref_count=1, + ) + db.add(asset) + db.flush() + job = DownloadJob( + user_id=owner.id, + asset_id=asset.id, + source_kind="url", + source_ref=_URL, + status="done", + queue_pos=0, + profile_snapshot={}, + display_name="Facebook", + ) + db.add(job) + db.commit() + asset_id, job_id = asset.id, job.id + + def poster_finds_rows_free(a, root): + with SessionLocal() as other: + other.execute( + select(MediaAsset).where(MediaAsset.id == asset_id).with_for_update(nowait=True) + ) + other.execute( + select(DownloadJob).where(DownloadJob.id == job_id).with_for_update(nowait=True) + ) + other.rollback() + return None + + monkeypatch.setattr(backfill.editmod, "ensure_poster", poster_finds_rows_free) + + assert backfill.run()["retitled"] == 1 + + def test_a_download_titled_facebook_is_named_from_the_signed_out_page( db, make_user, monkeypatch, tmp_path ): diff --git a/backend/tests/test_owner_scoping.py b/backend/tests/test_owner_scoping.py index c85a69de..51ccb83f 100644 --- a/backend/tests/test_owner_scoping.py +++ b/backend/tests/test_owner_scoping.py @@ -181,7 +181,7 @@ def test_clearing_a_video_leaves_another_users_state(db, make_user, as_user): _videos(db, "v1") db.add(VideoState(user_id=other.id, video_id="v1", status="hidden", position_seconds=90)) db.commit() - assert as_user(me).delete("/api/videos/v1/state").status_code == 200 + assert as_user(me).delete("/api/videos/v1/progress").status_code == 200 assert _video_state(db, other) == ("hidden", 90) diff --git a/backend/tests/test_reader_service.py b/backend/tests/test_reader_service.py index 97522c3d..63e4c0b8 100644 --- a/backend/tests/test_reader_service.py +++ b/backend/tests/test_reader_service.py @@ -190,9 +190,6 @@ def test_a_cleared_resume_point_drops_a_bare_row_and_zeroes_one_with_a_status(db db.commit() assert _state(db, user, "v1") is None assert (_state(db, user, "v2").status, _state(db, user, "v2").position_seconds) == ("hidden", 0) - service.clear_state(db, user, "v2") - db.commit() - assert _state(db, user, "v2") is None # --- playlists ------------------------------------------------------------------------------- diff --git a/backend/tests/test_route_commits.py b/backend/tests/test_route_commits.py index 5dea6860..6491fec5 100644 --- a/backend/tests/test_route_commits.py +++ b/backend/tests/test_route_commits.py @@ -101,8 +101,10 @@ def test_a_video_state_cleared_through_the_route_is_gone(db, make_user, as_user) _video(db) db.add(VideoState(user_id=user.id, video_id="v1", status="hidden", position_seconds=90)) db.commit() - assert as_user(user).delete("/api/videos/v1/state").status_code == 200 - assert _state(db, user) is None + assert as_user(user).delete("/api/videos/v1/progress").status_code == 200 + db.expire_all() + row = _state(db, user) + assert (row.status, row.position_seconds) == ("hidden", 0) # --- saved views ------------------------------------------------------------------------------ diff --git a/backend/tests/test_takeout.py b/backend/tests/test_takeout.py index d63f3475..896b19ab 100644 --- a/backend/tests/test_takeout.py +++ b/backend/tests/test_takeout.py @@ -292,17 +292,16 @@ def test_a_known_video_is_marked_watched_at_its_takeout_time_and_the_rest_wait_o assert (states["older000000"].status, states["older000000"].position_seconds) == ("watched", 40) -@pytest.mark.parametrize("undo", ["unmark", "clear_state"]) -def test_a_video_the_user_un_marked_here_is_not_marked_again_by_an_older_view(db, make_user, undo): - """Un-marking (or resetting) a video leaves no state row, and the import inserted a watched one - over the user's own choice. A view on YouTube after the un-mark is a new view, and counts.""" - user = make_user(f"{undo}@example.com") +def test_a_video_the_user_un_marked_here_is_not_marked_again_by_an_older_view(db, make_user): + """Un-marking a video leaves no state row, and the import inserted a watched one over the user's + own choice. A view on YouTube after the un-mark is a new view, and counts.""" + user = make_user("unmark@example.com") for vid in ("undone00000", "again000000"): _video(db, vid) db.commit() for vid in ("undone00000", "again000000"): service.mark(db, user, vid, "watched") - getattr(service, undo)(db, user, vid) + service.unmark(db, user, vid) db.commit() assert _states(db, user) == {} later = datetime.now(timezone.utc) + timedelta(minutes=5) @@ -603,7 +602,6 @@ def test_staging_and_settling_take_the_users_one_lock(db, make_user): ), # Between the import's read of the un-marks and its marking, one would be overwritten. "unmark": lambda s, u: service.unmark(s, u, "aaaaaaaaaaa"), - "reset": lambda s, u: service.clear_state(s, u, "aaaaaaaaaaa"), } with SessionLocal() as holder: service.lock_takeout(holder, holder.get(User, user.id)) diff --git a/docs/DESIGN.md b/docs/DESIGN.md index ea073709..c173a62e 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -256,7 +256,7 @@ Also in force: - **A glyph means one thing.** Play (▷) starts playback, or resumes a paused transfer; a retry is the refresh arrow (`RefreshCw`), and so is doing a download again; a reset or revert is the counter-clockwise arrow (`RotateCcw`), and so is playing from the beginning, on the card and in its - menu alike; clearing a video's progress and status is the eraser (`Eraser`). A failed download's Retry + menu alike; clearing a video's watch progress is the eraser (`Eraser`). A failed download's Retry had worn the Play glyph, which promised a video. The skip arrows (⏮ ⏭) are Previous / Next; the auto-advance mode beside them wears list glyphs (`ListEnd` next, `ListStart` previous, `Shuffle` random; off is `ListEnd`, not lit) — a mode in the skip arrows put two identical ⏭ on the @@ -833,3 +833,4 @@ of a full-width panel and nothing on the right. | 2026-11-01 | Toolbar group glyphs (owner D9 a, D10 a, D11 a) | The feed's, channel page's and Plex Browse's filter pills name their group by a glyph (`Eye` watch state, `Shapes` content types, `ArrowDownUp` sort), the word in the tooltip and the accessible name; Sort + View end the toolbar's row, the chip row shows only with a chip; a channel page has no count pill | The value alone; Sort + View on a row of their own | "Watch state:" and "Sort:" cost a tablet row its room; the value says enough once the glyph names the group; the channel header already counts the videos | | 2026-11-01 | Channel header on a tablet (owner E2) | Below `xl` the action row drops its two words: "Prio" (the stepper's tooltip names it) and "Subscribed" (a ✓ glyph; its name in the aria-label, Unsubscribe in the tooltip); the tags keep the row's rest, folding into "+N" | The tag lane on a full line of its own below ~12rem; the tags under the channel name | On an iPad (820 px, 44 px targets) the lane got 29 px, a sliver; now 130 px, and every tag shows at 1180 | | 2026-11-01 | Clear watch progress (owner E a) | A video menu's Reset reads "Clear watch progress" in English (HU "Megtekintés visszaállítása" stays); "Reset view" is only a page's view reset (sort, filters, grouping) | "Reset view" for both; the playlist page's view reset renamed instead | On a playlist page the two stood one right-click apart under one name, and one of them erases the video's progress with no undo | +| 2026-11-01 | Clear watch progress clears only that (owner A) | The row shows only on a video with a resume point and clears that alone: a hidden video stays hidden, a watched one watched. Un-hiding and un-watching have rows of their own | diff --git a/frontend/src/components/ui/Tooltip.tsx b/frontend/src/components/ui/Tooltip.tsx index 6ce58ff6..78e17813 100644 --- a/frontend/src/components/ui/Tooltip.tsx +++ b/frontend/src/components/ui/Tooltip.tsx @@ -11,18 +11,22 @@ type Side = "top" | "bottom" | "left" | "right"; * is never clipped by an overflow/stacking ancestor, flips when the preferred side runs out of room * and slides back inside the viewport near an edge. * `always` bypasses the hints toggle (for a label that IS the primary affordance, e.g. the slim nav - * rail's flyout names); `openDelay` waits before showing (a rest-on-the-icon feel). */ + * rail's flyout names); `openDelay` waits before showing (a rest-on-the-icon feel); `stable` keeps + * the wrapper with no hint to show, so a hint that comes and goes does not remount the element and + * cost it the focus. */ export default function Tooltip({ hint, side = "top", always = false, openDelay = 0, + stable = false, children, }: { hint: string; side?: Side; always?: boolean; openDelay?: number; + stable?: boolean; children: React.ReactNode; }) { const hintsOn = useSyncExternalStore(subscribeHints, hintsEnabled, hintsEnabled); @@ -42,7 +46,7 @@ export default function Tooltip({ }); const { setReference } = pop.refs; - if (!enabled || !hint) return <>{children}; + if ((!enabled || !hint) && !stable) return <>{children}; return ( <> diff --git a/frontend/src/components/ui/unavailable.ts b/frontend/src/components/ui/unavailable.ts index 3c164467..5504c83b 100644 --- a/frontend/src/components/ui/unavailable.ts +++ b/frontend/src/components/ui/unavailable.ts @@ -1,7 +1,10 @@ /** An unavailable control's props. With a reason it stays focusable (`aria-disabled`, which - * `FocusSafeButton` keeps inert to a click, and the reason in its tooltip — DESIGN.md "Unavailable - * with a reason stays focusable"); without one it is plainly off. */ + * `FocusSafeButton` keeps inert to a click, and the reason read out with its name; the caller shows + * it in a tooltip — DESIGN.md "Unavailable with a reason stays focusable"); without one it is + * plainly off. */ export function unavailable(disabled: boolean | undefined, reason: string | undefined) { if (!disabled) return {}; - return reason ? { "aria-disabled": true as const } : { disabled: true }; + return reason + ? { "aria-disabled": true as const, "aria-description": reason } + : { disabled: true }; } diff --git a/frontend/src/features/player/PlayerModal.dom.test.tsx b/frontend/src/features/player/PlayerModal.dom.test.tsx index 790cdc44..1edf96b3 100644 --- a/frontend/src/features/player/PlayerModal.dom.test.tsx +++ b/frontend/src/features/player/PlayerModal.dom.test.tsx @@ -1050,14 +1050,14 @@ describe("PlayerModal — the side panel on a wide window", () => { expect(lastStart()?.start ?? 0).toBe(0); }); - // A reset from the related row is where it starts now: the position this session stored for it + // A clear from the related row is where it starts now: the position this session stored for it // (which a plain pick resumes from) is forgotten with it, not replayed and checkpointed back. - it("wide: a related video reset from its row starts at 0 on a plain pick", async () => { + it("wide: a related video cleared from its row starts at 0 on a plain pick", async () => { vi.spyOn(api, "saveProgress").mockImplementation(async (id: string) => ({ video_id: id, position_seconds: id === "rel2" ? 500 : 77, })); - const reset = vi.spyOn(api, "clearState").mockResolvedValue(undefined as never); + const reset = vi.spyOn(api, "clearProgress").mockResolvedValue(undefined as never); clock.current = 77; await playRel2(() => Promise.resolve()); clock.current = 500; @@ -1070,7 +1070,7 @@ describe("PlayerModal — the side panel on a wide window", () => { fireEvent.click( screen .queryAllByRole("menuitem", { hidden: true }) - .find((b) => b.textContent === "card.resetState")!, + .find((b) => b.textContent === "card.clearProgress")!, ); await waitFor(() => expect(reset).toHaveBeenCalledWith("rel2")); await settle(); @@ -1079,6 +1079,44 @@ describe("PlayerModal — the side panel on a wide window", () => { expect(lastStart()?.start ?? 0).toBe(0); }); + // The queue was read when the player opened, and a video in it can be in the related list too: a + // step to it after the clear starts at 0, not at that older position, which its first checkpoint + // would then write back over the clear. + it("wide: a queue item cleared from its related row starts at 0 on a step to it", async () => { + wideWindow(true); + vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); + vi.spyOn(api, "related").mockResolvedValue({ + items: [related("rel2", 120)], + has_more: false, + next_cursor: null, + }); + const clear = vi.spyOn(api, "clearProgress").mockResolvedValue(undefined as never); + const A = vid("A", 40); + renderModal( + {}} + onState={() => Promise.resolve()} + />, + ); + act(() => playerEvents.onReady?.()); + fireEvent.click(screen.getByRole("tab", { name: "player.related.title" })); + const links = await screen.findAllByRole("link", { name: "Video rel2" }); + fireEvent.contextMenu(links[0]!.closest("[data-video-id]")!); + fireEvent.click( + screen + .queryAllByRole("menuitem", { hidden: true }) + .find((b) => b.textContent === "card.clearProgress")!, + ); + await waitFor(() => expect(clear).toHaveBeenCalledWith("rel2")); + await settle(); + fireEvent.click(screen.getAllByRole("button", { name: "player.next" })[0]!); + await waitFor(() => expect(lastStart()?.videoId).toBe("rel2")); + expect(lastStart()?.start ?? 0).toBe(0); + }); + // Its row stays in the list while it plays; a new player for the same video is never built (one // per video), so Restart there is a seek. it("wide: Restart on the related video already playing seeks it to 0", async () => { @@ -1702,13 +1740,14 @@ describe("PlayerModal — the docked bar's fullscreen", () => { // The big view covers the card, and its controls stayed in the Tab order under it: past the bar's // last button the focus went on, unseen, to the card's Previous, Download and Remove. describe("PlayerModal — the card under the big view", () => { - const open = () => { + const open = (others: Video[] = []) => { vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); + vi.spyOn(api, "saveProgress").mockResolvedValue({ video_id: "A", position_seconds: 0 }); const A = vid("A", 0); renderModal( {}} onState={() => Promise.resolve()} @@ -1737,16 +1776,68 @@ describe("PlayerModal — the card under the big view", () => { it("does not leave a mouse on the way out, where a Space would leave the big view", () => { const enter = open(); - act(() => enter.focus()); const matches = vi.spyOn(enter, "matches").mockReturnValue(false); - fireEvent.click(enter); + act(() => enter.focus()); matches.mockRestore(); + fireEvent.click(enter); const card = screen.getByRole("dialog").querySelector("[tabindex='-1']"); expect(document.activeElement).toBe(card); fireEvent.click(screen.getByLabelText("player.exitMaximize")); expect(document.activeElement).toBe(card); }); + // Chrome turns a clicked button :focus-visible the moment a key is pressed, so the F that opens the + // big view cannot be what tells a mouse's focus from a key's. + it("keeps a click's focus a mouse's when F opens the view", () => { + open(); + const clicked = screen.getByLabelText("player.markWatched"); + const matches = vi.spyOn(clicked, "matches").mockReturnValue(false); + act(() => clicked.focus()); + matches.mockRestore(); // from here it answers true, as Chrome does after the key + fireEvent.keyDown(window, { key: "f" }); + const card = screen.getByRole("dialog").querySelector("[tabindex='-1']"); + expect(document.activeElement).toBe(card); + }); + + // A step to the queue's end in the big view turned the remembered Next off: it refuses the focus. + it("gives the card the focus when the control it came from has gone off", async () => { + open([vid("B", 0)]); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + const next = screen.getByRole("button", { name: "player.next" }); + act(() => next.focus()); + fireEvent.keyDown(window, { key: "f" }); + expect(document.activeElement).toBe(screen.getByLabelText("player.exitMaximize")); + fireEvent.keyDown(window, { key: "ArrowRight", shiftKey: true }); + await waitFor(() => expect(lastStart()?.videoId).toBe("B")); + fireEvent.keyDown(window, { key: "Escape" }); + expect((next as HTMLButtonElement).disabled).toBe(true); + const card = screen.getByRole("dialog").querySelector("[tabindex='-1']"); + expect(document.activeElement).toBe(card); + }); + + // The headings sit in the part that goes inert, and an inert element names nothing (measured in + // Chromium's accessibility tree; jsdom's would still give the name). + it("names the dialog from outside the part that goes inert", () => { + const enter = open(); + const id = screen.getByRole("dialog").getAttribute("aria-labelledby")!; + fireEvent.click(enter); + const names = document.querySelectorAll(`[id="${id}"]`); + expect(names).toHaveLength(1); + expect(names[0]!.closest("[inert]")).toBeNull(); + expect(names[0]!.textContent).toBe("Video A"); + }); + + // A divider squeezed to nothing where the bar's row overflows and scrolls. + it("keeps the bar's dividers when its row scrolls", () => { + fireEvent.click(open([vid("B", 0)])); + const scroller = screen + .getByRole("button", { name: "player.exitMaximize" }) + .parentElement!.querySelector(".overflow-x-auto")!; + const dividers = scroller.querySelectorAll("span.w-px"); + expect(dividers.length).toBeGreaterThan(1); + for (const d of dividers) expect(d.className).toContain("shrink-0"); + }); + it("does not strand the focus on the page when the bar goes with it", () => { const enter = open(); fireEvent.click(enter); @@ -1864,6 +1955,19 @@ describe("PlayerModal — clip mode", () => { expect(screen.queryByTestId("clip-panel")).toBeNull(); }); + it("leaves the focus in the panel when the view ends, whatever it came from", async () => { + open(); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + act(() => screen.getByLabelText("player.markWatched").focus()); + key("f"); + key("c"); + const panel = screen.getByTestId("clip-panel"); + const inPanel = panel.querySelector("button:not([disabled])")!; + act(() => inPanel.focus()); + key("f"); + expect(document.activeElement).toBe(inPanel); + }); + it("gives the focus back to the scissors the user can see: in the maximised view, the bar's", async () => { open(); await waitFor(() => expect(lastStart()?.videoId).toBe("A")); @@ -1999,8 +2103,10 @@ describe("PlayerModal — stepping over a video that cannot play", () => { for (const b of prevs) { expect((b as HTMLButtonElement).disabled).toBe(false); expect(b.getAttribute("aria-disabled")).toBe("true"); - expect(b.getAttribute("title")).toContain("player.noPlayableBefore"); } + // Where a key's focus can read it: a browser's own title shows for a mouse alone. + act(() => prevs[0]!.focus()); + expect(await screen.findByText(/player\.noPlayableBefore/)).toBeTruthy(); prev(); await settle(); expect(lastStart()?.videoId).toBe("B"); @@ -2009,6 +2115,24 @@ describe("PlayerModal — stepping over a video that cannot play", () => { expect(b.getAttribute("title")).not.toContain("player.noPlayableAfter"); }); + // The Next that the queue's end turned off keeps the focus; a step back gives it a reason, and the + // button that holds the focus must be the same button afterwards. + it("keeps the focus on a control when its reason arrives", async () => { + await open(0); + next(); + await waitFor(() => expect(lastStart()?.videoId).toBe("B")); + const button = screen.getByRole("button", { name: "player.next" }); + act(() => button.focus()); + next(); + await waitFor(() => expect(lastStart()?.videoId).toBe("C")); + fail(); + await settle(); + prev(); + await waitFor(() => expect(lastStart()?.videoId).toBe("B")); + expect(button.getAttribute("aria-disabled")).toBe("true"); + expect(document.activeElement).toBe(button); + }); + it("Previous at the queue's start is plainly off: the counter says why", async () => { await open(0); for (const b of screen.getAllByRole("button", { name: "player.previous" })) { diff --git a/frontend/src/features/player/PlayerModal.tsx b/frontend/src/features/player/PlayerModal.tsx index 8e4ffa8f..824b07db 100644 --- a/frontend/src/features/player/PlayerModal.tsx +++ b/frontend/src/features/player/PlayerModal.tsx @@ -14,7 +14,7 @@ import { useNavigationActions } from "../navigation/navigationContext"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { qk } from "../../lib/queryKeys"; import { usePref } from "../../lib/usePref"; -import { PlayerDescription, PlayerTransportControls } from "./playerModalControls"; +import { PlayerDescription, PlayerTransportControls, ReasonTooltip } from "./playerModalControls"; import { unavailable } from "../../components/ui/unavailable"; import { AUTO_MODES, LOOP_MODES, type AutoMode, type LoopMode } from "../../lib/playerModes"; import { @@ -544,17 +544,20 @@ export default function PlayerModal({ // The big view covers the card, so the card's part with the controls goes inert under it (see the // markup) and the focus cannot stay there. A key's focus moves to the way out and comes back to // the same control when the view ends. A mouse's goes to the card: on the way out, the next Space - // would leave the big view instead of pausing. A focus the view's end took with it (the bar's - // controls go with the bar) goes back to the card. After the clip panel's move above, which keeps - // its own focus. + // would leave the big view instead of pausing. Which it was is read as the focus arrives: Chrome + // calls a clicked button :focus-visible once any key is pressed, the F that opens the view too. + // When the view ends, a focus that has a place of its own stays there (the clip panel's, kept by + // its move above); one with none goes back to the card when the control refuses it (a Next that a + // step to the queue's end turned off). The bar's controls go with the bar. const cardPanelRef = useRef(null); const bigExitRef = useRef(null); const focusBeforeBig = useRef(null); + const panelFocusByKey = useRef(false); useLayoutEffect(() => { if (bigView) { const active = document.activeElement; if (!(active instanceof HTMLElement) || !cardPanelRef.current?.contains(active)) return; - const byKey = active.matches(":focus-visible"); + const byKey = panelFocusByKey.current; focusBeforeBig.current = byKey ? active : null; if (byKey && bigExitRef.current) bigExitRef.current.focus(); else focusModal(); @@ -562,8 +565,9 @@ export default function PlayerModal({ } const back = focusBeforeBig.current; focusBeforeBig.current = null; - if (back?.isConnected) back.focus(); - else reclaimFocus(); + const active = document.activeElement; + if (active === document.body || active === cardRef.current) back?.focus(); + if (document.activeElement !== back) reclaimFocus(); }, [bigView, focusModal, reclaimFocus]); /** Mark where the player is now (`mark` gets the range so far, or null when the panel is shut). The * panel lives under the video in the card, and in place of the docked bar when maximised; the @@ -859,9 +863,9 @@ export default function PlayerModal({ // Turned off on this instance (a daily limit of 0): no tab, no button — nothing that could only // be refused. const relatedOn = useMe().related_enabled; - // A video reset from the related list starts over: the position this session stored for it would + // A video cleared from the related list starts over: the position this session stored for it would // otherwise resume a plain pick there, and the player's close would patch it back into the lists. - const forgetPosition = (id: string) => void sessionPosRef.current.delete(id); + const clearPosition = (id: string) => void sessionPosRef.current.set(id, 0); const playRelated = (v: Video, startAt: number | null) => { setRelatedOpen(false); // The video already playing (its row stays in the list): a new player for the same id would @@ -1124,6 +1128,11 @@ export default function PlayerModal({ aria-modal="true" aria-labelledby={titleId} > + {/* What names the dialog: the headings go inert with the card's panel in the big view, and an + inert element names nothing. Hidden, so it is read as the name and only as that. */} + {/* The card. Previous / next live in the panel (its transport row, or the shut panel's strip) and on Shift+← / →: the full-height strips that once flanked the card cost the video 7rem of width for a second copy of the same two buttons (owner). */} @@ -1261,7 +1270,7 @@ export default function PlayerModal({ onCycleAuto={cycleAuto} onCycleLoop={cycleLoop} /> - + )} {/* Both work in fullscreen now: add-to-playlist's popover and download's Modal both @@ -1383,6 +1392,9 @@ export default function PlayerModal({
{ + panelFocusByKey.current = e.target.matches(":focus-visible"); + }} className={wide ? "relative shrink-0 border-l border-border" : undefined} style={wide ? { width: panelWidth } : undefined} > @@ -1391,10 +1403,7 @@ export default function PlayerModal({ data-testid="player-strip" className="absolute inset-0 flex flex-col items-center gap-1 py-2" > - {/* The dialog is named by this heading's id: the panel's own one is not rendered. */} -

- {heading} -

+

{heading}

{closeButton} - - - - - - + + + + + + + + + + )}
@@ -1474,9 +1487,7 @@ export default function PlayerModal({ {/* Title row — the title and its ⓘ (the description) on the left; stacked, Close on the right (wide, it heads the panel). */}
-

- {heading} -

+

{heading}

{!wide && ( )} @@ -1769,7 +1780,7 @@ export default function PlayerModal({ videoId={active.id} onPlay={playRelated} onWrite={onWrite} - onReset={forgetPosition} + onCleared={clearPosition} />
)} diff --git a/frontend/src/features/player/RelatedVideos.dom.test.tsx b/frontend/src/features/player/RelatedVideos.dom.test.tsx index 62f3c044..c67462f3 100644 --- a/frontend/src/features/player/RelatedVideos.dom.test.tsx +++ b/frontend/src/features/player/RelatedVideos.dom.test.tsx @@ -163,12 +163,12 @@ describe("RelatedVideos", () => { await waitFor(() => expect(titles()).toEqual(["Video A"])); }); - it("resets a started video's view from its menu", async () => { + it("clears a started video's progress from its menu", async () => { vi.spyOn(api, "related").mockResolvedValue(page([row("A", { position_seconds: 95 })])); - const reset = vi.spyOn(api, "clearState").mockResolvedValue(undefined as never); + const reset = vi.spyOn(api, "clearProgress").mockResolvedValue(undefined as never); setup(); fireEvent.contextMenu((await title("Video A")).closest("[data-video-id]")!); - fireEvent.click(menuItem("card.resetState")); + fireEvent.click(menuItem("card.clearProgress")); await waitFor(() => expect(reset).toHaveBeenCalledWith("A")); }); diff --git a/frontend/src/features/player/RelatedVideos.tsx b/frontend/src/features/player/RelatedVideos.tsx index 24dbf9bd..5cd7448b 100644 --- a/frontend/src/features/player/RelatedVideos.tsx +++ b/frontend/src/features/player/RelatedVideos.tsx @@ -25,7 +25,7 @@ import Tooltip from "../../components/ui/Tooltip"; import VideoCard from "../video/VideoCard"; import { VideoMenu } from "../video/VideoMenu"; import { announceVideoState } from "../video/stateNotice"; -import { resetVideoState, writeVideoState, writeWatchLater } from "../video/videoWrites"; +import { clearVideoProgress, writeVideoState, writeWatchLater } from "../video/videoWrites"; import { msg } from "../../lib/notifications"; /** Which list YouTube fills in — on the video's topic or with what trends in Hungary — is a mood, @@ -54,15 +54,15 @@ export function RelatedVideos({ videoId, onPlay, onWrite, - onReset, + onCleared, }: { videoId: string; /** `startAt` null resumes the video where it was left; a number (Restart's 0) starts there. */ onPlay: (v: Video, startAt: number | null) => void; /** Each write this list makes, for the host's player session (see PlayerModal `onWrite`). */ onWrite?: (write: Promise) => void; - /** A video's view was reset from its row: the host forgets where its session left it. */ - onReset?: (id: string) => void; + /** A video's progress was cleared from its row: the host forgets where its session left it. */ + onCleared?: (id: string) => void; }) { const { t } = useTranslation(); const qc = useQueryClient(); @@ -109,10 +109,10 @@ export function RelatedVideos({ }); }; - const onResetState = (id: string) => { - onReset?.(id); - const seq = begin(id, { status: "new", position_seconds: 0 }); - const write = resetVideoState(qc, id); + const onClearProgress = (id: string) => { + onCleared?.(id); + const seq = begin(id, { position_seconds: 0 }); + const write = clearVideoProgress(qc, id); onWrite?.(write); write .then(() => { @@ -120,7 +120,7 @@ export function RelatedVideos({ void qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" }); }) .catch((e: unknown) => { - notifyActionError(e, msg("errors.writeFailed.videoReset")); + notifyActionError(e, msg("errors.writeFailed.videoProgress")); settle(seq); }); }; @@ -250,7 +250,7 @@ export function RelatedVideos({ )} )} - + ); } diff --git a/frontend/src/features/player/playerModalControls.tsx b/frontend/src/features/player/playerModalControls.tsx index 0134f50d..16f39214 100644 --- a/frontend/src/features/player/playerModalControls.tsx +++ b/frontend/src/features/player/playerModalControls.tsx @@ -4,6 +4,7 @@ import { ListEnd, ListStart, Repeat, Repeat1, Shuffle, SkipBack, SkipForward } f import type { VideoDetail } from "../../lib/api"; import type { AutoMode, LoopMode } from "../../lib/playerModes"; import { IconButton } from "../../components/ui/IconButton"; +import Tooltip from "../../components/ui/Tooltip"; import { StateMessage } from "../../components/ui/QueryState"; import { unavailable } from "../../components/ui/unavailable"; import { ClampedText } from "../../components/ui/ClampedText"; @@ -13,6 +14,25 @@ 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`. +/** Why a control is off, where a key's focus reads it too: a browser's own tooltip (`title`, left on + * the button for what it does) shows for a mouse alone. `stable`: the reason comes and goes under a + * control that may hold the focus. */ +export function ReasonTooltip({ + reason, + side, + children, +}: { + reason: string | undefined; + side?: "left"; + children: ReactNode; +}) { + return ( + + {children} + + ); +} + /** 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) and the big-view docked control bar (`variant:"docked"`, white-on-dark). Glyphs only @@ -60,7 +80,7 @@ export function PlayerTransportControls({ const docked = variant === "docked"; // Never broken or squeezed: in a row that scrolls, a squeezed "1 / 55" stood on three lines. const counter = `shrink-0 whitespace-nowrap tabular-nums ${docked ? "text-white/70" : "text-muted"}`; - const divider = docked ? "h-4 w-px bg-white/20" : "w-px h-4 bg-border"; + const divider = `h-4 w-px shrink-0 ${docked ? "bg-white/20" : "bg-border"}`; const autoName = `${t("player.autoAdvance.label")}: ${t(`player.autoAdvance.${autoMode}`)}`; const loopName = `${t("player.loop.label")}: ${t(`player.loop.${loopMode}`)}`; type Control = { @@ -114,17 +134,18 @@ export function PlayerTransportControls({ }, ]; const render = (c: Control) => ( - - {c.glyph} - + + + {c.glyph} + + ); return ( <> diff --git a/frontend/src/features/video/VideoCard.actions.dom.test.tsx b/frontend/src/features/video/VideoCard.actions.dom.test.tsx index 08fc5393..9a128b96 100644 --- a/frontend/src/features/video/VideoCard.actions.dom.test.tsx +++ b/frontend/src/features/video/VideoCard.actions.dom.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment jsdom // // The card shows its state and the everyday toggles (watched, save, add to playlist) and a -// "…" into the list's ONE menu; download, details, hide and reset live only in that menu. Every view +// "…" into the list's ONE menu; download, details, hide and clear progress live only in that menu. Every view // carries the same set, and a view that shows the "…" twice (the card: in the hover row on the // picture, and beside the title for touch — only one is displayed per device) points both at the // same menu. @@ -77,7 +77,7 @@ describe.each(Object.keys(FEED_VIEW_SPEC) as FeedView[])("the %s view's actions" for (const want of ["card.markWatched", "card.saveForLater", "playlists.addToPlaylist"]) expect(all).toContain(want); expect(all).toContain("card.menu.more"); - for (const gone of ["downloads.button.label", "card.hide", "card.resetState", "card.info"]) + for (const gone of ["downloads.button.label", "card.hide", "card.clearProgress", "card.info"]) expect(all).not.toContain(gone); }); diff --git a/frontend/src/features/video/VideoCard.tsx b/frontend/src/features/video/VideoCard.tsx index 6a892e30..eafa32d6 100644 --- a/frontend/src/features/video/VideoCard.tsx +++ b/frontend/src/features/video/VideoCard.tsx @@ -49,7 +49,7 @@ import RemoteImage from "../../components/ui/RemoteImage"; type More = { props: Record; open: boolean }; // The card keeps its state and the everyday toggles — watched, save, add to playlist — and -// a "…" for everything else (download, details, copy, hide, reset, pin), which is the list's ONE +// a "…" for everything else (download, details, copy, hide, clear progress, pin), which is the list's ONE // menu (useVideoActions). On a mouse the row shows on hover or keyboard focus; on touch the toggles // are gone (the "…" and the long-press are the way in) and only the "…" stays. /** The card's hover row (watched, save, add, "…"); Home's hero docks the same row on its picture. */ diff --git a/frontend/src/features/video/VideoMenu.dom.test.tsx b/frontend/src/features/video/VideoMenu.dom.test.tsx index 99dd2ac8..ca75da34 100644 --- a/frontend/src/features/video/VideoMenu.dom.test.tsx +++ b/frontend/src/features/video/VideoMenu.dom.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment jsdom // // A video's ONE action list. The right-click and the card's "…" open the same menu, which -// holds everything the card no longer shows (download, details, copy, hide, reset, pin) beside the +// holds everything the card no longer shows (download, details, copy, hide, clear progress, pin) beside the // everyday toggles, and each row fires the same handler the card's own buttons do. import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { cleanup, fireEvent, render, screen } from "@testing-library/react"; @@ -102,18 +102,18 @@ function setup(v: Video) { const handlers = { onState: vi.fn(), onToggleSave: vi.fn(), - onResetState: vi.fn(), + onClearProgress: vi.fn(), onOpen: vi.fn(), }; const qc = new QueryClient(); - render( + const { unmount } = render( , ); - return { handlers }; + return { handlers, unmount }; } const rightClick = () => fireEvent.contextMenu(screen.getByText("card")); const more = () => screen.getByRole("button", { name: "more" }); @@ -137,16 +137,29 @@ describe("a video's menu", () => { ]); }); - it("offers Continue and the restart on a half-watched one, the way back and the reset", async () => { + it("offers Continue and the restart on a half-watched one, the way back and the clearing", async () => { const { handlers } = setup(video({ position_seconds: 30, saved: true })); rightClick(); expect(labels()).toContain("card.restartTitle"); expect(labels()).toContain("card.menu.unsave"); - expect(labels()).toContain("card.resetState"); + expect(labels()).toContain("card.clearProgress"); fireEvent.click(item("card.restartTitle")); await vi.waitFor(() => expect(handlers.onOpen).toHaveBeenLastCalledWith(expect.anything(), 0)); }); + // It clears a resume point and nothing else: with none, there is nothing for it to do. + it("offers clearing only where there is a resume point, whatever the status", () => { + for (const status of ["hidden", "watched"] as const) { + const { unmount } = setup(video({ status })); + rightClick(); + expect(labels()).not.toContain("card.clearProgress"); + unmount(); + } + setup(video({ status: "hidden", position_seconds: 30 })); + rightClick(); + expect(labels()).toContain("card.clearProgress"); + }); + it("each toggle fires the card's own handler", async () => { const { handlers } = setup(video({ status: "watched" })); rightClick(); diff --git a/frontend/src/features/video/useVideoActions.dom.test.tsx b/frontend/src/features/video/useVideoActions.dom.test.tsx index 1f72e517..f1ca38ec 100644 --- a/frontend/src/features/video/useVideoActions.dom.test.tsx +++ b/frontend/src/features/video/useVideoActions.dom.test.tsx @@ -39,7 +39,8 @@ function downloadRow(state: string | undefined) { {children} ); const { result } = renderHook( - () => useVideoActions({ onState: vi.fn(), onToggleSave: vi.fn(), onResetState: vi.fn() }, true), + () => + useVideoActions({ onState: vi.fn(), onToggleSave: vi.fn(), onClearProgress: vi.fn() }, true), { wrapper }, ); return () => result.current.items(video, null).find((i) => i.id === "download")!; @@ -74,7 +75,7 @@ describe("the video menu's channel-filter row", () => { { onState: vi.fn(), onToggleSave: vi.fn(), - onResetState: vi.fn(), + onClearProgress: vi.fn(), channelFilter: ids ? { ids, set } : undefined, }, true, diff --git a/frontend/src/features/video/useVideoActions.tsx b/frontend/src/features/video/useVideoActions.tsx index bd5e3b08..72517f6e 100644 --- a/frontend/src/features/video/useVideoActions.tsx +++ b/frontend/src/features/video/useVideoActions.tsx @@ -20,7 +20,7 @@ import { } from "lucide-react"; import { api, type Video, type VideoStatus } from "../../lib/api"; -import { isResettable } from "../../lib/api/feed"; +import { hasResumePoint } from "../../lib/api/feed"; import { qk } from "../../lib/queryKeys"; import { pinVideo, useComfortPin } from "../comfort/comfortPin"; import { useCopyItems } from "../../components/ui/copyItems"; @@ -33,13 +33,13 @@ import { raw } from "../../lib/notifications"; // long-press, Shift+F10 and the card's "…" all open a menu built here, so the feed, a channel page // and the Home shelves list the same actions in the same order. The card itself keeps only its // state and the everyday toggles (watched, save, add to playlist) on hover; everything else — -// download, details, copy, hide, reset, pin — lives here, and only here. +// download, details, copy, hide, clear progress, pin — lives here, and only here. export interface VideoActionHandlers { onState: (id: string, status: VideoStatus) => void; onToggleSave: (id: string, saved: boolean) => void; /** Back to fresh: its state row and resume point both go. */ - onResetState: (id: string) => void; + onClearProgress: (id: string) => void; /** Omitted where the host menu leads with its own Play row (Home's shelf card resolves `next`). */ onOpen?: (v: Video, startAt?: number | null) => void; /** The feed's channel filter, where the menu can narrow it: the library feed. Omitted on a channel @@ -71,7 +71,7 @@ export function useVideoActions(handlers: VideoActionHandlers, open: boolean) { anchor: Element | null, { pin: withPin = true }: { pin?: boolean } = {}, ): ActionMenuItem[] => { - const { onState, onToggleSave, onResetState, onOpen, channelFilter } = handlers; + const { onState, onToggleSave, onClearProgress, onOpen, channelFilter } = handlers; const watched = v.status === "watched"; const hidden = v.status === "hidden"; const inProgress = v.position_seconds > 0 && !watched; @@ -189,13 +189,13 @@ export function useVideoActions(handlers: VideoActionHandlers, open: boolean) { separatorBefore: true, onSelect: () => onState(v.id, hidden ? "new" : "hidden"), }, - ...(isResettable(v) + ...(hasResumePoint(v) ? [ { - id: "reset", - label: t("card.resetState"), + id: "clearProgress", + label: t("card.clearProgress"), icon: , - onSelect: () => onResetState(v.id), + onSelect: () => onClearProgress(v.id), }, ] : []), diff --git a/frontend/src/features/video/videoWrites.contract.test.ts b/frontend/src/features/video/videoWrites.contract.test.ts index 0a29000e..252bf91b 100644 --- a/frontend/src/features/video/videoWrites.contract.test.ts +++ b/frontend/src/features/video/videoWrites.contract.test.ts @@ -21,7 +21,7 @@ const ALLOWED = new Set([ // 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/; + /\bapi\s*\.\s*(setState|clearProgress|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", () => { diff --git a/frontend/src/features/video/videoWrites.dom.test.ts b/frontend/src/features/video/videoWrites.dom.test.ts index 5aa52125..aa10f1ef 100644 --- a/frontend/src/features/video/videoWrites.dom.test.ts +++ b/frontend/src/features/video/videoWrites.dom.test.ts @@ -11,7 +11,7 @@ import { addPlaylistItem, movePlaylistItem, removePlaylistItem, - resetVideoState, + clearVideoProgress, writeVideoState, writeWatchLater, } from "./videoWrites"; @@ -41,11 +41,11 @@ describe("videoWrites", () => { expect(marks(qc)).toEqual({ X: { status: "hidden" } }); }); - it("marks a reset as new, from the start", async () => { - vi.spyOn(api, "clearState").mockResolvedValue({ ok: true } as never); + it("marks a cleared video as back at the start, its status as it was", async () => { + vi.spyOn(api, "clearProgress").mockResolvedValue({ ok: true } as never); const qc = new QueryClient(); - await resetVideoState(qc, "X"); - expect(marks(qc)).toEqual({ X: { status: "new", position_seconds: 0 } }); + await clearVideoProgress(qc, "X"); + expect(marks(qc)).toEqual({ X: { position_seconds: 0 } }); }); it("marks a Watch-later save and its removal", async () => { diff --git a/frontend/src/features/video/videoWrites.ts b/frontend/src/features/video/videoWrites.ts index 38fa22f0..4f928095 100644 --- a/frontend/src/features/video/videoWrites.ts +++ b/frontend/src/features/video/videoWrites.ts @@ -7,7 +7,7 @@ import { markVideo } from "../../lib/progressCache"; // 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 +// Notification Center's undo, the feed's clear, 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 @@ -20,10 +20,10 @@ export function writeVideoState(qc: QueryClient, id: string, status: VideoStatus }); } -/** Back to pristine: the state row and its resume position both go. */ -export function resetVideoState(qc: QueryClient, id: string) { - return api.clearState(id).then((r) => { - markVideo(qc, id, { status: "new", position_seconds: 0 }); +/** Forget where the user left it: the resume point goes, the status stays. */ +export function clearVideoProgress(qc: QueryClient, id: string) { + return api.clearProgress(id).then((r) => { + markVideo(qc, id, { position_seconds: 0 }); return r; }); } diff --git a/frontend/src/i18n/locales/en/card.json b/frontend/src/i18n/locales/en/card.json index 47ef1ec8..e566dbed 100644 --- a/frontend/src/i18n/locales/en/card.json +++ b/frontend/src/i18n/locales/en/card.json @@ -8,7 +8,7 @@ "saveForLater": "Save for later", "unhide": "Unhide", "hide": "Hide", - "resetState": "Clear watch progress", + "clearProgress": "Clear watch progress", "thisChannel": "This channel", "continueTitle": "Continue where you left off", "continue": "Continue", diff --git a/frontend/src/i18n/locales/en/errors.json b/frontend/src/i18n/locales/en/errors.json index e9e86c90..f4f8cbfe 100644 --- a/frontend/src/i18n/locales/en/errors.json +++ b/frontend/src/i18n/locales/en/errors.json @@ -96,7 +96,7 @@ "prefs": "Couldn't save your settings", "deleteAccount": "Couldn't delete your account", "videoState": "Couldn't change the video's state", - "videoReset": "Couldn't reset the video", + "videoProgress": "Couldn't clear the video's watch progress", "watchLaterAdd": "Couldn't add it to Watch later", "watchLaterRemove": "Couldn't remove it from Watch later", "clearSearch": "Couldn't clear the search results", diff --git a/frontend/src/i18n/locales/hu/card.json b/frontend/src/i18n/locales/hu/card.json index ec6f0092..80d8a9b8 100644 --- a/frontend/src/i18n/locales/hu/card.json +++ b/frontend/src/i18n/locales/hu/card.json @@ -8,7 +8,7 @@ "saveForLater": "Mentés későbbre", "unhide": "Megjelenítés", "hide": "Elrejtés", - "resetState": "Megtekintés visszaállítása", + "clearProgress": "Megtekintés visszaállítása", "thisChannel": "Ez a csatorna", "continueTitle": "Folytatás ott, ahol abbahagytad", "continue": "Folytatás", diff --git a/frontend/src/i18n/locales/hu/errors.json b/frontend/src/i18n/locales/hu/errors.json index d09b4406..299bf3ca 100644 --- a/frontend/src/i18n/locales/hu/errors.json +++ b/frontend/src/i18n/locales/hu/errors.json @@ -96,7 +96,7 @@ "prefs": "Nem sikerült menteni a beállításaidat", "deleteAccount": "Nem sikerült törölni a fiókodat", "videoState": "Nem sikerült módosítani a videó állapotát", - "videoReset": "Nem sikerült alaphelyzetbe állítani a videót", + "videoProgress": "Nem sikerült visszaállítani a videó megtekintését", "watchLaterAdd": "Nem sikerült hozzáadni a Megnézem később listához", "watchLaterRemove": "Nem sikerült eltávolítani a Megnézem később listából", "clearSearch": "Nem sikerült törölni a keresési találatokat", diff --git a/frontend/src/lib/api/feed.ts b/frontend/src/lib/api/feed.ts index 53625713..d6ba1f82 100644 --- a/frontend/src/lib/api/feed.ts +++ b/frontend/src/lib/api/feed.ts @@ -29,10 +29,10 @@ export interface Video { watch_url: string; } -/** Whether a video has anything its reset would clear: pristine = never opened, the default status - * and no resume position. The card's reset button and the card menu's reset both ask this. */ -export function isResettable(v: Pick): boolean { - return v.status !== "new" || v.position_seconds > 0; +/** Whether a video has a resume point, the one thing "Clear watch progress" clears. Its status is no + * part of it: a hidden video stays hidden, a watched one watched. */ +export function hasResumePoint(v: Pick): boolean { + return v.position_seconds > 0; } /** A video's related videos, in the locale YouTube fills the list in: on the video's topic, or with @@ -290,8 +290,8 @@ export const feedApi = { { method: "POST", body: JSON.stringify({ status }) }, { idempotent: true }, ), - clearState: (id: string) => - req(`/api/videos/${id}/state`, { method: "DELETE" }, { idempotent: true }), + clearProgress: (id: string) => + req(`/api/videos/${id}/progress`, { method: "DELETE" }, { idempotent: true }), // Returns the position the server actually STORED — which is not always what we sent: a // trivially-early or near-finished checkpoint is cleared to 0 (see set_video_progress). Callers // that mirror the position into a cache must write this answer, never their own reading, so the diff --git a/frontend/src/lib/menuGlyphs.contract.test.ts b/frontend/src/lib/menuGlyphs.contract.test.ts index 8476dd96..33fbaed9 100644 --- a/frontend/src/lib/menuGlyphs.contract.test.ts +++ b/frontend/src/lib/menuGlyphs.contract.test.ts @@ -7,7 +7,7 @@ import { describe, expect, it } from "vitest"; // Redownload. Each row is a `label: t("…")` followed by its `icon`. const ROW_GLYPHS: Record = { "card.restartTitle": "RotateCcw", - "card.resetState": "Eraser", + "card.clearProgress": "Eraser", "files.actions.redownload": "RefreshCw", }; diff --git a/frontend/src/modules/feed/Feed.reset.dom.test.tsx b/frontend/src/modules/feed/Feed.clearProgress.dom.test.tsx similarity index 72% rename from frontend/src/modules/feed/Feed.reset.dom.test.tsx rename to frontend/src/modules/feed/Feed.clearProgress.dom.test.tsx index a506acc1..bea48773 100644 --- a/frontend/src/modules/feed/Feed.reset.dom.test.tsx +++ b/frontend/src/modules/feed/Feed.clearProgress.dom.test.tsx @@ -18,16 +18,16 @@ vi.mock("../../features/wizard/wizardContext", () => ({ vi.mock("./VirtualFeed", () => ({ default: ({ items, - onResetState, + onClearProgress, }: { items: { id: string; position_seconds: number }[]; - onResetState: (id: string) => void; + onClearProgress: (id: string) => void; }) => (
    {items.map((v) => (
  1. {v.id}@{v.position_seconds} - +
    2. ))}
@@ -81,16 +81,16 @@ const halfWatched = { watch_url: "https://www.youtube.com/watch?v=v1", } as unknown as Video; -function mount(clear: () => Promise) { - vi.spyOn(api, "feed").mockResolvedValue({ items: [halfWatched], next_cursor: null } as never); +function mount(clear: () => Promise, video = halfWatched, filters = FILTERS) { + vi.spyOn(api, "feed").mockResolvedValue({ items: [video], next_cursor: null } as never); vi.spyOn(api, "feedSummary").mockResolvedValue({ count: 1, facets: {} } as never); - vi.spyOn(api, "clearState").mockImplementation(clear as never); + vi.spyOn(api, "clearProgress").mockImplementation(clear as never); const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); qc.setQueryData(qk.me(), { id: 1, can_write: true, prefs: {} }); render( {}} view="cards" setView={() => {}} @@ -104,29 +104,41 @@ function mount(clear: () => Promise) { return qc; } -describe("Feed — resetting a half-watched video", () => { - it("clears its resume position at once, not only its status", async () => { +describe("Feed — clearing a half-watched video's progress", () => { + it("leaves its status alone: a hidden video stays in the Hidden view", async () => { + mount( + () => Promise.resolve({ ok: true }), + { ...halfWatched, status: "hidden" }, + { ...FILTERS, show: "hidden" }, + ); + fireEvent.click(await screen.findByText("clear v1")); + await screen.findByText("v1@0"); + await act(() => new Promise((r) => setTimeout(r, 20))); + expect(screen.getByText("v1@0")).toBeTruthy(); + }); + + it("clears its resume position at once", async () => { // Never settles: what the card shows before the server answers is the optimistic layer alone. mount(() => new Promise(() => {})); await waitFor(() => expect(screen.getByText("v1@95")).toBeTruthy()); - fireEvent.click(screen.getByText("reset v1")); + fireEvent.click(screen.getByText("clear v1")); await waitFor(() => expect(screen.getByText("v1@0")).toBeTruthy()); }); it("shows the position a replay after it saved, so a reopen resumes there", async () => { - // Reset, played to 40 s and closed: the player writes what the server stored into the cache. - // A reset mask over the cache kept showing 0, a reopen resumed at 0, and its first checkpoint + // Cleared, played to 40 s and closed: the player writes what the server stored into the cache. + // A mask over the cache kept showing 0, a reopen resumed at 0, and its first checkpoint // wrote over the 40. const qc = mount(() => Promise.resolve({ ok: true })); - fireEvent.click(await screen.findByText("reset v1")); + fireEvent.click(await screen.findByText("clear v1")); await screen.findByText("v1@0"); act(() => patchCachedProgress(qc, new Map([["v1", 40]]))); await waitFor(() => expect(screen.getByText("v1@40")).toBeTruthy()); }); - it("keeps the position at 0 through a refetch that set out before the reset", async () => { - // That refetch read the row the reset deletes: its answer put the old position back under a - // card the reset had made new, and a reopen resumed there. + it("keeps the position at 0 through a refetch that set out before the clear", async () => { + // That refetch read the row before the clear: its answer put the old position back under a + // card the clear had reset, and a reopen resumed there. let answer: (v: unknown) => void = () => {}; const qc = mount(() => new Promise(() => {})); await screen.findByText("v1@95"); @@ -134,16 +146,16 @@ describe("Feed — resetting a half-watched video", () => { act(() => void qc.invalidateQueries({ queryKey: qk.feed() })); await waitFor(() => expect(api.feed).toHaveBeenCalledTimes(2)); await act(() => new Promise((r) => setTimeout(r, 20))); - fireEvent.click(screen.getByText("reset v1")); + fireEvent.click(screen.getByText("clear v1")); await screen.findByText("v1@0"); await act(async () => answer({ items: [halfWatched], next_cursor: null })); await act(() => new Promise((r) => setTimeout(r, 20))); expect(screen.getByText("v1@0")).toBeTruthy(); }); - it("puts the position back when the server refuses the reset", async () => { + it("puts the position back when the server refuses the clear", async () => { mount(() => Promise.reject(new Error("down"))); - fireEvent.click(await screen.findByText("reset v1")); + fireEvent.click(await screen.findByText("clear v1")); await waitFor(() => expect(screen.getByText("v1@95")).toBeTruthy()); }); }); diff --git a/frontend/src/modules/feed/Feed.tsx b/frontend/src/modules/feed/Feed.tsx index 1072a9b2..53ab5d8c 100644 --- a/frontend/src/modules/feed/Feed.tsx +++ b/frontend/src/modules/feed/Feed.tsx @@ -37,7 +37,7 @@ import { import { notifyActionError } from "../../lib/actionErrors"; import { announceVideoState, type NoticeVideo } from "../../features/video/stateNotice"; import { - resetVideoState, + clearVideoProgress, writeVideoState, writeWatchLater, } from "../../features/video/videoWrites"; @@ -210,9 +210,9 @@ export default function Feed({ const writeSeq = useRef(0); const answered = useRef(new Set()); const answeredAtRefetch = useRef>(new Set()); - // A reset's position 0 lives in the cached lists (see onResetState), where a refetch that set out - // before the server took the reset puts the old position back: written again on its landing. - const resets = useRef(new Map()); + // A cleared position's 0 lives in the cached lists (see onClearProgress), where a refetch that set + // out before the server took the clear puts the old position back: written again on its landing. + const clears = useRef(new Map()); // The open player: which video and where to start (null = resume from saved position). `alone`: // opened from an address naming a video this list has not loaded, so it plays without the queue. const [activeVideo, setActiveVideo] = useState<{ @@ -237,8 +237,8 @@ export default function Feed({ }); const openVideo = useCallback((video: Video, startAt: number | null = null) => { playerSession.current.open = true; - // Its player writes the next position: a reset no longer speaks for it. - resets.current.delete(video.id); + // Its player writes the next position: a clear no longer speaks for it. + clears.current.delete(video.id); setActiveVideo({ video, startAt }); }, []); // Stable identity, like Playlists' `closePlayer`: PlayerModal's keydown effect @@ -356,8 +356,8 @@ export default function Feed({ setSavedOverrides((o) => unconfirmed(o, known)); for (const seq of known) answered.current.delete(seq); const zero = new Map(); - for (const [id, seq] of resets.current) { - if (known.has(seq)) resets.current.delete(id); + for (const [id, seq] of clears.current) { + if (known.has(seq)) clears.current.delete(id); else zero.set(id, 0); } patchCachedProgress(qc, zero); @@ -429,30 +429,26 @@ export default function Feed({ [qc, trackSessionWrite], ); - const onResetState = useCallback( + const onClearProgress = useCallback( (id: string) => { - // Reset to pristine (drop the whole VideoState row, incl. resume position) — the - // un-watch toggle can't clear an in-progress position, this can. + // The resume point alone: the status stays, so no status override. const seq = ++writeSeq.current; - setOverrides((o) => ({ ...o, [id]: { value: "new", seq } })); // The position goes into the cached lists, where the player's own closing patch writes the // next one: as a mask over them it outlived a replay, so a reopen resumed at 0 and its first // checkpoint wrote over the progress just made. const before = loadedRef.current.find((v) => v.id === id)?.position_seconds; patchCachedProgress(qc, new Map([[id, 0]])); - resets.current.set(id, seq); - resetVideoState(qc, id) - // Stale-mark only (see setState above): the optimistic override already reset the card. + clears.current.set(id, seq); + clearVideoProgress(qc, id) + // Stale-mark only (see setState above): the patch above already cleared the card. .then(() => { answered.current.add(seq); - setOverrides((o) => withAnswer(o, id, seq)); qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" }); }) .catch((e: unknown) => { - notifyActionError(e, msg("errors.writeFailed.videoReset")); - setOverrides((o) => withoutWrite(o, id, seq)); - if (resets.current.get(id) !== seq) return; - resets.current.delete(id); + notifyActionError(e, msg("errors.writeFailed.videoProgress")); + if (clears.current.get(id) !== seq) return; + clears.current.delete(id); if (before !== undefined) patchCachedProgress(qc, new Map([[id, before]])); }); }, @@ -666,7 +662,7 @@ export default function Feed({ view={view} onState={onState} onToggleSave={onToggleSave} - onResetState={onResetState} + onClearProgress={onClearProgress} onOpen={openVideo} hasNextPage={false} isFetching={false} @@ -1004,7 +1000,7 @@ export default function Feed({ view={view} onState={onState} onToggleSave={onToggleSave} - onResetState={onResetState} + onClearProgress={onClearProgress} onOpen={openVideo} // A channel page's view is the channel already: no row there to narrow it to one. channelFilter={ diff --git a/frontend/src/modules/feed/VirtualFeed.tsx b/frontend/src/modules/feed/VirtualFeed.tsx index 42a4ae38..c50b07c1 100644 --- a/frontend/src/modules/feed/VirtualFeed.tsx +++ b/frontend/src/modules/feed/VirtualFeed.tsx @@ -27,7 +27,7 @@ export interface VirtualFeedProps { view: FeedView; onState: (id: string, status: VideoStatus) => void; onToggleSave: (id: string, saved: boolean) => void; - onResetState: (id: string) => void; + onClearProgress: (id: string) => void; onOpen: (v: Video, startAt?: number | null) => void; /** The card menu's channel-filter row (see `useVideoActions`); omitted where it has no place. */ channelFilter?: VideoActionHandlers["channelFilter"]; @@ -44,7 +44,7 @@ export default function VirtualFeed({ view, onState, onToggleSave, - onResetState, + onClearProgress, onOpen, channelFilter, hasNextPage, @@ -92,7 +92,7 @@ export default function VirtualFeed({ /> ); diff --git a/frontend/src/modules/home/Home.dom.test.tsx b/frontend/src/modules/home/Home.dom.test.tsx index ef674864..8f4fbf2b 100644 --- a/frontend/src/modules/home/Home.dom.test.tsx +++ b/frontend/src/modules/home/Home.dom.test.tsx @@ -46,14 +46,14 @@ vi.mock("react-i18next", async (importOriginal) => { const discover = vi.fn(); const discoverPick = vi.fn(); const videoRow = vi.fn(); -const clearState = vi.fn((_id: string) => Promise.resolve()); +const clearProgress = vi.fn((_id: string) => Promise.resolve()); vi.mock("../../lib/api", () => ({ api: { discover: (...a: unknown[]) => discover(...a), videoRow: (...a: unknown[]) => videoRow(...a), discoverPick: (...a: unknown[]) => discoverPick(...a), setState: () => Promise.resolve(), - clearState: (id: string) => clearState(id), + clearProgress: (id: string) => clearProgress(id), watchLaterAdd: () => Promise.resolve(), watchLaterRemove: () => Promise.resolve(), downloadIndex: () => Promise.resolve({}), // the cards' download buttons read it @@ -816,11 +816,11 @@ describe("Home", () => { fireEvent.click((await screen.findAllByRole("button", { name: "card.menu.more" }))[0]!); const reset = screen .getAllByRole("menuitem", { hidden: true }) - .find((b) => b.textContent === "card.resetState"); + .find((b) => b.textContent === "card.clearProgress"); expect(reset).toBeTruthy(); const asked = discover.mock.calls.length; fireEvent.click(reset!); - await waitFor(() => expect(clearState).toHaveBeenCalledWith("v1")); + await waitFor(() => expect(clearProgress).toHaveBeenCalledWith("v1")); await waitFor(() => expect(discover.mock.calls.length).toBeGreaterThan(asked)); }); diff --git a/frontend/src/modules/home/Home.tsx b/frontend/src/modules/home/Home.tsx index b1652c42..c2e80e61 100644 --- a/frontend/src/modules/home/Home.tsx +++ b/frontend/src/modules/home/Home.tsx @@ -53,7 +53,7 @@ import { LoadingState, StateMessage } from "../../components/ui/QueryState"; import VideoCard, { VideoHoverActions } from "../../features/video/VideoCard"; import { useVideoActions } from "../../features/video/useVideoActions"; import { - resetVideoState, + clearVideoProgress, writeVideoState, writeWatchLater, } from "../../features/video/videoWrites"; @@ -161,14 +161,14 @@ function useHomeCardMenu({ // A video tile's own action list, the same one the feed's menu shows; the rail keeps its // Play row (it resolves `next`) and its shelf rows around it. const qc = useQueryClient(); - // Back to fresh: what the rails answer changes with it (a reset video leaves Continue watching), + // What the rails answer changes with it (a video with no resume point leaves Continue watching), // so Home asks again, as its marks do. - const onResetState = (id: string) => { - void resetVideoState(qc, id) + const onClearProgress = (id: string) => { + void clearVideoProgress(qc, id) .then(() => qc.invalidateQueries({ queryKey: qk.discover() })) - .catch((e: unknown) => notifyActionError(e, msg("errors.writeFailed.videoReset"))); + .catch((e: unknown) => notifyActionError(e, msg("errors.writeFailed.videoProgress"))); }; - const videoActions = useVideoActions({ onState, onToggleSave, onResetState }, menu.open); + const videoActions = useVideoActions({ onState, onToggleSave, onClearProgress }, menu.open); // A Plex tile's, likewise (its "Media info" row goes where the tile's `onInfo` does). const plexActions = usePlexCardActions( { diff --git a/frontend/src/modules/playlists/Playlists.dom.test.tsx b/frontend/src/modules/playlists/Playlists.dom.test.tsx index ed9d4732..746b531b 100644 --- a/frontend/src/modules/playlists/Playlists.dom.test.tsx +++ b/frontend/src/modules/playlists/Playlists.dom.test.tsx @@ -570,14 +570,14 @@ async function rowMenu(title: string, item: string) { } describe("Playlists — a row's watch state", () => { - it("resets a started video's view from its menu", async () => { - vi.spyOn(api, "clearState").mockResolvedValue(undefined as never); + it("clears a started video's progress from its menu", async () => { + vi.spyOn(api, "clearProgress").mockResolvedValue(undefined as never); mount({ 1: [vid("A", 0), vid("B", 300)] }); - await rowMenu("Video B", "card.resetState"); - await waitFor(() => expect(api.clearState).toHaveBeenCalledWith("B")); + await rowMenu("Video B", "card.clearProgress"); + await waitFor(() => expect(api.clearProgress).toHaveBeenCalledWith("B")); }); - it("offers no reset on a video never started", async () => { + it("offers no clearing on a video never started", async () => { mount({ 1: [vid("A", 0)] }); const row = (await screen.findByRole("button", { name: "card.play — Video A" })).closest("tr")!; fireEvent.contextMenu(row); @@ -587,7 +587,7 @@ describe("Playlists — a row's watch state", () => { expect( screen .getAllByRole("menuitem", { hidden: true }) - .some((b) => b.textContent === "card.resetState"), + .some((b) => b.textContent === "card.clearProgress"), ).toBe(false); }); }); diff --git a/frontend/src/modules/playlists/Playlists.tsx b/frontend/src/modules/playlists/Playlists.tsx index fb4f4c9f..133aab48 100644 --- a/frontend/src/modules/playlists/Playlists.tsx +++ b/frontend/src/modules/playlists/Playlists.tsx @@ -3,12 +3,12 @@ import { useLatest, useStableCallback } from "../../lib/useLatest"; import { useTranslation } from "react-i18next"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { qk } from "../../lib/queryKeys"; -import { isResettable } from "../../lib/api/feed"; +import { hasResumePoint } from "../../lib/api/feed"; import { addPlaylistItem, movePlaylistItem, removePlaylistItem, - resetVideoState, + clearVideoProgress, writeVideoState, } from "../../features/video/videoWrites"; import { @@ -685,14 +685,14 @@ function PlaylistDetail() { return write; } - // Back to fresh (the video's state row and resume point both go), from the row's menu. - function resetRow(id: string) { - resetVideoState(qc, id) + // The video's resume point goes (its status stays), from the row's menu. + function clearRowProgress(id: string) { + clearVideoProgress(qc, id) .then(() => { qc.invalidateQueries({ queryKey: qk.playlist() }); qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" }); }) - .catch((e: unknown) => notifyActionError(e, msg("errors.writeFailed.videoReset"))); + .catch((e: unknown) => notifyActionError(e, msg("errors.writeFailed.videoProgress"))); } // Stable identity: PlayerModal's keydown effect depends on it, so a fresh closure @@ -744,13 +744,13 @@ function PlaylistDetail() { onSelect: () => openPlayer(ctxRow.id), }, ...comfortPin.item(pinVideo(ctxRow.id), raw(ctxRow.title ?? "")), - ...(isResettable(ctxRow) + ...(hasResumePoint(ctxRow) ? [ { - id: "reset", - label: t("card.resetState"), + id: "clearProgress", + label: t("card.clearProgress"), icon: , - onSelect: () => resetRow(ctxRow.id), + onSelect: () => clearRowProgress(ctxRow.id), }, ] : []),