diff --git a/CHANGELOG.md b/CHANGELOG.md index 85cd6411..76a1e8f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ edit the release notes and re-run the generator. Upgrading, pinning a version and rolling back: [docs/self-hosting.md](docs/self-hosting.md#upgrading). +## 0.115.0 — 2026-11-01 + +### New + +- Clear watch progress is on every YouTube video's menu, on Home, in playlists and in the player's related videos. It forgets where you left a started video and changes nothing else. +- Channels have an Added column, and an About card opens only when the pointer rests on it. +- In Files, a press anywhere on a row plays the file. +- The feed's toolbar pills are named by a glyph, with Sort and View on the filters' row. + +### Fixed + +- In a play queue, Previous and Next pass over a video that can't play, in the direction you were going, and say why when only such videos are left. +- A Facebook download titled only “Facebook” now takes the post's text as its name. +- The player's big view keeps its way out in sight on a tablet, and Tab no longer lands on controls hidden under it. + ## 0.114.3 — 2026-11-01 ### Fixed diff --git a/VERSION b/VERSION index c672d9c2..bdc80994 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.114.3 +0.115.0 diff --git a/backend/app/downloads/adapters.py b/backend/app/downloads/adapters.py index 7878bffe..ee693f6c 100644 --- a/backend/app/downloads/adapters.py +++ b/backend/app/downloads/adapters.py @@ -66,6 +66,11 @@ class SourceAdapter: """The download's raw title, before `titles.normalize_title` cleans it. Generic: yt-dlp's.""" return info.get("title") + def is_placeholder_title(self, title: str | None) -> bool: + """Whether `title` is a stand-in the source gives in place of the real one (worth one more, + signed-out read of the page). Generic: never.""" + return False + def uploader_url(self, info: dict) -> str | None: """The channel/uploader page URL. Generic: whatever yt-dlp exposes directly (YouTube gives `channel_url`), else None (reddit / a raw media URL have no derivable channel page).""" @@ -201,11 +206,17 @@ class FacebookAdapter(SourceAdapter): # Measured (2026-09-14): a SIGNED-IN read gives the placeholder "Video" (plain and as Chrome), and an # 18+ reel only readable as Chrome gives "Facebook"; the post's text is still in `description`. A # signed-out read titles the page with that same text behind the engagement counts, cut short. + # Measured (2026-11-01, yt-dlp 2026.08.19): since 2026-10-28 every signed-in reel read gives + # "Facebook" and NO description, while a signed-out read of the same reel still carries the text — + # so a placeholder left with nothing behind it gets one signed-out read (`backfill.public_title_info`). _PLACEHOLDER_TITLES = frozenset({"video", "facebook"}) + def is_placeholder_title(self, title: str | None) -> bool: + return (title or "").strip().lower() in self._PLACEHOLDER_TITLES + def title(self, info: dict) -> str | None: raw = info.get("title") - if (raw or "").strip().lower() not in self._PLACEHOLDER_TITLES: + if not self.is_placeholder_title(raw): return raw line = next( (ln.strip() for ln in (info.get("description") or "").splitlines() if ln.strip()), "" diff --git a/backend/app/downloads/backfill.py b/backend/app/downloads/backfill.py index ae400902..a79ed4e2 100644 --- a/backend/app/downloads/backfill.py +++ b/backend/app/downloads/backfill.py @@ -5,7 +5,10 @@ auto-detected channel becomes a clickable link) and `poster_path` (an ffmpeg fra thumbnail-less sources, so the card / watch page / link preview still shows an image). Best-effort and safe to re-run: it only touches assets still missing the field, and only re-fetches YouTube / Facebook sources (a direct media URL like a reddit HLS manifest carries no channel and its signed -URL may have expired — those just get a locally-generated poster instead). +URL may have expired — those just get a locally-generated poster instead). A Facebook download +titled with the source's placeholder ("Facebook", "Video") is retitled from a signed-out read of its +page, and so is each download name that only repeated the placeholder. That page read +(`public_title_info`) also titles every new download, from the worker. Run once per environment: docker exec siftlode-worker-1 python -c "from app.downloads import backfill; print(backfill.run())" @@ -14,7 +17,8 @@ Run once per environment: from urllib.parse import urlsplit import yt_dlp -from sqlalchemy import select +from sqlalchemy import select, update +from sqlalchemy.orm import Session from app import logs from app.config import settings @@ -22,12 +26,14 @@ from app.db import SessionLocal from app.downloads import adapters from app.downloads import edit as editmod from app.downloads import formats, service -from app.downloads.models import MediaAsset +from app.downloads.models import DownloadJob, MediaAsset +from app.models import DISPLAY_TEXT_MAX +from app.titles import normalize_title log = logs.get_logger(__name__) -def _extract_meta(url: str, adapter: adapters.SourceAdapter, asset_id: int) -> dict | None: +def extract_meta(url: str, adapter: adapters.SourceAdapter, asset_id: int) -> dict | None: """Metadata-only yt-dlp extraction, using the source adapter's client sets + options (YouTube needs the POT/player-client strategy even just to extract).""" with formats.hiding(url): @@ -62,8 +68,43 @@ def _extract_meta_scoped(url: str, adapter: adapters.SourceAdapter, asset_id: in return None +def public_title_info(info: dict, url: str, adapter: adapters.SourceAdapter, asset_id: int) -> dict: + """`info` to title a download from: itself, unless its title is a source's placeholder with + nothing behind it (a signed-in Facebook read: "Facebook", no description). Then one signed-out, + metadata-only read of the page lends its text — the description, else its title — as the + description the adapter takes a placeholder's title from. Nothing better there: `info` as it was.""" + if not adapter.is_placeholder_title(adapter.title(info)): + return info + public = extract_meta(url, adapter, asset_id) + text = public and (public.get("description") or public.get("title")) + return {**info, "description": text} if text else info + + +def _retitle(db: Session, a: MediaAsset, adapter: adapters.SourceAdapter, url: str) -> bool: + """Swap a placeholder title kept from an earlier download for the page's text, and the download + names that only repeated it. A name its owner gave (anything else) stays.""" + if not adapter.is_placeholder_title(a.title): + return False + info = public_title_info({"title": a.title, "description": None}, url, adapter, a.id) + 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 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 == placeholder) + .values(display_name=named[:DISPLAY_TEXT_MAX]) + ) + return True + + def run() -> dict: - filled_url = filled_poster = 0 + filled_url = filled_poster = retitled = 0 with SessionLocal() as db: assets = ( db.execute( @@ -78,7 +119,7 @@ def run() -> dict: adapter = adapters.resolve_adapter(a.source_kind, a.source_ref) if a.uploader_url is None and adapter.wants_metadata_backfill(): url = a.source_webpage_url or service.source_url(a.source_kind, a.source_ref) - info = _extract_meta(url, adapter, a.id) + info = extract_meta(url, adapter, a.id) uu = adapter.uploader_url(info) if info else None if uu: a.uploader_url = uu @@ -86,6 +127,14 @@ def run() -> dict: log.info( "asset %s uploader_url filled", a.id ) # the URL names what they downloaded + 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: # Record a self-hosted poster for every download (the reliable og:image source): # ensure_poster returns the existing .jpg thumbnail sidecar, or cuts a frame. @@ -99,6 +148,7 @@ def run() -> dict: "assets": len(assets), "filled_uploader_url": filled_url, "filled_poster": filled_poster, + "retitled": retitled, } log.info("backfill done: %s", result) return result diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py index 6dd29a92..1ccbf85a 100644 --- a/backend/app/downloads/pipeline.py +++ b/backend/app/downloads/pipeline.py @@ -7,6 +7,7 @@ from datetime import date, datetime, timezone from pathlib import Path import yt_dlp +from sqlalchemy import update from yt_dlp.utils import download_range_func from app import logs, sysconfig @@ -16,6 +17,7 @@ from app.downloads import clip as clipmod from app.downloads import edit as editmod from app.downloads import errors as dlerrors from app.downloads import adapters, credentials, formats, mediakind, playable, probe, quota +from app.downloads import backfill from app.downloads import service, storage from app.downloads import urlguard from app.downloads.jobdb import ( @@ -585,9 +587,14 @@ def _download( # missing duration hid the trim action. Probed before the sidecars so the .nfo runtime has it # too. Same probe the upload ingest uses. vcodec, acodec, width, height, duration_s = probe.resolve_media_facts(produced, info) + # A placeholder title with nothing behind it ("Facebook" from a signed-in read) borrows the + # page's text from one signed-out read; the download itself stays the signed-in one. The page + # yt-dlp read, not the link queued: a group post's link is rewritten to its video, and + # signed out the post itself is the login wall. + titled = backfill.public_title_info(info, info.get("webpage_url") or url, adapter, asset_id) meta = storage.MediaMeta( video_id=info.get("id") or source_ref, - title=normalize_title(adapter.title(info)) or source_ref, + title=normalize_title(adapter.title(titled)) or source_ref, uploader=info.get("uploader") or info.get("channel") or "Unknown Channel", upload_date=_parse_upload_date(info.get("upload_date")), uniqueid_type=adapter.nfo_uniqueid_type(), @@ -1116,14 +1123,20 @@ def _process_clip(job_id: int, asset_id: int) -> None: fields: dict = {"status": JobStatus.DONE, "progress": 100, "phase": None} # A video outside the catalog was queued as "Clip (clip)": the fetch has its title now. placeholder = service.clip_title(None, None) + named = None if asset and fetched_title and asset.title == placeholder: named = service.clip_title(fetched_title, None) asset.title = named - if job_row := db.get(DownloadJob, job_id): - if job_row.display_name == placeholder: - fields["display_name"] = named[:DISPLAY_TEXT_MAX] title = asset.title if asset else None finalized = service.finalize_if_running(db, job_id, **fields) + if finalized and named: + # Only a name that still repeats the placeholder, in one conditional write: one its + # owner gave meanwhile stays. + db.execute( + update(DownloadJob) + .where(DownloadJob.id == job_id, DownloadJob.display_name == placeholder) + .values(display_name=named[:DISPLAY_TEXT_MAX]) + ) db.commit() if finalized: _notify_download_done(db, user_id, title, job_id, kind="edit") 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/channels.py b/backend/app/routes/channels.py index 284ea366..beedfa9e 100644 --- a/backend/app/routes/channels.py +++ b/backend/app/routes/channels.py @@ -76,6 +76,10 @@ def list_channels(user: User = Depends(current_user), db: Session = Depends(get_ "details_synced": ch.details_synced_at is not None, "recent_synced": ch.recent_synced_at is not None, "backfill_done": ch.backfill_done, + # When the follow reached this list: written by the subscription import the run that + # first saw it, or by a subscribe made here — not the channel row's own date, which an + # explored or discovered channel had before it was followed. + "added_at": iso(sub.created_at), } for sub, ch in rows ] 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_catalog_routes.py b/backend/tests/test_catalog_routes.py index e144433a..01be059a 100644 --- a/backend/tests/test_catalog_routes.py +++ b/backend/tests/test_catalog_routes.py @@ -72,3 +72,23 @@ def test_the_sync_status_counts_the_callers_subscriptions(db, make_user, as_user assert (body["subscriptions"], body["channels_total"]) == (1, 1) mine = as_user(user).get("/api/sync/my-status").json() assert (mine["channels_total"], mine["channels_recent_pending"]) == (1, 1) + + +def test_a_channel_lists_when_it_entered_this_users_list_not_when_the_channel_did( + db, make_user, as_user +): + from datetime import datetime, timezone + + user = make_user() + db.add(Channel(id="UCa", title="A", created_at=datetime(2026, 6, 10, tzinfo=timezone.utc))) + db.flush() + db.add( + Subscription( + user_id=user.id, + channel_id="UCa", + created_at=datetime(2026, 10, 12, 8, 30, tzinfo=timezone.utc), + ) + ) + db.commit() + listed = as_user(user).get("/api/channels").json() + assert [(c["id"], c["added_at"]) for c in listed] == [("UCa", "2026-10-12T08:30:00+00:00")] diff --git a/backend/tests/test_facebook_titles.py b/backend/tests/test_facebook_titles.py new file mode 100644 index 00000000..25372183 --- /dev/null +++ b/backend/tests/test_facebook_titles.py @@ -0,0 +1,299 @@ +"""A Facebook download's title when the signed-in read gives only a placeholder. + +Measured on prod (2026-11-01): every Facebook download since 2026-10-28 is titled "Facebook" — the +signed-in read (the owner's cookie) now gives that placeholder and NO description, so the adapter had +no post text to fall back on, and the download name copied the placeholder too. A signed-out read of +the same reel (yt-dlp 2026.08.19, as on prod) still carries the post's text in its description. +""" + +from sqlalchemy import select, update + +from app.config import settings +from app.db import SessionLocal +from app.downloads import adapters, backfill, pipeline +from app.downloads.models import DownloadJob, MediaAsset + +_URL = "https://www.facebook.com/reel/1860682468422402/" +# The page yt-dlp says it read for it (`webpage_url`); a group post's link resolves the same way. +_PAGE = "https://www.facebook.com/watch/?v=1860682468422402" +# The measured signed-out read of that reel. +_PUBLIC = { + "title": "163K reactions · 14K shares | sorry not helping #workhumor #officelife | Work dot exe", + "description": "sorry not helping\n\n#workhumor #officelife", + "uploader": "Work dot exe", +} + + +def _fake_ydl(monkeypatch, outcome): + """yt-dlp faked at its `YoutubeDL` boundary: records each read (its options and its `url`), + answers `outcome`.""" + seen: list[dict] = [] + + class FakeYDL: + def __init__(self, opts): + self.opts = opts + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def extract_info(self, url, download): + seen.append({**self.opts, "url": url}) + return outcome + + monkeypatch.setattr(backfill.yt_dlp, "YoutubeDL", FakeYDL) + return seen + + +def test_a_placeholder_with_nothing_behind_it_takes_the_signed_out_pages_text(monkeypatch): + seen = _fake_ydl(monkeypatch, _PUBLIC) + fb = adapters.FacebookAdapter() + + info = backfill.public_title_info({"title": "Facebook", "description": None}, _URL, fb, 7) + + assert fb.title(info) == "sorry not helping" + # One metadata-only read, and signed out: no cookie goes with it. + assert len(seen) == 1 + assert seen[0]["skip_download"] is True + assert not seen[0].get("cookiefile") + + +def test_a_real_title_reads_nothing_more(monkeypatch): + seen = _fake_ydl(monkeypatch, _PUBLIC) + info = {"title": "Funny Dark Humor Dad Joke!", "description": None} + + assert backfill.public_title_info(info, _URL, adapters.FacebookAdapter(), 7) is info + assert seen == [] + + +def test_a_placeholder_with_the_posts_text_behind_it_reads_nothing_more(monkeypatch): + seen = _fake_ydl(monkeypatch, _PUBLIC) + info = {"title": "Video", "description": "Office jokes, part two"} + + assert backfill.public_title_info(info, _URL, adapters.FacebookAdapter(), 7) is info + assert seen == [] + + +def test_a_placeholder_the_signed_out_page_cannot_better_stays(monkeypatch): + _fake_ydl(monkeypatch, {"title": "Facebook", "description": None}) + fb = adapters.FacebookAdapter() + + info = backfill.public_title_info({"title": "Facebook", "description": None}, _URL, fb, 7) + + assert fb.title(info) == "Facebook" + + +def test_another_source_is_never_read_again(monkeypatch): + seen = _fake_ydl(monkeypatch, _PUBLIC) + info = {"title": "Video", "description": None} + + assert backfill.public_title_info(info, _URL, adapters.SourceAdapter(), 7) is info + assert seen == [] + + +def test_the_backfill_retitles_a_placeholder_download_and_the_names_that_repeated_it( + db, make_user, monkeypatch +): + seen = _fake_ydl(monkeypatch, _PUBLIC) + owner, other = make_user(), make_user() + asset = MediaAsset( + source_kind="url", + source_ref=_URL, + format_sig="fb", + status="ready", + rel_path="facebook/x.mp4", + poster_path="facebook/x.jpg", + uploader_url="https://www.facebook.com/1", + title="Facebook", + ref_count=2, + ) + db.add(asset) + db.flush() + copied = 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", + ) + renamed = DownloadJob( + user_id=other.id, + asset_id=asset.id, + source_kind="url", + source_ref=_URL, + status="done", + queue_pos=0, + profile_snapshot={}, + display_name="Office jokes", + ) + db.add_all([copied, renamed]) + db.commit() + + assert backfill.run()["retitled"] == 1 + assert len(seen) == 1 + + db.expire_all() + assert db.get(MediaAsset, asset.id).title == "sorry not helping" + assert db.get(DownloadJob, copied.id).display_name == "sorry not helping" + assert db.get(DownloadJob, renamed.id).display_name == "Office jokes" + # Run again: nothing left to retitle, and no page read for it. + assert backfill.run()["retitled"] == 0 + assert len(seen) == 1 + + +def test_a_name_given_while_the_backfill_runs_stays(db, make_user, monkeypatch): + # 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( + 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() + + read_page = backfill.public_title_info + + def renamed_while_reading(*args): + with SessionLocal() as other: + other.execute( + update(DownloadJob) + .where(DownloadJob.id == job.id) + .values(display_name="Office jokes") + ) + other.commit() + return read_page(*args) + + monkeypatch.setattr(backfill, "public_title_info", renamed_while_reading) + monkeypatch.setattr(backfill.editmod, "ensure_poster", lambda a, root: None) + + assert backfill.run()["retitled"] == 1 + + 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 +): + # The whole download path with yt-dlp and the media tools faked: the signed-in read gives only + # the placeholder, and the file and the download's name get the post's text. + seen = _fake_ydl(monkeypatch, _PUBLIC) + monkeypatch.setattr(settings, "files_root", str(tmp_path)) + monkeypatch.setattr(pipeline.urlguard, "check_url", lambda url: None) + monkeypatch.setattr(pipeline, "_ensure_phone_safe", lambda _j, produced, *_a: produced) + monkeypatch.setattr( + pipeline.probe, "resolve_media_facts", lambda produced, info: ("h264", "aac", 720, 1280, 9) + ) + monkeypatch.setattr(pipeline.editmod, "thumbnail_to_jpg", lambda *a: None) + monkeypatch.setattr(pipeline.editmod, "ensure_poster", lambda *a: None) + owner = make_user() + asset = MediaAsset( + source_kind="url", source_ref=_URL, format_sig="fb", status="downloading", 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="running", + queue_pos=0, + profile_snapshot={}, + ) + db.add(job) + db.commit() + + def signed_in_read(job_id, asset_id, url, spec, staging, adapter, **kw): + f = staging / "1860682468422402.mp4" + f.write_bytes(b"v" * 100) + return { + "id": "1860682468422402", + "title": "Facebook", + "uploader": "Work dot exe", + "webpage_url": _PAGE, + "requested_downloads": [{"filepath": str(f)}], + } + + monkeypatch.setattr(pipeline, "_extract_with_fallback", signed_in_read) + pipeline._download(job.id, asset.id, "url", _URL, {}, owner.id) + + # The signed-out read goes to the page yt-dlp read, not the link queued: a group post's link + # resolves to its video, and signed out the post itself is the login wall. + assert [r["url"] for r in seen] == [_PAGE] + db.expire_all() + assert db.get(DownloadJob, job.id).status == "done" + assert db.get(MediaAsset, asset.id).title == "sorry not helping" + assert db.get(DownloadJob, job.id).display_name == "sorry not helping" diff --git a/backend/tests/test_failure_lines.py b/backend/tests/test_failure_lines.py index 4e43e501..e5122140 100644 --- a/backend/tests/test_failure_lines.py +++ b/backend/tests/test_failure_lines.py @@ -368,7 +368,7 @@ def test_the_metadata_backfill_names_the_asset_and_not_the_download(sink, monkey ) monkeypatch.setattr(backfill.yt_dlp, "YoutubeDL", _Refusing) - assert backfill._extract_meta(url, adapters.YouTubeAdapter(), 11) is None + assert backfill.extract_meta(url, adapters.YouTubeAdapter(), 11) is None lines = [o for o in sink.objects() if "meta extract failed" in o["msg"]] assert lines and all("dQw4w9WgXcQ" not in json.dumps(o) for o in lines) @@ -393,7 +393,7 @@ def test_the_metadata_backfill_takes_out_a_generic_name_whatever_it_holds(sink, raise yt_dlp.utils.DownloadError("ERROR: [generic] Our Trip: Day 1: Unable to download") monkeypatch.setattr(backfill.yt_dlp, "YoutubeDL", _Refusing) - assert backfill._extract_meta(url, adapters.SourceAdapter(), 11) is None + assert backfill.extract_meta(url, adapters.SourceAdapter(), 11) is None lines = [o for o in sink.objects() if "meta extract failed" in o["msg"]] assert lines and all("Day 1" not in json.dumps(o) for o in lines) 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/backend/tests/test_upload.py b/backend/tests/test_upload.py index 0b385594..e0fcd46d 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -579,7 +579,7 @@ def _display_write_sites() -> dict[tuple[str, str], set[str]]: # enqueue_clip slices the source's title to the same width. # files.ingest_spooled passes through `spool_upload`'s value, bounded there. # routes enqueue_clip the `display_name=` keyword passes on the name bound above it. -# pipeline._download / _process_clip / worker._finish_from_cache +# pipeline._download / _process_clip / worker._finish_from_cache / backfill._retitle # slice a fetched or stored title to the column's width. # # Held as EXACT EQUALITY rather than as a list of offenders. The offender form could only fail @@ -596,6 +596,7 @@ DISPLAY_WRITE_SITES = { ("downloads/service.py", "enqueue_clip"): {"display_name or asset.title[:DISPLAY_TEXT_MAX]"}, ("downloads/pipeline.py", "_download"): {"meta.title[:DISPLAY_TEXT_MAX]"}, ("downloads/pipeline.py", "_process_clip"): {"named[:DISPLAY_TEXT_MAX]"}, + ("downloads/backfill.py", "_retitle"): {"named[:DISPLAY_TEXT_MAX]"}, ("worker.py", "_finish_from_cache"): {"asset.title[:DISPLAY_TEXT_MAX]"}, ("downloads/upload.py", "ingest_upload"): {"display_text(display_name)"}, ("routes/auth.py", "callback"): {"display_text(userinfo.get('name'))"}, diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 01c946fa..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 @@ -829,3 +829,8 @@ of a full-width panel and nothing on the right. | 2026-10-25 | Auto-advance glyphs (owner E A, H a) | Auto-advance wears list glyphs (`ListEnd` next and off, `ListStart` previous, `Shuffle` random), in the panel and the docked bar; the auto-advance and loop cycles are `lit`, not `pressed` | The skip arrows for the mode; an X'd list for off; `aria-pressed` on a cycle | Beside Previous / Next the skip arrows read as a second Next; a cycle of three or four modes is no two-state switch, and its name already says which | | 2026-10-25 | Restart and Reset glyphs (owner L A) | A menu's Play from the beginning wears `RotateCcw`, as the card's own Restart does; Reset (clear progress and status) wears `Eraser`; Redownload wears `RefreshCw`, the retry arrow | The skip arrow ⏮ on the menu's Restart; `RotateCcw` on both Restart and Reset; `TimerReset` for Restart | ⏮ is Previous beside the player; one action wore two glyphs, and two actions in one menu would have worn one | | 2026-10-28 | Notification order (owner A-D1 a, A-D2 a, ★ a) | What asks for the reader first, from either source ("Needs you · n"), then one stream newest first; read rows older than a week fold under "Earlier (n)", closed, its label in the section-heading style with a chevron | One stream with no band; this browser's activity above the server's (the two fixed sections); a server-side retention that deletes old rows | An admin's access requests sat under up to a hundred server rows; week-old read notices crowded what is new; folding keeps the rows (nothing is deleted) and an unread row never folds, whatever its age | +| 2026-11-01 | Docked bar glyphs (owner D8 a) | The maximised view's docked bar is glyph-only too (tooltips carry the names and keys): transport, auto-advance, loop, Mark watched (✓ / ✓✓); its controls scroll on a narrow window while Fullscreen and Exit stay pinned at its right end | Labels from `xl` up | On a tablet the labelled bar ran past the right edge and took Exit with it; the side panel had already lost its labels | +| 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/e2e/channel.spec.ts b/frontend/e2e/channel.spec.ts index 09c78984..b0bd837d 100644 --- a/frontend/e2e/channel.spec.ts +++ b/frontend/e2e/channel.spec.ts @@ -114,7 +114,9 @@ test.describe("channel view", () => { }) => { await openApp(page); await page.getByTestId("video-card-channel").first().click(); - const count = page.getByTestId("item-count"); + // A pill of the feed's toolbar, which rides in the fixed chrome (the page has no count pill: its + // header counts the videos). + const count = page.getByTestId("feed-show"); await expect(count).toBeVisible(); await expect(page.getByTestId("video-card").first()).toBeVisible(); await expect(page.getByTestId("channel-folded")).toHaveCount(0); diff --git a/frontend/src/components/ui/DataTable.cards.dom.test.tsx b/frontend/src/components/ui/DataTable.cards.dom.test.tsx index abf57aef..f543a227 100644 --- a/frontend/src/components/ui/DataTable.cards.dom.test.tsx +++ b/frontend/src/components/ui/DataTable.cards.dom.test.tsx @@ -2,9 +2,9 @@ // // Below `md` a table is a list of cards, its columns flowing one after another. A column marked // `cardRow` takes a line of its own there — a row's lone "…" otherwise landed at the end of one -// card's line and on the next line of another. +// card's line and on the next line of another. A card is its row's press surface too, as the row is. import { describe, it, expect, afterEach, vi } from "vitest"; -import { render, cleanup, screen } from "@testing-library/react"; +import { render, cleanup, screen, fireEvent } from "@testing-library/react"; import DataTable, { type Column } from "./DataTable"; vi.mock("react-i18next", async (importOriginal) => ({ @@ -43,4 +43,21 @@ describe("DataTable cards", () => { expect(screen.getByText("more").parentElement!.className).toContain("basis-full"); expect(screen.getByText("1 MB").parentElement!.className).not.toContain("basis-full"); }); + + it("opens its object from a tap on the card, and leaves its control's tap to the control", () => { + narrow(false); + const open = vi.fn(); + render( + r.id} + rowOpen={() => open} + />, + ); + fireEvent.click(screen.getByText("1 MB"), { detail: 1 }); + expect(open).toHaveBeenCalledTimes(1); + fireEvent.click(screen.getByText("more"), { detail: 1 }); + expect(open).toHaveBeenCalledTimes(1); + }); }); diff --git a/frontend/src/components/ui/DataTable.tsx b/frontend/src/components/ui/DataTable.tsx index bf179689..463243b4 100644 --- a/frontend/src/components/ui/DataTable.tsx +++ b/frontend/src/components/ui/DataTable.tsx @@ -9,6 +9,7 @@ import { alignClass } from "./tableAlign"; import { columnHideClass, filterRows, type Column, type FilterMap } from "../../lib/tableColumns"; import { navItemAttrs, useListNav } from "../../lib/listNav"; import { useMediaQuery } from "../../lib/useMediaQuery"; +import { withRowOpen } from "./rowOpen"; import { SURFACE } from "./surfaces"; import { HiddenFilterCue } from "./HiddenFilterCue"; import { useProgressiveRows } from "./progressiveRows"; @@ -57,6 +58,7 @@ interface TableRowProps { index: number; columns: Column[]; rowProps?: (row: T) => Record; + rowOpen?: (row: T) => (() => void) | null; rowClassName?: (row: T) => string; keyboardNav: boolean; } @@ -69,14 +71,16 @@ function TableRow({ index, columns, rowProps, + rowOpen, rowClassName, keyboardNav, }: TableRowProps) { + const open = rowOpen?.(row) ?? null; return ( {columns.map((col) => ( ({ emptyText, rowClassName, rowProps, + rowOpen, controlsPosition = "bottom", controlsLeading, controlsInBand = false, @@ -137,6 +142,10 @@ export default function DataTable({ // answer the same gesture. Deliberately not a bag of named callbacks: the table stays domain-free // and never learns what a menu is. rowProps?: (row: T) => Record; + // The row's default action (play, open), or null where a row has none: a press anywhere on the row + // or its card twin runs it, except on a control inside it or at the end of a text selection + // (`rowOpen.ts`). The keyboard's Enter keeps reaching it through the row's `data-nav-open` element. + rowOpen?: (row: T) => (() => void) | null; controlsPosition?: "top" | "bottom" | "both"; controlsLeading?: ReactNode; // Render the controls row (controlsLeading + pager) into the shell's fixed toolbar band instead @@ -414,6 +423,7 @@ export default function DataTable({ index={i} columns={columns} rowProps={rowProps} + rowOpen={rowOpen} rowClassName={rowClassName} keyboardNav={keyboardNav} /> @@ -430,9 +440,9 @@ export default function DataTable({ {shown.map((row, i) => (
{columns .filter((c) => c.cardPrimary) diff --git a/frontend/src/components/ui/FilterSelect.tsx b/frontend/src/components/ui/FilterSelect.tsx index d94b4997..822d4e52 100644 --- a/frontend/src/components/ui/FilterSelect.tsx +++ b/frontend/src/components/ui/FilterSelect.tsx @@ -1,6 +1,6 @@ -import { useState } from "react"; +import { useState, type ReactNode } from "react"; import { MenuCheck } from "./MenuCheck"; -import { SelectTrigger } from "./SelectTrigger"; +import { GroupGlyph, SelectTrigger } from "./SelectTrigger"; import { PopoverItem, PopoverSurface } from "./AnchoredPopover"; import { useAnchoredPopover } from "./anchoredPopoverHooks"; import { SURFACE } from "./surfaces"; @@ -27,6 +27,10 @@ export interface FilterSelectProps { /** Show only the value on the trigger — for a form row whose own caption already names the field. * The label still names the control and its menu for assistive tech. */ hideLabel?: boolean; + /** A glyph that names the group in place of its word, for a toolbar where the value says enough + * once the group is recognisable (the feed's watch state: an eye). The word moves to the tooltip + * and stays in the accessible name. */ + icon?: ReactNode; options: readonly FilterSelectOption[]; selected: string[]; onChange: (next: string[]) => void; @@ -46,6 +50,7 @@ export default function FilterSelect({ multiple = false, requireOne = true, hideLabel = false, + icon, testId, }: FilterSelectProps) { const [open, setOpen] = useState(false); @@ -95,7 +100,9 @@ export default function FilterSelect({ {icon}} + title={icon ? label : undefined} ref={setReference} {...pop.getReferenceProps()} data-testid={testId} diff --git a/frontend/src/components/ui/README.md b/frontend/src/components/ui/README.md index 8dd13ac4..7418c0ad 100644 --- a/frontend/src/components/ui/README.md +++ b/frontend/src/components/ui/README.md @@ -35,13 +35,13 @@ Counter-examples, deliberately NOT here: `PosterTile` (knows about Plex montage ## What lives here -| area | components | -| ---------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| overlays & anchoring | `AnchoredPopover` + `anchoredPopoverHooks` (the popover engine: arrows, typeahead, Escape, anchor-flip), `ActionMenu`, `MembershipMenu` (the body of a "which sets is it in" menu: checkbox rows + a field naming a new set), `ContextMenu` + `useContextMenu` (the SAME menu, opened by right-click / long-press / Shift+F10 / a row's `MoreButton` "…" via `buttonProps`; `anchor` for a panel a row opens; `PageMenu` for a page's background menu), `Modal` (+ `ModalFooter`; `initialFocus` by purpose, C4), `Overlay`, `SidePanel`, `Tooltip`, `Toaster` (+ `toastLevels`, `overlayOrder`; `lib/toastDock` docks one control under the stack, in every host), `CommandPalette`, `KeyboardHelp`, `ConfirmProvider` + `confirmContext` (`useConfirm` / `useChoice`: the app's promise-based confirm, mounted once at the root; a decision with more than two outcomes is one `useChoice`, never two chained confirms), `dialogFocus` (the focus half of the dialog contract: `Modal`'s, and the wizard's own shell), `menuGroups` (one menu from its groups, a rule only between non-empty ones), `pageMenuSlot` + `pageMenuItems` (a page's background menu, mirroring its own toolbar) | -| actions & inputs | `Button`, `IconButton`, `Pressable` (`selectable` when its text is a title or a name), `Chip` + `chipStyles`, `DragHandle` + `OrderButtons` / `useOrderItems` (the non-drag reorder, WCAG 2.5.7) (one per button family, docs/DESIGN.md; a raw ` +
+ +
+
, + ); + expect(focusablesIn(container).map((el) => el.textContent)).toEqual(["reachable"]); + offsetParent.mockRestore(); + }); +}); + describe("useDialogChrome · initial focus", () => { it("lands on the first control by default", () => { // jsdom has no layout, so the visibility filter (offsetParent) would see nothing focusable. diff --git a/frontend/src/components/ui/dialogFocus.ts b/frontend/src/components/ui/dialogFocus.ts index 35a00c20..03687254 100644 --- a/frontend/src/components/ui/dialogFocus.ts +++ b/frontend/src/components/ui/dialogFocus.ts @@ -9,12 +9,15 @@ import { useEffect, useEffectEvent, useRef, useState, type RefObject } from "rea // Tabbable elements inside the dialog, in DOM order. `:not([tabindex="-1"])` drops programmatically // focusable-only nodes; the visibility filter (offsetParent) drops display:none ones (e.g. a -// collapsed disclosure) so the trap never lands focus on something the user can't see. +// collapsed disclosure) so the trap never lands focus on something the user can't see, and an inert +// part's are not tabbable: counted, the trap's last control is one Tab never reaches. const FOCUSABLE = 'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), [tabindex]:not([tabindex="-1"])'; export const focusablesIn = (root: HTMLElement): HTMLElement[] => - [...root.querySelectorAll(FOCUSABLE)].filter((el) => el.offsetParent !== null); + [...root.querySelectorAll(FOCUSABLE)].filter( + (el) => el.offsetParent !== null && !el.closest("[inert]"), + ); /** A form's first field — where a dialog opened to fill something in takes the focus: its first * control, unless one is marked `data-initial-focus`. The mark is for a form with a control above diff --git a/frontend/src/components/ui/rowOpen.ts b/frontend/src/components/ui/rowOpen.ts new file mode 100644 index 00000000..bcce3de9 --- /dev/null +++ b/frontend/src/components/ui/rowOpen.ts @@ -0,0 +1,73 @@ +import type { MouseEvent, PointerEvent } from "react"; +import { endsTextSelection, movedPastSlop } from "../../lib/textSelection"; + +// A table row (or its narrow card twin) as the click and tap surface of its object's default action, +// the way a card is: a press anywhere on the row opens it, not only on the title inside it. +// +// What the row does NOT claim: +// - a press on a control inside it (its quick icons, its "…", a link, a field): that is the control's; +// - a click that ends a text selection, or a press that travelled (a drag across the row's padding): +// DESIGN.md "Text stays selectable" — no play or open fires; +// - an event from a portal (a popover a cell opened): React bubbles it through the row, the DOM does +// not, so only a target the row really contains counts; +// - a press that began while a field inside the row held the focus (its rename): that press ends the +// editing — its mousedown blurs the field, which saves — and does nothing more. Read at pointerdown, +// because by the click the blur has already put the row back as it is when nothing is edited. +// The long-press menu's trailing click is swallowed by its own trigger (`useContextMenu`), before it +// reaches the row. + +// A field the focus can be editing in. +const FIELD = 'input, textarea, select, [contenteditable="true"]'; + +// What counts as a control of its own. `[tabindex]` covers a focusable cell (a hover card's trigger); +// the row's own roving tab stop is the row itself, which the containment check below leaves out. +const CONTROL = + 'a, button, input, select, textarea, label, summary, [role="button"], [role="menuitem"], [role="checkbox"], [role="switch"], [contenteditable="true"], [tabindex]'; + +// Where each row's press began, for the drag slop, and whether it began while a field inside the row +// was being edited. Per element: the table row and its card twin are two elements, and a press on one +// must not be measured against the other. +const pressedAt = new WeakMap(); + +type Handlers = { + onPointerDown?: (e: PointerEvent) => void; + onClick?: (e: MouseEvent) => void; +}; + +/** The row's props with its default action added on top of the caller's own (a context menu's + * trigger props, which keep their handlers). `open` null: the row has no default action. */ +export function withRowOpen( + props: Record | undefined, + open: (() => void) | null, +): Record { + const base = props ?? {}; + if (!open) return base; + const { onPointerDown, onClick } = base as Handlers; + return { + ...base, + onPointerDown: (e: PointerEvent) => { + const row = e.currentTarget; + const focused = document.activeElement; + const editing = !!focused && row.contains(focused) && focused.matches(FIELD); + pressedAt.set(row, { x: e.clientX, y: e.clientY, editing }); + onPointerDown?.(e); + }, + onClick: (e: MouseEvent) => { + onClick?.(e); + const claimed = !e.defaultPrevented && claims(e); + // One press, one click: a later keyboard click (no press) is not judged by this one. + pressedAt.delete(e.currentTarget); + if (claimed) open(); + }, + }; +} + +function claims(e: MouseEvent): boolean { + const row = e.currentTarget; + const target = e.target; + if (!(target instanceof Element) || !row.contains(target)) return false; + const control = target.closest(CONTROL); + if (control && control !== row && row.contains(control)) return false; + const pressed = pressedAt.get(row) ?? null; + return !pressed?.editing && !endsTextSelection(e) && !movedPastSlop(pressed, e); +} diff --git a/frontend/src/components/ui/unavailable.ts b/frontend/src/components/ui/unavailable.ts new file mode 100644 index 00000000..5504c83b --- /dev/null +++ b/frontend/src/components/ui/unavailable.ts @@ -0,0 +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 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, "aria-description": reason } + : { disabled: true }; +} diff --git a/frontend/src/content/release-notes/en.json b/frontend/src/content/release-notes/en.json index a1cf90ee..971cca82 100644 --- a/frontend/src/content/release-notes/en.json +++ b/frontend/src/content/release-notes/en.json @@ -1,4 +1,19 @@ [ + { + "version": "0.115.0", + "date": "2026-11-01", + "new": [ + "Clear watch progress is on every YouTube video's menu, on Home, in playlists and in the player's related videos. It forgets where you left a started video and changes nothing else.", + "Channels have an Added column, and an About card opens only when the pointer rests on it.", + "In Files, a press anywhere on a row plays the file.", + "The feed's toolbar pills are named by a glyph, with Sort and View on the filters' row." + ], + "fixed": [ + "In a play queue, Previous and Next pass over a video that can't play, in the direction you were going, and say why when only such videos are left.", + "A Facebook download titled only “Facebook” now takes the post's text as its name.", + "The player's big view keeps its way out in sight on a tablet, and Tab no longer lands on controls hidden under it." + ] + }, { "version": "0.114.3", "date": "2026-11-01", diff --git a/frontend/src/content/release-notes/hu.json b/frontend/src/content/release-notes/hu.json index ee918e94..efb06aca 100644 --- a/frontend/src/content/release-notes/hu.json +++ b/frontend/src/content/release-notes/hu.json @@ -1,4 +1,19 @@ [ + { + "version": "0.115.0", + "date": "2026-11-01", + "new": [ + "A Megtekintés visszaállítása minden YouTube-videó menüjében elérhető: a Kezdőlapon, a listákban és a lejátszó kapcsolódó videóinál. Egy megkezdett videónál törli, hol tartottál, mást nem változtat.", + "A csatornáknál van Hozzáadva oszlop, és a Leírás kártya csak akkor nyílik meg, ha a mutató megáll rajta.", + "A Fájloknál a sorban bárhová kattintva elindul a fájl.", + "A Feed eszköztárának gombjait ikon jelöli, a Rendezés és a Nézet a szűrők sorába került." + ], + "fixed": [ + "A lejátszási sorban az Előző és a Következő átlépi a nem lejátszható videót abban az irányban, amerre haladtál, és megmondja az okát, ha már csak ilyenek maradtak.", + "A csak „Facebook” címmel letöltött Facebook-videó most a bejegyzés szövegét kapja névnek.", + "A lejátszó nagy nézetében tableten is látszik a kilépés, és a Tab már nem kerül a takart gombokra." + ] + }, { "version": "0.114.3", "date": "2026-11-01", diff --git a/frontend/src/features/player/PlayerModal.dom.test.tsx b/frontend/src/features/player/PlayerModal.dom.test.tsx index 2c2bbd90..1edf96b3 100644 --- a/frontend/src/features/player/PlayerModal.dom.test.tsx +++ b/frontend/src/features/player/PlayerModal.dom.test.tsx @@ -63,7 +63,11 @@ const started: { videoId: string; start: number | undefined }[] = []; // What the player was asked to switch to in place (a description link, a related video). const loaded: { videoId: string; startSeconds: number }[] = []; // The last player's event handlers, so a test can say "the player is ready" when it wants to. -let playerEvents: { onReady?: () => void; onStateChange?: (e: { data: number }) => void } = {}; +let playerEvents: { + onReady?: () => void; + onStateChange?: (e: { data: number }) => void; + onError?: (e: { data: number }) => void; +} = {}; const base = { getCurrentTime: () => clock.current, getDuration: () => clock.duration, @@ -1046,6 +1050,73 @@ describe("PlayerModal — the side panel on a wide window", () => { expect(lastStart()?.start ?? 0).toBe(0); }); + // 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 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, "clearProgress").mockResolvedValue(undefined as never); + clock.current = 77; + await playRel2(() => Promise.resolve()); + clock.current = 500; + fireEvent.click(screen.getByRole("button", { name: "player.back" })); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + await settle(); + const list = screen.getByTestId("player-related"); + const links = () => within(list).getAllByRole("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(reset).toHaveBeenCalledWith("rel2")); + await settle(); + fireEvent.click(titleOf(links())); + await waitFor(() => expect(lastStart()?.videoId).toBe("rel2")); + 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 () => { @@ -1644,6 +1715,137 @@ describe("PlayerModal — the docked bar's fullscreen", () => { it("is left out where it cannot", () => { expect(maximised(false)).toBe(1); }); + + // On an iPad the bar's row outgrew the screen and its last control — the way out of the big + // view — scrolled off the right edge. The way out stays put; only the controls before it scroll. + it("keeps the way out in sight: glyphs only, and out of the scrolling part", () => { + maximised(true); + const out = screen.getByRole("button", { name: "player.exitMaximize" }); + const bar = out.parentElement!; + // The windowed panel keeps a Fullscreen of its own too; this is the bar's. + const full = screen.getAllByLabelText("player.fullscreen").find((b) => bar.contains(b))!; + expect(full).toBeTruthy(); + // The windowed row keeps its own Mark watched; this is the bar's. + const watched = screen + .getAllByRole("button", { name: "player.markWatched" }) + .find((b) => bar.contains(b))!; + for (const b of [out, full, watched]) expect(b.textContent).toBe(""); + const scroller = watched.closest(".overflow-x-auto"); + expect(scroller).not.toBeNull(); + expect(scroller!.contains(out)).toBe(false); + expect(scroller!.contains(full)).toBe(false); + }); +}); + +// 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 = (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()} + />, + ); + return screen.getByLabelText("player.fullscreen"); + }; + + it("takes the card out of reach while the view is big, and gives it back", () => { + const enter = open(); + expect(enter.closest("[inert]")).toBeNull(); + fireEvent.click(enter); + expect(enter.closest("[inert]")).not.toBeNull(); + fireEvent.keyDown(window, { key: "Escape" }); + expect(enter.closest("[inert]")).toBeNull(); + }); + + it("walks a keyboard from the card's button to the way out, and back to where it was", () => { + const enter = open(); + act(() => enter.focus()); // jsdom calls every focus :focus-visible — a key's focus, here + fireEvent.click(enter); + expect(document.activeElement).toBe(screen.getByLabelText("player.exitMaximize")); + fireEvent.click(screen.getByLabelText("player.exitMaximize")); + expect(document.activeElement).toBe(enter); + }); + + it("does not leave a mouse on the way out, where a Space would leave the big view", () => { + const enter = open(); + const matches = vi.spyOn(enter, "matches").mockReturnValue(false); + 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); + act(() => screen.getByLabelText("player.exitMaximize").focus()); + fireEvent.keyDown(window, { key: "Escape" }); + expect(document.activeElement).not.toBe(document.body); + expect(screen.getByRole("dialog").contains(document.activeElement)).toBe(true); + }); }); // Clip mode: the panel sits under the video (the video is its preview), Escape closes the panel and @@ -1753,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")); @@ -1816,3 +2031,174 @@ describe("PlayerModal — clip mode", () => { expect(screen.getByTestId("clip-panel")).toBeTruthy(); }); }); + +// An embed-blocked or deleted video answers with onError, never 'ended'. Stepping BACK onto one used +// to skip forward again (the auto-advance direction), straight to the item you had just left. +describe("PlayerModal — stepping over a video that cannot play", () => { + const A = vid("A", 0); + const B = vid("B", 0); + const C = vid("C", 0); + const fail = () => act(() => playerEvents.onError?.({ data: 150 })); + const opened = (id: string) => started.filter((s) => s.videoId === id).length; + + beforeEach(() => { + vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); + vi.spyOn(api, "saveProgress").mockImplementation(async (id: string) => ({ + video_id: id, + position_seconds: 0, + })); + }); + + const open = async (startIndex: number) => { + renderModal( + {}} + onState={vi.fn()} + />, + ); + await waitFor(() => expect(lastStart()?.videoId).toBe([A, B, C][startIndex]!.id)); + }; + + it("Previous onto a dead video goes on back, not forward to where you came from", async () => { + me = { related_enabled: true, preferences: { playerAutoAdvance: "next" } } as unknown as Me; + await open(2); + prev(); + await waitFor(() => expect(lastStart()?.videoId).toBe("B")); + fail(); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + }); + + it("a step of your own skips a dead video with auto-advance off too", async () => { + await open(2); + prev(); + await waitFor(() => expect(lastStart()?.videoId).toBe("B")); + fail(); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + }); + + it("a video found dead is stepped over without loading it again", async () => { + await open(2); + prev(); + await waitFor(() => expect(lastStart()?.videoId).toBe("B")); + fail(); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + next(); + await waitFor(() => expect(lastStart()?.videoId).toBe("C")); + expect(opened("B")).toBe(1); + }); + + // Off, and saying why: the counter shows a video before this one, so a dark button alone would not + // tell (DESIGN "Unavailable with a reason stays focusable"). + it("Previous is off, with its reason, when only dead videos are before you", async () => { + await open(0); + fail(); // opened on it, auto-advance off: it stays, and it is known dead now + await settle(); + expect(lastStart()?.videoId).toBe("A"); + next(); + await waitFor(() => expect(lastStart()?.videoId).toBe("B")); + const prevs = screen.getAllByRole("button", { name: "player.previous" }); + for (const b of prevs) { + expect((b as HTMLButtonElement).disabled).toBe(false); + expect(b.getAttribute("aria-disabled")).toBe("true"); + } + // 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"); + // Next has playable C ahead: on, and no reason. + for (const b of screen.getAllByRole("button", { name: "player.next" })) + 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" })) { + expect((b as HTMLButtonElement).disabled).toBe(true); + expect(b.getAttribute("title")).not.toContain("player.noPlayableBefore"); + } + }); + + // Removing the playing video passes over the ones known dead, the way a step does, and goes back + // when nothing playable followed it. + it("removing the last video goes back over a dead one to the one that plays", async () => { + renderModal( + {}} + onState={vi.fn()} + onRemoveFromList={async () => true} + />, + ); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + next(); + await waitFor(() => expect(lastStart()?.videoId).toBe("B")); + fail(); + await waitFor(() => expect(lastStart()?.videoId).toBe("C")); + fireEvent.click(trash("C")); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + expect(opened("B")).toBe(1); + }); + + // The player gives up after three in a row: it stays on the third's error, and all three are + // remembered — a later step passes over them. + it("three dead in a row stop on the third, and a step then passes over all three", async () => { + const [D, E] = [vid("D", 0), vid("E", 0)]; + renderModal( + {}} + onState={vi.fn()} + />, + ); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + next(); + await waitFor(() => expect(lastStart()?.videoId).toBe("B")); + fail(); + await waitFor(() => expect(lastStart()?.videoId).toBe("C")); + fail(); + await waitFor(() => expect(lastStart()?.videoId).toBe("D")); + fail(); + await settle(); + expect(lastStart()?.videoId).toBe("D"); + prev(); + await waitFor(() => expect(lastStart()?.videoId).toBe("A")); + next(); + await waitFor(() => expect(lastStart()?.videoId).toBe("E")); + expect([opened("B"), opened("C"), opened("D")]).toEqual([1, 1, 1]); + }); + + it("a video opened dead with auto-advance off stays on its error", async () => { + await open(1); + fail(); + await settle(); + expect(lastStart()?.videoId).toBe("B"); + expect(started).toHaveLength(1); + }); +}); diff --git a/frontend/src/features/player/PlayerModal.tsx b/frontend/src/features/player/PlayerModal.tsx index ed6cd06a..824b07db 100644 --- a/frontend/src/features/player/PlayerModal.tsx +++ b/frontend/src/features/player/PlayerModal.tsx @@ -14,7 +14,8 @@ 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 { AlertTriangle, @@ -72,7 +73,7 @@ import { linkedPlayer, playerParamUpdates, type PlayerPanelTab } from "../../lib import ChannelNameLink from "../video/ChannelNameLink"; import Toaster from "../../components/ui/Toaster"; import { useOverlayOrder } from "../../components/ui/overlayOrder"; -import { afterRemoval } from "../../lib/playerQueue"; +import { afterRemoval, stepIndex } from "../../lib/playerQueue"; import { useNativeControlsYield, useYtBigView, @@ -118,6 +119,11 @@ function canScrollBy(el: HTMLElement | null, dir: -1 | 1): boolean { // a rem width, so it grows with the text size; the video takes what is left. const WIDE_PLAYER_PX = 1280; const SIDE_PANEL = "20rem"; +// The way a queue step goes: back, on, or to a random other item. +type Way = 1 | -1 | "random"; +const autoWay = (a: Exclude): Way => + a === "random" ? "random" : a === "next" ? 1 : -1; + // The side panel shut to a strip: one icon button wide, close / open / previous / next in a column. const PANEL_STRIP = "2.75rem"; // The narrowest the stacked video gets to make room for the chrome under it (see the card's maxWidth). @@ -444,7 +450,8 @@ export default function PlayerModal({ // above it ends there. const clipDockRef = useRef(null); const [clipDockH, setClipDockH] = useState(0); - const clipDocked = clipOpen && (maximized || isFullscreen); + const bigView = maximized || isFullscreen; + const clipDocked = clipOpen && bigView; // Its two places are two parents (the dock is IN the stage, the card's slot under it), and a panel // rendered in the other one is a new panel: the typed name, Loop and the focus were lost on every // F. So it renders once, into a host node of its own, and the host moves between the two. @@ -527,14 +534,41 @@ export default function PlayerModal({ const lost = document.activeElement == null || document.activeElement === document.body; if (!refocusClip.current && !lost) return; refocusClip.current = false; - const big = maximized || isFullscreen; - const target = big ? clipDockButtonRef.current : clipButtonRef.current; + const target = bigView ? clipDockButtonRef.current : clipButtonRef.current; if (target) target.focus(); else focusModal(); }); useEffect(() => { if (!clipOpen) refocusAfterClip(); }, [clipOpen]); + // 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. 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 = panelFocusByKey.current; + focusBeforeBig.current = byKey ? active : null; + if (byKey && bigExitRef.current) bigExitRef.current.focus(); + else focusModal(); + return; + } + const back = focusBeforeBig.current; + focusBeforeBig.current = null; + 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 * browser's fullscreen steps back to the maximised view, which has room for it. */ @@ -657,20 +691,55 @@ export default function PlayerModal({ // Prev/next stepping through the queue (the feed's order, or a playlist). Read the live // queue+index from a ref so the window keydown handler (bound once) always steps from the // current position, not a stale closure. - const navRef = useLatest({ queue, index }); - const goPrev = () => { - const { queue: q, index: i } = navRef.current; - if (q && i > 0) setPlayingId(q[i - 1]!.id); + // + // The videos YouTube refused to play this session (embed-blocked, removed): a step passes over + // them, and Previous / Next go dark when only those are left that way. State, not a ref, because + // the buttons read it. + const [dead, setDead] = useState>(() => new Set()); + const navRef = useLatest({ queue, index, dead }); + // Which way the step that reached the playing item went — what an item that turns out unplayable + // carries on along. null: no step yet (the item the player opened on). + const stepRef = useRef<{ way: Way } | null>(null); + const stepTo = (id: string, way: Way) => { + stepRef.current = { way }; + setPlayingId(id); }; - const goNext = () => { - const { queue: q, index: i } = navRef.current; - if (q && i < q.length - 1) setPlayingId(q[i + 1]!.id); + // Step from `i` the given way over the videos known dead; whether there was anywhere to go. + const moveOn = ( + q: Video[], + i: number, + way: Way, + d: ReadonlySet, + wrap: boolean, + ): boolean => { + if (way === "random") { + const rest = q.filter((v, k) => k !== i && !d.has(v.id)); + if (!rest.length) return false; + stepTo(rest[Math.floor(Math.random() * rest.length)]!.id, "random"); + return true; + } + const j = stepIndex(q, i, way, d, wrap); + if (j < 0) return false; + stepTo(q[j]!.id, way); + return true; }; + const step = (dir: 1 | -1) => { + const { queue: q, index: i, dead: d } = navRef.current; + if (q) moveOn(q, i, dir, d, false); + }; + const goPrev = () => step(-1); + const goNext = () => step(1); + const canPrev = !!queue && stepIndex(queue, index, -1, dead) >= 0; + const canNext = !!queue && stepIndex(queue, index, 1, dead) >= 0; + // Off with items still left that way: only ones YouTube refused are there, which the counter alone + // does not tell. At the queue's ends the counter does. + const prevReason = queue && index > 0 ? t("player.noPlayableBefore") : undefined; + const nextReason = queue && index < queue.length - 1 ? t("player.noPlayableAfter") : undefined; // "Remove from this playlist", offered right where you are watching the thing you want gone. The // parent owns the removal (API + caches + the toast whose Undo puts it back); the player owns what happens NEXT, because it // is the only side that knows which item is playing — its queue is frozen at mount, so the parent - // cannot tell. Step to the item that followed (clamped when the last one goes), and close when the - // list runs out. + // cannot tell. Step to the item that followed (the one before when the last one goes), passing over + // the ones known unplayable, and close when the list runs out. // // Guarded against a double press by a REF, not by disabling the button: a trigger disabled for the // removal's lifetime was still disabled when focus came back to it (it once waited on a confirm) — @@ -687,19 +756,19 @@ export default function PlayerModal({ // Decide from where the player is NOW, not where it was when the button was pressed: prev/next // and auto-advance keep working while the request runs, and a cursor from before the await // threw the viewer back onto a video they had already moved past. - const { queue: q, index: i } = navRef.current; + const { queue: q, index: i, dead: d } = navRef.current; if ((q?.[i] ?? video).id !== victim.id) { // They moved on — take the removed item out of the queue and leave playback where it is. setQueue((cur) => cur?.filter((v) => v.id !== victim.id)); return; } - const next = afterRemoval(q, i, victim.id); + const next = afterRemoval(q, i, victim.id, d); if (!next) { onClose(); // that was the last one return; } setQueue(next.queue); - setPlayingId(next.playingId); + stepTo(next.playingId, next.way); // one that cannot play after all carries on the same way }; // Relative seek within the current video (plain Arrow keys), matching YouTube's ±5s. @@ -738,28 +807,28 @@ export default function PlayerModal({ if (l === "one") return replayCurrent(); // A related video's end is not the queue's: the list beside it is where the next one is picked. if (related) return; - const { queue: q, index: i } = navRef.current; - if (!q || q.length === 0) return; + const { queue: q, index: i, dead: d } = navRef.current; + if (!q || q.length === 0 || a === "off") return; const wrap = l === "all"; - if (a === "next") { - if (i < q.length - 1) setPlayingId(q[i + 1]!.id); - else if (wrap) { - if (q.length === 1) replayCurrent(); - else setPlayingId(q[0]!.id); - } - } else if (a === "prev") { - if (i > 0) setPlayingId(q[i - 1]!.id); - else if (wrap) { - if (q.length === 1) replayCurrent(); - else setPlayingId(q[q.length - 1]!.id); - } - } else if (a === "random" && q.length > 1) { - let r = i; - while (r === i) r = Math.floor(Math.random() * q.length); - setPlayingId(q[r]!.id); - } else if (a === "random" && wrap) { - replayCurrent(); - } + // Nowhere else to go (a one-item list, or every other item dead): Loop "All" repeats this one. + if (!moveOn(q, i, autoWay(a), d, wrap) && wrap) replayCurrent(); + }; + // The playing queue item turned out unplayable (YouTube's onError — it never reaches 'ended'). + // Remember it, then carry on the way the step that reached it went: Previous onto a dead video + // goes on back, not forward to the one you just left. The item the player opened on was reached + // by no step, so it moves on only with auto-advance on, in its direction. `go` false: the run of + // errors hit the player's cap — remembered, but playback stays on the error. + const skipUnplayable = (go: boolean) => { + if (related || clipOpenRef.current) return; + const { queue: q, index: i, dead: d } = navRef.current; + const gone = q?.[i]?.id; + if (!q || !gone) return; + const now = new Set(d).add(gone); + setDead(now); + if (!go || q.length <= 1) return; + const { autoMode: a, loopMode: l } = modeRef.current; + const way = stepRef.current?.way ?? (a === "off" ? null : autoWay(a)); + if (way != null) moveOn(q, i, way, now, l === "all"); }; // Lazy description (fetched only when it is opened). It opens from an ⓘ beside the title, by click @@ -794,6 +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 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 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 @@ -982,14 +1054,13 @@ export default function PlayerModal({ mountRef, playerRef, currentIdRef, - navRef, - modeRef, setWatched, setPlayerError, setLiveData, reapplyStored, reclaimFocus, advanceOnEnd, + skipUnplayable, sessionPosRef, qc, }); @@ -1057,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). */} @@ -1108,7 +1184,7 @@ export default function PlayerModal({ which would drop any class on mountRef itself — the iframe then sizes to this wrapper. */}
)} - {(maximized || isFullscreen) && !clipDocked && ( + {bigView && !clipDocked && (
e.stopPropagation()} - className="absolute inset-x-0 bottom-0 z-menu flex h-14 items-center justify-start gap-x-3 overflow-x-auto border-t border-white/10 bg-media-chrome/95 media-marks px-3 text-sm text-white" + className="absolute inset-x-0 bottom-0 z-menu flex h-14 items-center gap-x-3 border-t border-white/10 bg-media-chrome/95 media-marks px-3 text-sm text-white" > - {hasQueue && ( - <> - - - - )} - {/* Both work in fullscreen now: add-to-playlist's popover and download's Modal both + {/* Glyphs only, each named by its label and tooltip. The controls scroll when a + narrow window cannot hold them; the way out (fullscreen, leave the big view) + stays outside that part, in sight: on an iPad it had scrolled off the right. + The bar's full height and a little side room: a scroller clips what overflows + it, the focus ring drawn outside each control included. */} +
+ {hasQueue && ( + <> + + + + )} + {/* Both work in fullscreen now: add-to-playlist's popover and download's Modal both portal into the fullscreen element rather than (a body-portaled node is painted underneath the element holding the lock, i.e. invisible). */} - {!navigated && } - {!navigated && ( - - )} - {/* Clip mode here: the clip panel takes this bar's place. */} - {playerError == null && ( - - - - )} - {!navigated && ( - - )} - + {!navigated && } + {!navigated && ( + + )} + {/* Clip mode here: the clip panel takes this bar's place. */} + {playerError == null && ( + + + + )} + {!navigated && ( + setWatched(!watched)} + label={watched ? t("player.watchedUnmark") : t("player.markWatched")} + > + {watched ? : } + + )} +
+ {/* Absent where the browser cannot fullscreen an element (Brave on an iPad): there the window-filling view is the biggest, and YouTube's own button opens the system player. */} @@ -1232,9 +1319,9 @@ export default function PlayerModal({ {isFullscreen ? : } )} - + +
)} {/* Discreet badge while YouTube's own controls have taken over (overlay yielded). */} @@ -1304,6 +1390,11 @@ export default function PlayerModal({ {/* Wide: the side panel. Its content scrolls inside a box the video's height, so a long description never makes the card taller than the video. */}
{ + panelFocusByKey.current = e.target.matches(":focus-visible"); + }} className={wide ? "relative shrink-0 border-l border-border" : undefined} style={wide ? { width: panelWidth } : undefined} > @@ -1312,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} - - - - = queue!.length - 1} - label={t("player.next")} - title={`${t("player.next")} · Shift+→`} - > - - + + + + + + + + + + )}
@@ -1377,6 +1469,10 @@ export default function PlayerModal({ variant="modal" index={index} queueLength={queue!.length} + canPrev={canPrev} + canNext={canNext} + prevReason={prevReason} + nextReason={nextReason} autoMode={autoMode} loopMode={loopMode} onPrev={goPrev} @@ -1391,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 && ( - + )} {(navigated || related) && ( @@ -1681,6 +1780,7 @@ export default function PlayerModal({ videoId={active.id} onPlay={playRelated} onWrite={onWrite} + onCleared={clearPosition} />
)} diff --git a/frontend/src/features/player/RelatedVideos.dom.test.tsx b/frontend/src/features/player/RelatedVideos.dom.test.tsx index 482562de..c67462f3 100644 --- a/frontend/src/features/player/RelatedVideos.dom.test.tsx +++ b/frontend/src/features/player/RelatedVideos.dom.test.tsx @@ -163,6 +163,15 @@ describe("RelatedVideos", () => { await waitFor(() => expect(titles()).toEqual(["Video A"])); }); + 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, "clearProgress").mockResolvedValue(undefined as never); + setup(); + fireEvent.contextMenu((await title("Video A")).closest("[data-video-id]")!); + fireEvent.click(menuItem("card.clearProgress")); + await waitFor(() => expect(reset).toHaveBeenCalledWith("A")); + }); + it("reads further with the cursor the last page gave", async () => { const asked = vi .spyOn(api, "related") diff --git a/frontend/src/features/player/RelatedVideos.tsx b/frontend/src/features/player/RelatedVideos.tsx index e412abb2..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 { 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,12 +54,15 @@ export function RelatedVideos({ videoId, onPlay, onWrite, + 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 progress was cleared from its row: the host forgets where its session left it. */ + onCleared?: (id: string) => void; }) { const { t } = useTranslation(); const qc = useQueryClient(); @@ -106,6 +109,22 @@ export function RelatedVideos({ }); }; + const onClearProgress = (id: string) => { + onCleared?.(id); + const seq = begin(id, { position_seconds: 0 }); + const write = clearVideoProgress(qc, id); + onWrite?.(write); + write + .then(() => { + settle(seq); + void qc.invalidateQueries({ queryKey: qk.feed(), refetchType: "none" }); + }) + .catch((e: unknown) => { + notifyActionError(e, msg("errors.writeFailed.videoProgress")); + settle(seq); + }); + }; + const onToggleSave = (id: string, saved: boolean) => { const seq = begin(id, { saved }); const write = writeWatchLater(qc, id, saved); @@ -231,7 +250,7 @@ export function RelatedVideos({ )} )} - + ); } diff --git a/frontend/src/features/player/playerModalControls.dom.test.tsx b/frontend/src/features/player/playerModalControls.dom.test.tsx index 41cecc0f..dce07efd 100644 --- a/frontend/src/features/player/playerModalControls.dom.test.tsx +++ b/frontend/src/features/player/playerModalControls.dom.test.tsx @@ -24,6 +24,8 @@ function mount(variant: "modal" | "docked", autoMode: AutoMode, loopMode: LoopMo variant={variant} index={1} queueLength={3} + canPrev + canNext autoMode={autoMode} loopMode={loopMode} onPrev={() => {}} @@ -58,6 +60,20 @@ describe("PlayerTransportControls", () => { expect(loop.hasAttribute("data-lit")).toBe(false); }); + // The big view's bar spoke its controls' names from a width up, and on an iPad the row outgrew the + // screen: its last control scrolled out of sight. A glyph in both places; the name is the label. + for (const variant of ["modal", "docked"] as const) + it(`${variant}: every control is its glyph alone`, () => { + mount(variant, "next", "all"); + for (const name of [ + "player.previous", + "player.next", + /^player\.autoAdvance\.label: /, + /^player\.loop\.label: /, + ]) + expect(button(name).textContent).toBe(""); + }); + 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) { diff --git a/frontend/src/features/player/playerModalControls.tsx b/frontend/src/features/player/playerModalControls.tsx index e9e62378..16f39214 100644 --- a/frontend/src/features/player/playerModalControls.tsx +++ b/frontend/src/features/player/playerModalControls.tsx @@ -3,9 +3,10 @@ import { useTranslation } from "react-i18next"; 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"; 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"; import { renderDescription } from "./descriptionLinks"; @@ -13,13 +14,32 @@ 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, 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, + * glass theme) and the big-view docked control bar (`variant:"docked"`, white-on-dark). Glyphs only + * in both: the docked bar once spoke the names from a width up, and on an iPad its row outgrew the + * screen. What a control does, its glyph and its tooltip are said once; only the look differs. + * Every control's tooltip carries its full name (and the shortcut, where it has one), so a glyph + * 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. */ @@ -27,6 +47,10 @@ export function PlayerTransportControls({ variant, index, queueLength, + canPrev, + canNext, + prevReason, + nextReason, autoMode, loopMode, onPrev, @@ -37,6 +61,14 @@ export function PlayerTransportControls({ variant: "docked" | "modal"; index: number; queueLength: number; + /** Whether there is a playable item that way (one YouTube refused is passed over). */ + canPrev: boolean; + canNext: boolean; + /** Why Previous / Next are off when items are left that way (only ones YouTube refused): the + * button stays focusable and its tooltip says so. Unset at the queue's ends, which explain + * themselves. */ + prevReason?: string; + nextReason?: string; autoMode: AutoMode; loopMode: LoopMode; onPrev: () => void; @@ -46,8 +78,9 @@ export function PlayerTransportControls({ }) { const { t } = useTranslation(); const docked = variant === "docked"; - const counter = docked ? "tabular-nums text-white/70" : "text-muted tabular-nums"; - const divider = docked ? "h-4 w-px bg-white/20" : "w-px h-4 bg-border"; + // 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 = `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 = { @@ -57,11 +90,8 @@ export function PlayerTransportControls({ title: string; onClick: () => void; disabled?: boolean; + reason?: string; 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[] = [ { @@ -70,8 +100,8 @@ export function PlayerTransportControls({ name: t("player.previous"), title: `${t("player.previous")} · Shift+←`, onClick: onPrev, - disabled: index === 0, - labelFrom: "sm", + disabled: !canPrev, + reason: canPrev ? undefined : prevReason, }, { key: "next", @@ -79,9 +109,8 @@ export function PlayerTransportControls({ name: t("player.next"), title: `${t("player.next")} · Shift+→`, onClick: onNext, - disabled: index === queueLength - 1, - labelFrom: "sm", - trailing: true, + disabled: !canNext, + reason: canNext ? undefined : nextReason, }, ]; // Persistent playback settings — cycle on click, saved to your account. @@ -94,7 +123,6 @@ export function PlayerTransportControls({ title: `${autoName} · ${t("player.autoAdvance.hint")}`, onClick: onCycleAuto, lit: autoMode !== "off", - labelFrom: "md", }, { key: "loop", @@ -103,39 +131,22 @@ export function PlayerTransportControls({ title: `${loopName} · ${t("player.loop.hint")}`, onClick: onCycleLoop, lit: loopMode !== "off", - labelFrom: "md", }, ]; - const render = (c: Control) => - docked ? ( - - ) : ( + const render = (c: Control) => ( + {c.glyph} - ); + + ); return ( <> {render(step[0]!)} 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 d32bf903..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() }, 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,6 +75,7 @@ describe("the video menu's channel-filter row", () => { { onState: vi.fn(), onToggleSave: 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 1abfeba2..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; - /** Omitted where a surface cannot reset a video's state (Home's shelves). */ - onResetState?: (id: string) => void; + /** Back to fresh: its state row and resume point both go. */ + 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"), }, - ...(onResetState && 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 2513b3c9..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": "Reset — clear watch progress/status", + "clearProgress": "Clear watch progress", "thisChannel": "This channel", "continueTitle": "Continue where you left off", "continue": "Continue", diff --git a/frontend/src/i18n/locales/en/channels.json b/frontend/src/i18n/locales/en/channels.json index 4cc834fa..2b45a4a6 100644 --- a/frontend/src/i18n/locales/en/channels.json +++ b/frontend/src/i18n/locales/en/channels.json @@ -82,6 +82,7 @@ "stored": "Stored", "subs": "Subscribers", "lastUpload": "Last upload", + "added": "Added", "length": "Length", "tags": "Tags", "actions": "Actions" 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/en/player.json b/frontend/src/i18n/locales/en/player.json index 76783093..5c6d1a52 100644 --- a/frontend/src/i18n/locales/en/player.json +++ b/frontend/src/i18n/locales/en/player.json @@ -5,6 +5,8 @@ "back": "Back", "previous": "Previous", "next": "Next", + "noPlayableBefore": "No video before this one can play here", + "noPlayableAfter": "No video after this one can play here", "autoAdvance": { "label": "Auto", "hint": "When a video ends, what plays next. Click to cycle Off, Next, Previous, Random. Saved to your account.", @@ -45,7 +47,6 @@ "channel": "Channel", "views": "{{formattedCount}} views", "stream": "stream", - "watched": "Watched", "markWatched": "Mark watched", "watchedUnmark": "Watched — click to unmark", "jumpToTime": "Jump to this time", diff --git a/frontend/src/i18n/locales/hu/card.json b/frontend/src/i18n/locales/hu/card.json index f8bd0835..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": "Alaphelyzet — nézési előzmény/státusz törlése", + "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/channels.json b/frontend/src/i18n/locales/hu/channels.json index 5ff9c159..a800caf5 100644 --- a/frontend/src/i18n/locales/hu/channels.json +++ b/frontend/src/i18n/locales/hu/channels.json @@ -82,6 +82,7 @@ "stored": "Tárolt", "subs": "Feliratkozók", "lastUpload": "Utolsó feltöltés", + "added": "Hozzáadva", "length": "Hossz", "tags": "Címkék", "actions": "Műveletek" 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/i18n/locales/hu/player.json b/frontend/src/i18n/locales/hu/player.json index c0afd43a..8172ad45 100644 --- a/frontend/src/i18n/locales/hu/player.json +++ b/frontend/src/i18n/locales/hu/player.json @@ -5,6 +5,8 @@ "back": "Vissza", "previous": "Előző", "next": "Következő", + "noPlayableBefore": "Előtte nincs itt lejátszható videó", + "noPlayableAfter": "Utána nincs itt lejátszható videó", "autoAdvance": { "label": "Automatikus", "hint": "A videó végén mi jöjjön. Kattintásra vált: Ki, Következő, Előző, Véletlen. A fiókodhoz mentve.", @@ -45,7 +47,6 @@ "channel": "Csatorna", "views": "{{formattedCount}} megtekintés", "stream": "élő adás", - "watched": "Megtekintve", "markWatched": "Megtekintettnek jelöl", "watchedUnmark": "Megtekintve — kattints a jelölés visszavonásához", "jumpToTime": "Ugrás erre az időpontra", diff --git a/frontend/src/lib/api/channels.ts b/frontend/src/lib/api/channels.ts index c3bd4b3f..7ca3c755 100644 --- a/frontend/src/lib/api/channels.ts +++ b/frontend/src/lib/api/channels.ts @@ -73,6 +73,9 @@ export interface ManagedChannel { description: string | null; stored_videos: number; last_video_at: string | null; + /** When the follow reached this list: the subscription import that first saw it, or a + * subscribe made here. */ + added_at: string; total_duration_seconds: number; priority: number; hidden: boolean; 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/lib/playerModalHooks.apiload.dom.test.tsx b/frontend/src/lib/playerModalHooks.apiload.dom.test.tsx index 41acf1af..36ee92cc 100644 --- a/frontend/src/lib/playerModalHooks.apiload.dom.test.tsx +++ b/frontend/src/lib/playerModalHooks.apiload.dom.test.tsx @@ -34,14 +34,13 @@ function mountPlayer(setPlayerError: (code: number | null) => void) { mountRef: { current: mount }, playerRef: createRef() as RefObject, currentIdRef: { current: VID }, - navRef: { current: { queue: undefined, index: 0 } }, - modeRef: { current: { autoMode: "off", loopMode: "off" } }, setWatched: () => {}, setPlayerError, setLiveData: () => {}, reapplyStored: () => {}, reclaimFocus: () => {}, advanceOnEnd: () => {}, + skipUnplayable: () => {}, sessionPosRef: { current: new Map() }, qc: new QueryClient(), }), diff --git a/frontend/src/lib/playerModalHooks.progress.dom.test.tsx b/frontend/src/lib/playerModalHooks.progress.dom.test.tsx index 0f9aaa25..17331d38 100644 --- a/frontend/src/lib/playerModalHooks.progress.dom.test.tsx +++ b/frontend/src/lib/playerModalHooks.progress.dom.test.tsx @@ -76,14 +76,13 @@ function mountPlayer(qc: QueryClient, sessionPos: Map) { mountRef: { current: mount }, playerRef: createRef() as RefObject, currentIdRef: { current: VID }, - navRef: { current: { queue: undefined, index: 0 } }, - modeRef: { current: { autoMode: "off", loopMode: "off" } }, setWatched: () => {}, setPlayerError: () => {}, setLiveData: () => {}, reapplyStored: () => {}, reclaimFocus: () => {}, advanceOnEnd: () => {}, + skipUnplayable: () => {}, sessionPosRef: { current: sessionPos }, qc, }), diff --git a/frontend/src/lib/playerModalHooks.ts b/frontend/src/lib/playerModalHooks.ts index dfe5de55..e956bc83 100644 --- a/frontend/src/lib/playerModalHooks.ts +++ b/frontend/src/lib/playerModalHooks.ts @@ -17,12 +17,11 @@ import { requestFullscreen, } from "./fullscreen"; import { useBackToExit } from "./history"; -import { api, type Video } from "./api"; +import { api } from "./api"; import { qk } from "./queryKeys"; import { patchCachedProgress, trackProgressFlush } from "./progressCache"; import type { YTNamespace, YTPlayer, YTPlayerEvent } from "./youtube"; import { logYouTubeApiFailed, logYouTubeError, logYouTubeState } from "./mediaLog"; -import type { AutoMode, LoopMode } from "./playerModes"; // How close to the end (seconds) counts as "finished" → auto-mark watched. const FINISH_MARGIN = 10; @@ -334,7 +333,7 @@ function playerTimes(p: YTPlayer | null): { cur: number; dur: number } | null { /** Create the YouTube IFrame player for the active item, resume from the saved position, checkpoint * progress every 5 s (+ a pagehide beacon), and auto-mark watched once playback reaches the end. Keyed * on activeId (a queue advance recreates the player to reuse this single-video logic). The once-bound - * event closures read fresh queue/mode/volume via the refs passed in. Mirrors Plex's usePlexProgress — + * event closures read what must stay live via the refs passed in. Mirrors Plex's usePlexProgress — * side-effect only, returns nothing. */ export function useYtProgress({ activeId, @@ -343,14 +342,13 @@ export function useYtProgress({ mountRef, playerRef, currentIdRef, - navRef, - modeRef, setWatched, setPlayerError, setLiveData, reapplyStored, reclaimFocus, advanceOnEnd, + skipUnplayable, sessionPosRef, qc, }: { @@ -361,14 +359,14 @@ export function useYtProgress({ mountRef: RefObject; playerRef: RefObject; currentIdRef: RefObject; - navRef: RefObject<{ queue: Video[] | undefined; index: number }>; - modeRef: RefObject<{ autoMode: AutoMode; loopMode: LoopMode }>; setWatched: (on: boolean) => void; setPlayerError: (code: number | null) => void; setLiveData: (d: { title?: string; author?: string } | null) => void; reapplyStored: () => void; reclaimFocus: () => void; advanceOnEnd: () => void; + /** The item turned out unplayable; `go` false once a run of them hit the cap (stay on the error). */ + skipUnplayable: (go: boolean) => void; // Positions this player session has actually STORED, per video id. Written from the server's // answer, read back by the modal's resume so stepping next→prev inside one session does not // rewind to the value the frozen launch queue was snapshotted with. @@ -388,6 +386,7 @@ export function useYtProgress({ reapplyStored, reclaimFocus, advanceOnEnd, + skipUnplayable, qc, })); @@ -403,6 +402,7 @@ export function useYtProgress({ reapplyStored, reclaimFocus, advanceOnEnd, + skipUnplayable, qc, } = itemSnapshot(); // A position this session already stored for the item beats the queue's snapshot of it. @@ -492,16 +492,12 @@ export function useYtProgress({ onError: (e: YTPlayerEvent) => { logYouTubeError(typeof e?.data === "number" ? e.data : -1); setPlayerError(typeof e?.data === "number" ? e.data : -1); - // onError (not 'ended') fires for an unplayable item, so advanceOnEnd() never runs and an - // auto-advancing queue would freeze — skip forward instead, but bounded so a run of dead - // items can't spin. We do NOT mark the broken video watched. + // onError (not 'ended') fires for an unplayable item, so advanceOnEnd() never runs and the + // queue would freeze on it — the modal skips it the way the step that reached it went, + // bounded so a run of dead items can't spin. We do NOT mark the broken video watched. if (currentIdRef.current !== id) return; // a navigated description-link error: leave the queue alone - const { autoMode: a } = modeRef.current; - const { queue: q } = navRef.current; - if (a === "off" || !q || q.length <= 1) return; errorStreakRef.current += 1; - if (errorStreakRef.current >= MAX_CONSECUTIVE_ERRORS) return; // give up; leave the overlay - advanceOnEnd(); + skipUnplayable(errorStreakRef.current < MAX_CONSECUTIVE_ERRORS); }, }, }); @@ -565,9 +561,9 @@ export function useYtProgress({ // would be the bug, not the fix. `setWatched` is the clearest case: bound to the item, it lets // this cleanup's late auto-mark land on the item being LEFT, which the modal records for that // item and never paints on the next one. What genuinely has to stay live mid-session is behind - // refs (currentIdRef, modeRef, navRef, sessionPosRef, autoMarkedRef, errorStreakRef, playerRef) + // refs (currentIdRef, sessionPosRef, autoMarkedRef, errorStreakRef, playerRef) // — and THAT split is the enforcement: a value that is not a snapshot does not belong in this // closure. (This replaced an exhaustive-deps suppression, which made the React Compiler skip // the whole hook.) - }, [activeId, mountRef, playerRef, currentIdRef, navRef, modeRef, sessionPosRef]); + }, [activeId, mountRef, playerRef, currentIdRef, sessionPosRef]); } diff --git a/frontend/src/lib/playerQueue.test.ts b/frontend/src/lib/playerQueue.test.ts index 3add3a90..486e83eb 100644 --- a/frontend/src/lib/playerQueue.test.ts +++ b/frontend/src/lib/playerQueue.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { afterRemoval } from "./playerQueue"; +import { afterRemoval, stepIndex } from "./playerQueue"; import type { Video } from "./api"; // "Delete" from inside the player (the owner asked for it: today the only way to drop a video from a @@ -10,6 +10,27 @@ const v = (id: string): Video => ({ id }) as Video; const ids = (q: Video[]) => q.map((x) => x.id); describe("afterRemoval", () => { + it("passes over a follower known dead, on the way it went", () => { + const q = [v("a"), v("b"), v("c"), v("d")]; + const next = afterRemoval(q, 1, "b", new Set(["c"])); + expect(next).toMatchObject({ playingId: "d", way: 1 }); + }); + + it("goes back over the dead ones when nothing playable followed", () => { + const q = [v("a"), v("b"), v("c")]; + expect(afterRemoval(q, 2, "c", new Set(["b"]))).toMatchObject({ playingId: "a", way: -1 }); + // The last one going, nothing dead: the one before it, and that is a step back. + expect(afterRemoval(q, 2, "c")).toMatchObject({ playingId: "b", way: -1 }); + }); + + it("with none left that can play, takes the follower all the same", () => { + const q = [v("a"), v("b"), v("c")]; + expect(afterRemoval(q, 0, "a", new Set(["b", "c"]))).toMatchObject({ + playingId: "b", + way: 1, + }); + }); + it("plays the item that followed the one you removed", () => { const q = [v("a"), v("b"), v("c")]; const next = afterRemoval(q, 1, "b"); @@ -51,3 +72,25 @@ describe("afterRemoval", () => { expect(next!.playingId).toBe("a"); }); }); + +describe("stepIndex", () => { + const q = [v("a"), v("b"), v("c"), v("d")]; + const none = new Set(); + it("steps to the neighbour that way", () => { + expect(stepIndex(q, 1, 1, none)).toBe(2); + expect(stepIndex(q, 1, -1, none)).toBe(0); + }); + it("passes over a video known dead", () => { + expect(stepIndex(q, 3, -1, new Set(["c", "b"]))).toBe(0); + expect(stepIndex(q, 0, 1, new Set(["b"]))).toBe(2); + }); + it("is -1 at the end, or when only dead videos are left that way", () => { + expect(stepIndex(q, 3, 1, none)).toBe(-1); + expect(stepIndex(q, 2, -1, new Set(["a", "b"]))).toBe(-1); + }); + it("wraps when asked, and never lands on where it started", () => { + expect(stepIndex(q, 3, 1, none, true)).toBe(0); + expect(stepIndex(q, 0, -1, new Set(["d"]), true)).toBe(2); + expect(stepIndex(q, 1, 1, new Set(["a", "c", "d"]), true)).toBe(-1); + }); +}); diff --git a/frontend/src/lib/playerQueue.ts b/frontend/src/lib/playerQueue.ts index 2eac82ce..4cf90b5a 100644 --- a/frontend/src/lib/playerQueue.ts +++ b/frontend/src/lib/playerQueue.ts @@ -12,19 +12,50 @@ export interface AfterRemoval { queue: Video[]; /** The item to play next. */ playingId: string; + /** Which way that item lies from the removed one: what it carries on along if it cannot play. */ + way: 1 | -1; } /** `null` means nothing is left — the caller closes the player. * * The next item is the one that FOLLOWED the removed one (that is what "delete and keep going" - * means when you are working down a list), clamped to the end when the last item goes. */ + * means when you are working down a list), the one before it when the last item goes. A video known + * to be unplayable (`dead`, see `stepIndex`) is passed over, not loaded again: the nearest playable + * one after, else before. None playable: the follower all the same, where the player shows why. */ export function afterRemoval( queue: Video[] | undefined, index: number, victimId: string, + dead: ReadonlySet = new Set(), ): AfterRemoval | null { const rest = (queue ?? []).filter((v) => v.id !== victimId); if (rest.length === 0) return null; - const next = rest[Math.min(Math.max(index, 0), rest.length - 1)]!; - return { queue: rest, playingId: next.id }; + // The removed item's slot: what followed it now stands there (past the end when it was the last). + const at = Math.min(Math.max(index, 0), rest.length); + const pick = (j: number, way: 1 | -1) => ({ queue: rest, playingId: rest[j]!.id, way }); + for (let j = at; j < rest.length; j++) if (!dead.has(rest[j]!.id)) return pick(j, 1); + for (let j = at - 1; j >= 0; j--) if (!dead.has(rest[j]!.id)) return pick(j, -1); + return at < rest.length ? pick(at, 1) : pick(rest.length - 1, -1); +} + +/** The nearest item from `index` going `dir` that is not known to be unplayable, or -1 when there is + * none that way. `wrap` goes round the ends (Loop "All"), never back onto `index` itself. + * + * A video YouTube refused to play (embed-blocked, removed) is remembered for the player session, so + * Previous / Next pass over it instead of loading it again only to be bounced off it. */ +export function stepIndex( + queue: readonly Video[], + index: number, + dir: 1 | -1, + dead: ReadonlySet, + wrap = false, +): number { + const n = queue.length; + for (let k = 1; k < n; k++) { + const raw = index + dir * k; + if (!wrap && (raw < 0 || raw >= n)) return -1; + const j = ((raw % n) + n) % n; + if (!dead.has(queue[j]!.id)) return j; + } + return -1; } diff --git a/frontend/src/modules/channels/AboutCell.dom.test.tsx b/frontend/src/modules/channels/AboutCell.dom.test.tsx new file mode 100644 index 00000000..889b9119 --- /dev/null +++ b/frontend/src/modules/channels/AboutCell.dom.test.tsx @@ -0,0 +1,80 @@ +// @vitest-environment jsdom +// +// The About cell's card opened the instant a pointer crossed the cell, so scrolling the table over +// a column of them popped card after card. It waits for the pointer to rest on the cell now, and a +// scroll puts an open one away — unless the keyboard's focus opened it, whose own scroll into view +// would put it away the moment it opened. +import { describe, it, expect, vi, afterEach, beforeEach } from "vitest"; +import { render, screen, fireEvent, act, cleanup } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +import AboutCell from "./AboutCell"; +import { api } from "../../lib/api"; + +beforeEach(() => { + vi.useFakeTimers(); + vi.spyOn(api, "channelDetail").mockReturnValue(new Promise(() => {})); +}); +afterEach(() => { + cleanup(); + vi.useRealTimers(); + vi.restoreAllMocks(); +}); + +function mount() { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + return screen.getByText("An element of truth"); +} +// Present in the DOM is enough: it paints once floating-ui has placed it, which a fake clock never lets happen. +const card = () => document.querySelector('[role="tooltip"]'); +const wait = (ms: number) => act(() => vi.advanceTimersByTime(ms)); +const enter = (el: HTMLElement) => + act(() => { + fireEvent.pointerEnter(el); + fireEvent.mouseEnter(el); + fireEvent.mouseMove(el); + }); + +describe("AboutCell — the hover card", () => { + it("does not open for a pointer passing over the cell", async () => { + const cell = mount(); + enter(cell); + await wait(300); + expect(card()).toBeNull(); + fireEvent.mouseLeave(cell); + await wait(600); + expect(card()).toBeNull(); + }); + + it("opens once the pointer rests on it", async () => { + const cell = mount(); + enter(cell); + await wait(550); + expect(card()).not.toBeNull(); + }); + + it("goes away when the table scrolls", async () => { + const cell = mount(); + enter(cell); + await wait(550); + expect(card()).not.toBeNull(); + act(() => void window.dispatchEvent(new Event("scroll"))); + await wait(50); + expect(card()).toBeNull(); + }); + + it("stays when the focus that opened it scrolls its cell into view", async () => { + const cell = mount(); + act(() => cell.focus()); + await wait(10); + expect(card()).not.toBeNull(); + act(() => void window.dispatchEvent(new Event("scroll"))); + await wait(50); + expect(card()).not.toBeNull(); + }); +}); diff --git a/frontend/src/modules/channels/AboutCell.tsx b/frontend/src/modules/channels/AboutCell.tsx index 955bdc57..75a41ec3 100644 --- a/frontend/src/modules/channels/AboutCell.tsx +++ b/frontend/src/modules/channels/AboutCell.tsx @@ -19,17 +19,29 @@ import { SURFACE } from "../../components/ui/surfaces"; * The card keeps the platform's default hover role (`tooltip`); making this rich, interactive panel * properly keyboard-reachable belongs to the focus/a11y work, not to the mechanical migration. */ +// How long the pointer rests on a cell before its card opens. +const ABOUT_REST_MS = 500; + export default function AboutCell({ text, channelId }: { text: string | null; channelId: string }) { const preview = (text ?? "").trim(); const [open, setOpen] = useState(false); + // Opened by the keyboard's focus: the scroll that brings a focused cell into view is that focus's + // own, and would put the card away the moment it opened — so only a pointer's card goes on a scroll. + const [byFocus, setByFocus] = useState(false); const pop = useAnchoredPopover({ open: open && !!preview, - onOpenChange: setOpen, + onOpenChange: (next, _event, reason) => { + setByFocus(next && reason === "focus"); + setOpen(next); + }, mode: "hover", offset: 6, - // Pop instantly like the old cell did; the close grace lets the cursor reach the card. - hoverDelay: { open: 0, close: 160 }, + // Opens once the pointer rests on the cell, not as it crosses one: scrolling the table under a + // still pointer, or moving down the column, used to pop card after card. The close grace lets + // the cursor reach the card; a scroll puts an open one away. + hoverDelay: { open: ABOUT_REST_MS, close: 160 }, + dismissOnScroll: !byFocus, // Cap the panel to the space between the trigger and the viewport edge (publishes --fit-max-h) so a // long About never runs its top off-screen — at high browser zoom the short viewport made a fixed // max-height overflow above the top edge, where the scroll container can't reach. diff --git a/frontend/src/modules/channels/channelColumns.dom.test.tsx b/frontend/src/modules/channels/channelColumns.dom.test.tsx index a6ebbd23..977f10b5 100644 --- a/frontend/src/modules/channels/channelColumns.dom.test.tsx +++ b/frontend/src/modules/channels/channelColumns.dom.test.tsx @@ -13,6 +13,8 @@ import { } from "./channelColumns"; import { NO_TAG_SENTINEL, type ManagedChannel, type Tag } from "../../lib/api"; import type { TFunction } from "i18next"; +import { render, screen } from "@testing-library/react"; +import { formatDate } from "../../lib/format"; const ch = (over: Partial): ManagedChannel => ({ id: "c", @@ -24,6 +26,7 @@ const ch = (over: Partial): ManagedChannel => ({ description: null, stored_videos: 0, last_video_at: null, + added_at: "2026-06-10T08:00:00+00:00", total_duration_seconds: 0, priority: 0, hidden: false, @@ -147,3 +150,39 @@ describe("buildChannelColumns — the row's end", () => { expect(cols[cols.length - 1]!.render(ch({ id: "UCz" }))).toBe("more:UCz"); }); }); + +describe("buildChannelColumns — Added", () => { + const noop = () => {}; + const cols = buildChannelColumns({ + t: ((k: string) => k) as never, + lang: "en", + prioOptions: [], + userTags: [], + bumpPriority: noop, + setPriority: noop, + more: () => null, + onView: noop, + toggleTag: noop, + createAndAttachTag: noop, + onFilterByTag: noop, + suggestionsFor: () => [], + onAcceptSuggestion: noop, + onDismissSuggestion: noop, + }); + const added = cols.find((c) => c.key === "added"); + + it("follows Last upload, and sorts by when the follow reached the list", () => { + expect(added?.header).toBe("channels.cols.added"); + const keys = cols.map((c) => c.key); + expect(keys.indexOf("added")).toBe(keys.indexOf("lastUpload") + 1); + expect(added?.sortable).toBe(true); + const early = ch({ added_at: "2026-06-10T08:00:00+00:00" }); + const late = ch({ added_at: "2026-10-12T08:00:00+00:00" }); + expect(added!.sortValue!(late) as number).toBeGreaterThan(added!.sortValue!(early) as number); + }); + + it("shows the day, not a relative time", () => { + render(<>{added!.render(ch({ added_at: "2026-10-12T08:00:00+00:00" }))}); + expect(screen.getByText(formatDate("2026-10-12T08:00:00+00:00", "en"))).toBeTruthy(); + }); +}); diff --git a/frontend/src/modules/channels/channelColumns.tsx b/frontend/src/modules/channels/channelColumns.tsx index 499f3ee3..1c8899c0 100644 --- a/frontend/src/modules/channels/channelColumns.tsx +++ b/frontend/src/modules/channels/channelColumns.tsx @@ -2,7 +2,7 @@ import type { ComponentProps, ReactNode } from "react"; import type { TFunction } from "i18next"; import { NO_TAG_SENTINEL, type ManagedChannel, type Tag } from "../../lib/api"; import { tagPickLabel, type TagList } from "../../lib/tagPicks"; -import { formatCountOrDash, formatTotalHours } from "../../lib/format"; +import { formatCountOrDash, formatDate, formatTotalHours } from "../../lib/format"; import type { Column } from "../../components/ui/DataTable"; import { PriorityStepper } from "../../components/ui/PriorityStepper"; import ChannelLink from "./ChannelLink"; @@ -204,6 +204,20 @@ export function buildChannelColumns(deps: ChannelColumnDeps): Column (c.last_video_at ? new Date(c.last_video_at).getTime() : 0), render: (c) => , }, + { + key: "added", + header: t("channels.cols.added"), + nowrap: true, + // The day, not a relative time: beside Last upload two "… ago" columns read as one, and when + // a channel was added is a fixed fact. Room for "2026. okt. 12." and a sort arrow. + width: "7.5rem", + tabletHidden: true, + sortable: true, + sortValue: (c) => new Date(c.added_at).getTime(), + render: (c) => ( + {formatDate(c.added_at, lang)} + ), + }, { key: "length", header: t("channels.cols.length"), diff --git a/frontend/src/modules/channels/useChannelActions.dom.test.tsx b/frontend/src/modules/channels/useChannelActions.dom.test.tsx index 108ccfa8..66d0bd32 100644 --- a/frontend/src/modules/channels/useChannelActions.dom.test.tsx +++ b/frontend/src/modules/channels/useChannelActions.dom.test.tsx @@ -59,6 +59,7 @@ const ch = (over: Partial = {}): ManagedChannel => ({ description: null, stored_videos: 0, last_video_at: null, + added_at: "2026-06-10T08:00:00+00:00", total_duration_seconds: 0, priority: 0, hidden: false, diff --git a/frontend/src/modules/feed/ChannelPage.actions.dom.test.tsx b/frontend/src/modules/feed/ChannelPage.actions.dom.test.tsx new file mode 100644 index 00000000..5a41055b --- /dev/null +++ b/frontend/src/modules/feed/ChannelPage.actions.dom.test.tsx @@ -0,0 +1,97 @@ +// @vitest-environment jsdom +// +// The channel header's action row on a tablet. The tags beside Subscribed got the row's leftover width, +// and on an iPad (820 px, 44 px touch targets) that was 29 px: a sliver of the tag control. Below the +// widest windows the row's two words — "Prio" before the stepper and "Subscribed" on its button — give +// their room to the tags: the stepper names itself by its tooltip, and the button keeps its name for a +// screen reader and in its tooltip. +import type { ReactNode } from "react"; +import { describe, it, expect, vi, beforeAll, afterEach } from "vitest"; +import { render, cleanup, screen, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +vi.mock("react-i18next", async (importOriginal) => ({ + ...(await importOriginal()), + useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }), +})); +vi.mock("../../features/navigation/navigationContext", () => ({ + useNavigationActions: () => ({ closeChannel: vi.fn(), openChannel: vi.fn() }), +})); +vi.mock("../../components/ui/confirmContext", () => ({ useConfirm: () => async () => true })); +vi.mock("./feedFiltersContext", () => ({ + useFeedFilters: () => ({ includeNormal: true, includeShorts: false, includeLive: false }), + useFeedFiltersActions: () => ({ setFilters: () => {} }), +})); +vi.mock("./Feed", () => ({ default: () => null })); +vi.mock("../../features/channel/ChannelAbout", () => ({ default: () => null })); +vi.mock("../../components/ui/PageShell", async (importOriginal) => ({ + ...(await importOriginal()), + PageToolbar: ({ children }: { children?: ReactNode }) => <>{children}, +})); + +import ChannelPage from "./ChannelPage"; +import { api, type ChannelDetail, type Me } from "../../lib/api"; + +beforeAll(() => { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +async function renderPage() { + vi.spyOn(api, "channelDetail").mockResolvedValue({ + id: "UCx", + title: "StarTalk", + subscribed: true, + blocked: false, + hidden: false, + priority: 0, + deep_requested: false, + backfill_done: true, + stored_videos: 10, + tag_ids: [], + } as unknown as ChannelDetail); + vi.spyOn(api, "tags").mockResolvedValue([] as never); + render( + + + , + ); + return waitFor(() => screen.getByRole("button", { name: "channel.subscribedState" })); +} + +// The word is laid out only from `xl` up: below it the element is `display: none`. +const onlyWide = (el: Element) => + el.classList.contains("hidden") && el.classList.contains("xl:inline"); + +describe("ChannelPage — the action row leaves the tags room on a tablet", () => { + it("shows Subscribed's word only on a wide window, and keeps the button's name", async () => { + const button = await renderPage(); + const word = [...button.querySelectorAll("span")].find( + (s) => s.textContent === "channel.subscribedState", + ); + expect(word).toBeTruthy(); + expect(onlyWide(word!)).toBe(true); + expect(button.getAttribute("title")).toBe("channel.unsubscribe"); + }); + + it("shows the Prio word only on a wide window; the stepper keeps its own name", async () => { + await renderPage(); + const word = screen.getByText("channels.cols.priority"); + expect(onlyWide(word)).toBe(true); + expect(screen.getByRole("group", { name: "channels.row.priorityHint" })).toBeTruthy(); + }); +}); diff --git a/frontend/src/modules/feed/ChannelPage.tsx b/frontend/src/modules/feed/ChannelPage.tsx index a68f8713..57e4895d 100644 --- a/frontend/src/modules/feed/ChannelPage.tsx +++ b/frontend/src/modules/feed/ChannelPage.tsx @@ -474,7 +474,10 @@ export default function ChannelPage({ /> {/* Unfolded, the actions share the tabs' line and the tag lane gives way ("+N"). Folded, Back and the name take that room, so at a tablet width the actions wrap onto a line of their - own rather than push the page sideways (min-w-0 let them overflow: 1072 px at 1024). */} + own rather than push the page sideways (min-w-0 let them overflow: 1072 px at 1024). + Below `xl` the row's two words ("Prio", "Subscribed") give their room to the tags: on an + iPad (820 px, 44 px touch targets) the tag lane was left 29 px, a sliver of its control. + The stepper names itself by its tooltip, the button by its aria-label and tooltip. */}
- {t("channels.cols.priority")} + + {t("channels.cols.priority")} + bumpPriority(channelId, d)} @@ -571,9 +576,10 @@ export default function ChannelPage({ onClick={onUnsubscribe} loading={unsubscribe.isPending} title={t("channel.unsubscribe")} + aria-label={t("channel.subscribedState")} icon={} > - {t("channel.subscribedState")} + {t("channel.subscribedState")} ) : ( + ))} @@ -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.toolbar.dom.test.tsx b/frontend/src/modules/feed/Feed.toolbar.dom.test.tsx new file mode 100644 index 00000000..86182a82 --- /dev/null +++ b/frontend/src/modules/feed/Feed.toolbar.dom.test.tsx @@ -0,0 +1,129 @@ +// @vitest-environment jsdom +// +// The feed's toolbar, on the feed and on a channel page. The filter pills name their group by a glyph +// (the word moved to the tooltip and the accessible name), Sort + View end the first row instead of +// taking a row of their own, and a channel page drops the count pill: its header already says how +// many videos the channel has. +import { describe, it, expect, vi, beforeAll, afterEach } from "vitest"; +import { render, screen, cleanup } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +vi.mock("react-i18next", async (importOriginal) => ({ + ...(await importOriginal()), + useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }), +})); +vi.mock("../../features/wizard/wizardContext", () => ({ + useWizard: () => ({ openWizard: vi.fn() }), +})); +vi.mock("./VirtualFeed", () => ({ default: () => null })); +vi.mock("../../features/filters/SelectedViewChip", () => ({ default: () => null })); +vi.mock("../../features/filters/ActiveFilterChips", () => ({ default: () => null })); + +import Feed from "./Feed"; +import { api, DEFAULT_TAG_MODES, type FeedFilters } from "../../lib/api"; +import { qk } from "../../lib/queryKeys"; +import { PageToolbarSlot } from "../../components/ui/pageToolbarSlot"; + +beforeAll(() => { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const FILTERS: FeedFilters = { + tags: [], + tagModes: DEFAULT_TAG_MODES, + channelIds: [], + q: "", + sort: "newest", + scope: "my", + includeNormal: true, + includeShorts: false, + includeLive: false, + show: "unwatched", +}; + +const slot = document.body.appendChild(document.createElement("div")); + +async function renderFeed(channelScoped = false) { + vi.spyOn(api, "feed").mockResolvedValue({ items: [], next_cursor: null } as never); + vi.spyOn(api, "feedSummary").mockResolvedValue({ count: 12, facets: {} } as never); + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + qc.setQueryData(qk.me(), { id: 1, can_write: true, prefs: {} }); + render( + + + {}} + view="cards" + setView={() => {}} + canRead + ytSearch="" + onYtSearch={() => {}} + onExitYtSearch={() => {}} + channelScoped={channelScoped} + /> + + , + ); + // The toolbar renders once the first page has answered. + await screen.findByTestId("feed-show"); +} + +const trigger = (name: string) => + screen.getAllByRole("button").find((b) => b.getAttribute("aria-label")?.startsWith(`${name}:`))!; +const sortTrigger = () => trigger("feed.sortLabel"); +// The group's glyph, not the trigger's own chevron: the glyph sits in its aria-hidden wrapper. +const glyph = (el: HTMLElement) => el.querySelector('[aria-hidden="true"] > svg'); + +describe("Feed toolbar", () => { + it.each([ + ["feed-show", "feed.showLabel"], + ["feed-content", "feed.contentLabel"], + ])("the %s pill shows a glyph, not its group's word, and names itself", async (id, word) => { + await renderFeed(); + const pill = screen.getByTestId(id); + expect(pill.textContent).not.toContain(word); + expect(glyph(pill)).not.toBeNull(); + expect(pill.getAttribute("title")).toBe(word); + expect(pill.getAttribute("aria-label")).toMatch(new RegExp(`^${word}: `)); + }); + + it("the sort pill shows a glyph, not the word Sort, and names itself", async () => { + await renderFeed(); + const sort = sortTrigger(); + expect(sort.textContent).not.toContain("feed.sortLabel"); + expect(glyph(sort)).not.toBeNull(); + expect(sort.getAttribute("title")).toBe("feed.sortLabel"); + }); + + it.each([false, true])( + "Sort and View end the filters' row instead of a row of their own (channel page: %s)", + async (channelScoped) => { + await renderFeed(channelScoped); + const row = screen.getByTestId("feed-show").parentElement!; + expect(row.contains(sortTrigger())).toBe(true); + expect(row.contains(trigger("feed.viewLabel"))).toBe(true); + }, + ); + + it("a channel page shows no count pill; the feed keeps it", async () => { + await renderFeed(true); + expect(screen.queryByTestId("item-count")).toBeNull(); + cleanup(); + await renderFeed(false); + expect(screen.getByTestId("item-count")).toBeTruthy(); + }); +}); diff --git a/frontend/src/modules/feed/Feed.tsx b/frontend/src/modules/feed/Feed.tsx index 82a282c9..53ab5d8c 100644 --- a/frontend/src/modules/feed/Feed.tsx +++ b/frontend/src/modules/feed/Feed.tsx @@ -9,8 +9,10 @@ import { import { qk } from "../../lib/queryKeys"; import { nextPageOnClick } from "../../lib/nextPage"; import { + ArrowDownUp, ArrowLeft, Columns2, + Eye, Grid3x3, Info, LayoutGrid, @@ -18,6 +20,7 @@ import { List, RefreshCw, Rows3, + Shapes, Trash2, type LucideIcon, } from "lucide-react"; @@ -34,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"; @@ -207,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<{ @@ -234,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 @@ -353,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); @@ -426,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]])); }); }, @@ -663,7 +662,7 @@ export default function Feed({ view={view} onState={onState} onToggleSave={onToggleSave} - onResetState={onResetState} + onClearProgress={onClearProgress} onOpen={openVideo} hasNextPage={false} isFetching={false} @@ -749,9 +748,8 @@ export default function Feed({ (k) => t("feed.sortKey." + k), ); - // Sort + view mode are tightly bound to the grid below, so they live pinned to the right of the - // applied-filter row (directly above the cards), not up in the filter toolbar. Extracted here so the - // same controls render on both the feed and channel-scoped rows. + // Sort + view mode end the toolbar's row, on the feed and on a channel page alike: a row of their + // own under the filters left most of a wide window empty. // One setter per toolbar control, shared with the page's background menu (the same choices, the // same effect, two doors). const setSort = (next: SortState) => { @@ -792,6 +790,7 @@ export default function Feed({ sort={{ key: sortKey, dir: sortDir }} onChange={setSort} label={t("feed.sortLabel")} + icon={} ascLabel={t("feed.dirAsc")} descLabel={t("feed.dirDesc")} trailing={ @@ -814,16 +813,21 @@ export default function Feed({ ); + // One row: the count, the filters, then Sort + View at its end — they wrap to a line of their own + // only when the window cannot hold them beside the filters. const toolbar = (
{/* The item count is the fixed leading element — a fixed-width pill so a changing number - never shoves the controls beside it (see ItemCount). */} - + never shoves the controls beside it (see ItemCount). A channel page has none: its header + already says how many videos the channel has. */} + {!channelScoped && ( + + )} {/* Watch state is single-select (always exactly one); content types are an - independent multi-select. Both compress to one labeled dropdown pill via FilterSelect. */} + independent multi-select. Both compress to one dropdown pill via FilterSelect, named + by a glyph: the value says enough once the group is recognisable, and the word is + in the tooltip and the accessible name. */} } testId="feed-show" options={showOptions} selected={[filters.show]} @@ -842,6 +849,7 @@ export default function Feed({ /> } testId="feed-content" multiple options={contentOptions} @@ -863,10 +871,7 @@ export default function Feed({ )} )} - {/* Folded, the band is one row: Sort + View join it on the right. */} - {fold.folded && ( -
{sortView}
- )} +
{sortView}
); @@ -944,26 +949,19 @@ export default function Feed({
{toolbar} - {/* The controls row sits directly above the grid: the applied-filter chips flow from the left - (overflowing to a "+N"/count pill so they never crowd the controls), while Sort + View stay - pinned to the right. Channel-scoped feeds show no chips (their baseline isn't a "filter you - applied"), but still get the right-pinned Sort/View. */} - {/* Wraps on a phone: the chips and Sort + View did not fit one line, and the row pushed the - page 21px wide (prod UAT 2026-09-19). Sort + View then take a line of their own. */} - {!fold.folded && ( -
- {!channelScoped && ( - <> - {/* The active personal view (if any) leads the applied-filter row. */} - - setFilters(clearedFilters(filters))} - className="flex-1" - /> - - )} -
{sortView}
+ {/* The applied-filter chips, directly above the grid (overflowing to a "+N"/count pill so + they never crowd a line). A row only while there is a chip: both render nothing when + nothing is applied, and `empty:hidden` takes the empty row's padding away with them. + Channel-scoped feeds show no chips (their baseline isn't a "filter you applied"). */} + {!fold.folded && !channelScoped && ( +
+ {/* The active personal view (if any) leads the applied-filter row. */} + + setFilters(clearedFilters(filters))} + className="flex-1" + />
)}
@@ -1002,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/files/Files.menu.dom.test.tsx b/frontend/src/modules/files/Files.menu.dom.test.tsx index 72ae6dea..94a51796 100644 --- a/frontend/src/modules/files/Files.menu.dom.test.tsx +++ b/frontend/src/modules/files/Files.menu.dom.test.tsx @@ -1390,3 +1390,79 @@ describe("Files · a file watched part way", () => { expect(await screen.findByText("playing 3")).toBeTruthy(); }); }); + +// The row is the click (and tap) surface for its file's default action, as the name always was: a +// press anywhere on it plays. A press on one of its own controls is that control's, and a click that +// ends a selection or a drag across the row opens nothing (DESIGN "Text stays selectable"). +describe("Files · a press on the row plays it", () => { + const sizeCellOf = (text: string) => + [...rowOf(text).querySelectorAll("td")].find((td) => !td.querySelector("button"))!; + + it("plays the file from a click on the row, not only on its name", async () => { + mocks.files = [file(1)]; + mocks.folders = [YT]; + mount(); + fireEvent.click(sizeCellOf("Clip 1")); + expect(await screen.findByText("playing 1")).toBeTruthy(); + }); + + it("leaves a press on the row's own control to that control", async () => { + mocks.files = [file(1)]; + mocks.folders = [YT]; + mount(); + // Share opens its own dialog; the row must not start the player under it. + const share = rowOf("Clip 1").querySelector( + 'button[aria-label="files.actions.share"]', + )!; + fireEvent.click(share); + expect(await screen.findByRole("dialog")).toBeTruthy(); + expect(screen.queryByText("playing 1")).toBeNull(); + }); + + it("opens nothing for a press that travelled across the row", async () => { + mocks.files = [file(1)]; + mocks.folders = [YT]; + mount(); + const cell = sizeCellOf("Clip 1"); + // jsdom has no PointerEvent: the press's coordinates are set on the event by hand. + const down = createEvent.pointerDown(cell); + Object.defineProperties(down, { clientX: { value: 10 }, clientY: { value: 10 } }); + fireEvent(cell, down); + fireEvent.click(cell, { clientX: 60, clientY: 10, detail: 1 }); + await act(async () => {}); + expect(screen.queryByText("playing 1")).toBeNull(); + }); + + it("opens nothing for the click on the row that ends its rename", async () => { + mocks.files = [file(1)]; + mocks.folders = [YT]; + mount(); + const cell = sizeCellOf("Clip 1"); + await openMenu(() => fireEvent.contextMenu(rowOf("Clip 1"))); + fireEvent.click(menuItem("files.actions.rename")); + const field = await screen.findByRole("textbox", { name: "files.actions.rename" }); + // A click elsewhere on the row: the press begins with the field focused, its mousedown blurs + // the field (which saves the name), and the click lands on the row after that. + fireEvent.pointerDown(cell); + act(() => field.blur()); + expect(screen.queryByRole("textbox", { name: "files.actions.rename" })).toBeNull(); + fireEvent.click(cell, { detail: 1 }); + await act(async () => {}); + expect(screen.queryByText("playing 1")).toBeNull(); + }); + + it("opens nothing for the click that ends selecting the row's words", async () => { + mocks.files = [file(1)]; + mocks.folders = [YT]; + mount(); + const cell = sizeCellOf("Clip 1"); + const range = document.createRange(); + range.selectNodeContents(rowOf("Clip 1")); + window.getSelection()!.removeAllRanges(); + window.getSelection()!.addRange(range); + fireEvent.click(cell, { detail: 1 }); + await act(async () => {}); + window.getSelection()!.removeAllRanges(); + expect(screen.queryByText("playing 1")).toBeNull(); + }); +}); diff --git a/frontend/src/modules/files/Files.tsx b/frontend/src/modules/files/Files.tsx index 4fd1310e..5c09474d 100644 --- a/frontend/src/modules/files/Files.tsx +++ b/frontend/src/modules/files/Files.tsx @@ -690,6 +690,8 @@ export default function Files() { narrowedBy={q} pageScope={pageScope} rowProps={(f) => ({ ...rowMenu.triggerProps(f), "data-file-id": f.id })} + // A press anywhere on the row does what its name does: plays, or opens the viewer. + rowOpen={openOf} // `group`: the row's quick icons show while it is hovered or holds the focus. rowClassName={(f) => ringed === f.id ? "group bg-accent/8 ring-2 ring-inset ring-accent/60" : "group" diff --git a/frontend/src/modules/home/Home.dom.test.tsx b/frontend/src/modules/home/Home.dom.test.tsx index cfb8c0e9..8f4fbf2b 100644 --- a/frontend/src/modules/home/Home.dom.test.tsx +++ b/frontend/src/modules/home/Home.dom.test.tsx @@ -46,12 +46,14 @@ vi.mock("react-i18next", async (importOriginal) => { const discover = vi.fn(); const discoverPick = vi.fn(); const videoRow = vi.fn(); +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(), + clearProgress: (id: string) => clearProgress(id), watchLaterAdd: () => Promise.resolve(), watchLaterRemove: () => Promise.resolve(), downloadIndex: () => Promise.resolve({}), // the cards' download buttons read it @@ -803,6 +805,25 @@ describe("Home", () => { expect(setPage).toHaveBeenCalledWith("plex"); }); + it("a started video's menu resets its view, and Home asks again", async () => { + discover.mockResolvedValue({ + budget: "any", + hero: null, + continue_watching: [ytCard("v1", "Half seen")], + comfort: [], + }); + mount(); + fireEvent.click((await screen.findAllByRole("button", { name: "card.menu.more" }))[0]!); + const reset = screen + .getAllByRole("menuitem", { hidden: true }) + .find((b) => b.textContent === "card.clearProgress"); + expect(reset).toBeTruthy(); + const asked = discover.mock.calls.length; + fireEvent.click(reset!); + await waitFor(() => expect(clearProgress).toHaveBeenCalledWith("v1")); + await waitFor(() => expect(discover.mock.calls.length).toBeGreaterThan(asked)); + }); + // C5: what a tile's Play would continue, its menu can start over — the one way to on touch. it("a started film's menu starts it over: the player opens from the start", async () => { const movie = { diff --git a/frontend/src/modules/home/Home.tsx b/frontend/src/modules/home/Home.tsx index 67203451..c2e80e61 100644 --- a/frontend/src/modules/home/Home.tsx +++ b/frontend/src/modules/home/Home.tsx @@ -52,7 +52,11 @@ import SegmentedToggle from "../../components/ui/SegmentedToggle"; import { LoadingState, StateMessage } from "../../components/ui/QueryState"; import VideoCard, { VideoHoverActions } from "../../features/video/VideoCard"; import { useVideoActions } from "../../features/video/useVideoActions"; -import { writeVideoState, writeWatchLater } from "../../features/video/videoWrites"; +import { + clearVideoProgress, + writeVideoState, + writeWatchLater, +} from "../../features/video/videoWrites"; import { usePlexCardActions } from "../../features/plexCards/usePlexCardActions"; import { PlexHoverBar, PlexPosterCard } from "../../features/plexCards/plexCards"; import { usePlaylistsActions } from "../playlists"; @@ -156,7 +160,15 @@ function useHomeCardMenu({ const pin = useComfortPin(menu.open); // 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 videoActions = useVideoActions({ onState, onToggleSave }, menu.open); + const qc = useQueryClient(); + // 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 onClearProgress = (id: string) => { + void clearVideoProgress(qc, id) + .then(() => qc.invalidateQueries({ queryKey: qk.discover() })) + .catch((e: unknown) => notifyActionError(e, msg("errors.writeFailed.videoProgress"))); + }; + 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 d4c3136c..746b531b 100644 --- a/frontend/src/modules/playlists/Playlists.dom.test.tsx +++ b/frontend/src/modules/playlists/Playlists.dom.test.tsx @@ -569,6 +569,29 @@ async function rowMenu(title: string, item: string) { ); } +describe("Playlists — a row's watch state", () => { + 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.clearProgress"); + await waitFor(() => expect(api.clearProgress).toHaveBeenCalledWith("B")); + }); + + 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); + await waitFor(() => + expect(screen.getAllByRole("menuitem", { hidden: true }).length).toBeGreaterThan(0), + ); + expect( + screen + .getAllByRole("menuitem", { hidden: true }) + .some((b) => b.textContent === "card.clearProgress"), + ).toBe(false); + }); +}); + describe("Playlists — a change the server refused is taken back on screen", () => { // The resync refetch returns the SAME list the server had, react-query keeps the same `data` // object for a deep-equal answer, and the page's re-seed effect is keyed on that object — so the diff --git a/frontend/src/modules/playlists/Playlists.tsx b/frontend/src/modules/playlists/Playlists.tsx index 9e92bd80..133aab48 100644 --- a/frontend/src/modules/playlists/Playlists.tsx +++ b/frontend/src/modules/playlists/Playlists.tsx @@ -3,10 +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 { hasResumePoint } from "../../lib/api/feed"; import { addPlaylistItem, movePlaylistItem, removePlaylistItem, + clearVideoProgress, writeVideoState, } from "../../features/video/videoWrites"; import { @@ -25,7 +27,7 @@ import { verticalListSortingStrategy, } from "@dnd-kit/sortable"; import { CSS } from "@dnd-kit/utilities"; -import { Check, FolderInput, History, ListPlus, Play, X } from "lucide-react"; +import { Check, Eraser, FolderInput, History, ListPlus, Play, X } from "lucide-react"; import { YouTubeIcon } from "../../lib/brandIcons"; import { api, @@ -683,6 +685,16 @@ function PlaylistDetail() { return write; } + // 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.videoProgress"))); + } + // Stable identity: PlayerModal's keydown effect depends on it, so a fresh closure // every render re-subscribed those listeners and re-ran focusModal() under the open player. const closePlayer = () => setPlaying(null); @@ -732,6 +744,16 @@ function PlaylistDetail() { onSelect: () => openPlayer(ctxRow.id), }, ...comfortPin.item(pinVideo(ctxRow.id), raw(ctxRow.title ?? "")), + ...(hasResumePoint(ctxRow) + ? [ + { + id: "clearProgress", + label: t("card.clearProgress"), + icon: , + onSelect: () => clearRowProgress(ctxRow.id), + }, + ] + : []), ...copy(ctxRow.title, ctxRow.watch_url).map((it, i) => ({ ...it, separatorBefore: i === 0, diff --git a/frontend/src/modules/plex/PlexBrowse.toolbar.dom.test.tsx b/frontend/src/modules/plex/PlexBrowse.toolbar.dom.test.tsx new file mode 100644 index 00000000..49fa3639 --- /dev/null +++ b/frontend/src/modules/plex/PlexBrowse.toolbar.dom.test.tsx @@ -0,0 +1,81 @@ +// @vitest-environment jsdom +// +// The Browse grid's toolbar, the feed's in Plex: the watch-state pill and Sort name their group by a +// glyph (the word in the tooltip and the accessible name), and Sort ends the filters' row instead of +// taking a row of its own. +import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from "vitest"; +import { render, screen, cleanup } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +vi.mock("react-i18next", async (importOriginal) => ({ + ...(await importOriginal()), + useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }), +})); +vi.mock("./plexSavedViews", async (importOriginal) => ({ + ...(await importOriginal()), + usePlexSavedViews: () => ({ views: [], displayView: null, isDirty: false, activeViewId: null }), +})); + +import PlexBrowse from "./PlexBrowse"; +import { api } from "../../lib/api"; +import { PageToolbarSlot } from "../../components/ui/pageToolbarSlot"; + +beforeAll(() => { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; +}); +beforeEach(() => window.history.replaceState({ _sub: { kind: "grid" }, _subOf: "v" }, "")); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const slot = document.body.appendChild(document.createElement("div")); + +async function mount() { + vi.spyOn(api, "plexLibrary").mockResolvedValue({ items: [], total: 0, offset: 0 } as never); + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + + + , + ); + return screen.findByTestId("plex-show"); +} + +// The group's glyph, not the trigger's own chevron: the glyph sits in its aria-hidden wrapper. +const glyph = (el: HTMLElement) => el.querySelector('[aria-hidden="true"] > svg'); +const sortTrigger = () => + screen + .getAllByRole("button") + .find((b) => b.getAttribute("aria-label")?.startsWith("plex.filter.sort:"))!; + +describe("PlexBrowse toolbar", () => { + it("the watch-state pill shows a glyph, not its group's word, and names itself", async () => { + const pill = await mount(); + // The key of a value ("plex.filter.showOpt.all") holds the group's key: test the visible text. + expect(pill.textContent).toBe("plex.filter.showOpt.all"); + expect(glyph(pill)).not.toBeNull(); + expect(pill.getAttribute("title")).toBe("plex.filter.show"); + expect(pill.getAttribute("aria-label")).toMatch(/^plex\.filter\.show: /); + }); + + it("Sort shows a glyph, names itself, and ends the filters' row", async () => { + const pill = await mount(); + const sort = sortTrigger(); + expect(sort.textContent).not.toContain("plex.filter.sort"); + expect(glyph(sort)).not.toBeNull(); + expect(sort.getAttribute("title")).toBe("plex.filter.sort"); + expect(pill.parentElement!.contains(sort)).toBe(true); + }); +}); diff --git a/frontend/src/modules/plex/PlexBrowse.tsx b/frontend/src/modules/plex/PlexBrowse.tsx index dd820682..86f31603 100644 --- a/frontend/src/modules/plex/PlexBrowse.tsx +++ b/frontend/src/modules/plex/PlexBrowse.tsx @@ -7,7 +7,7 @@ import { useQueryClient, type InfiniteData, } from "@tanstack/react-query"; -import { User, X } from "lucide-react"; +import { ArrowDownUp, Eye, User, X } from "lucide-react"; import { qk } from "../../lib/queryKeys"; import { mayReadAhead, NEXT_PAGE, nextPageOnClick } from "../../lib/nextPage"; import { healPlexField } from "../../lib/plexSearch"; @@ -556,15 +556,15 @@ function PlexBrowseView({ sub }: { sub: ReturnType ); } - // Sort is tightly bound to the grid below, so on the grid view it lives pinned to the right of the - // applied-filter chip row (directly above the cards), not on its own toolbar line — the same idiom as - // the Feed's `sortView`. The Discover/Browse toggle moved up to the header (next to the scope pill). + // Sort ends the toolbar's row, the same idiom as the Feed's `sortView`. The Discover/Browse toggle + // moved up to the header (next to the scope pill). const sortView = ( setSort(next ?? PLEX_SORT_FALLBACK)} label={t("plex.filter.sort")} + icon={} ascLabel={t("plex.filter.dir.asc")} descLabel={t("plex.filter.dir.desc")} /> @@ -634,30 +634,28 @@ function PlexBrowseView({ sub }: { sub: ReturnType while only the poster grid scrolls. */}
-
+
{/* Watch state — a top-level toolbar pill next to the count (moved out of the rail), - the same idiom as the feed's Watch-state FilterSelect. */} + the same idiom as the feed's Watch-state FilterSelect, named by its glyph. */} {!fold.folded && ( } testId="plex-show" options={showOptions} selected={[show]} onChange={(next) => setShow(next[0] ?? "all")} /> )} - {/* Folded, the band is one row: Sort + the view toggle join it on the right. */} - {fold.folded && ( -
{sortView}
- )} +
{sortView}
- {/* The controls row sits directly above the grid: the applied-filter chips flow from the - left (overflowing to a "+N"/count pill so they never crowd the controls), while Sort + - the Discover/Browse toggle stay pinned to the right — the same layout as the Feed. */} + {/* The applied-filter chips, directly above the grid (overflowing to a "+N"/count pill so + they never crowd a line) — a row only while there is a chip: both render nothing when + nothing is applied, and `empty:hidden` takes the empty row's margin away with them. */} {!fold.folded && ( -
+
{/* The active personal view (if any) leads the applied-filter row, like the feed. */} onClearAll={clearAllFilters} className="flex-1" /> -
{sortView}
)}