From b5045165c0f2f619b1606a40f9093e9399420369 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 25 Oct 2026 00:32:28 +0200 Subject: [PATCH] fix(player): a related video played is tracked, and its list is honest about its state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A related video played in place is the player's item now: its title and its own watched / playlist / download controls, its progress checkpointed and resumed, marked watched at its end — and its end does not step the queue. Back returns to the opened video, where it was left. The related list: - its marks announce through the shared notice (the Notification Center finds them), join the host's player session (the feed refetches on close), and stay per write, so an earlier answer no longer undoes a later mark still on its way; - every host's state write patches the cached related lists, which are never refetched; - kept for the session (gcTime), a retry asks for the page that failed, a list read so far with nothing unseen says so, a refusal a retry cannot answer offers none, and the followed mark names itself on hover. Turned off on the instance (a limit of 0), the tab, the drawer button and the live-search buttons are left out. Keys: the player's keys and its focus reclaim defer to a layer above it (the drawer); going wide moves the drawer's list into the panel's tab; `j` goes on from the slot of a row removed inside the top layer; the help sheet lists the list keys over a layer that holds a list. --- docs/DESIGN.md | 12 +- .../components/ui/KeyboardHelp.dom.test.tsx | 37 +++- frontend/src/components/ui/KeyboardHelp.tsx | 12 +- .../features/player/PlayerModal.dom.test.tsx | 193 +++++++++++++++++- frontend/src/features/player/PlayerModal.tsx | 162 ++++++++++----- .../player/RelatedVideos.dom.test.tsx | 150 +++++++++++++- .../src/features/player/RelatedVideos.tsx | 136 ++++++------ frontend/src/features/video/stateNotice.ts | 38 ++++ frontend/src/i18n/locales/en/player.json | 1 + frontend/src/i18n/locales/hu/player.json | 1 + frontend/src/lib/api/account.ts | 4 + frontend/src/lib/api/feed.test.ts | 13 ++ frontend/src/lib/keymap.ts | 27 ++- frontend/src/lib/layerStack.ts | 3 +- frontend/src/lib/listNav.dom.test.tsx | 45 ++++ frontend/src/lib/listNav.ts | 18 +- .../playerModalHooks.progress.dom.test.tsx | 2 +- frontend/src/lib/progressCache.ts | 18 ++ .../channels/WatchedChannels.dom.test.tsx | 37 +++- .../modules/channels/WatchedVideosDialog.tsx | 3 + .../feed/Feed.playerSession.dom.test.tsx | 67 +++++- .../modules/feed/Feed.searchOff.dom.test.tsx | 85 ++++++++ frontend/src/modules/feed/Feed.tsx | 77 +++---- .../feed/FeedHeader.searchTip.dom.test.tsx | 21 +- frontend/src/modules/feed/FeedHeader.tsx | 3 +- frontend/src/modules/home/Home.dom.test.tsx | 25 +++ frontend/src/modules/home/Home.tsx | 7 +- .../modules/playlists/Playlists.dom.test.tsx | 19 ++ frontend/src/modules/playlists/Playlists.tsx | 13 +- .../src/shell/AppCommandPalette.dom.test.tsx | 2 + 30 files changed, 1024 insertions(+), 207 deletions(-) create mode 100644 frontend/src/features/video/stateNotice.ts create mode 100644 frontend/src/modules/feed/Feed.searchOff.dom.test.tsx diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 041f49d5..3cf78d66 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -571,9 +571,14 @@ of a full-width panel and nothing on the right. spare. Wide, a "Related" tab beside the description in the side panel; stacked, a button beside the ⓘ opens them in a drawer over the player. A row is the thumbnailed video row in its narrow form (a 6rem picture, the relative time only, the "…" its one control — every toggle is a row of that menu), with - a check before the channel the user follows. A click plays it in the same player, as a description - link does (Back returns), and the list stays the opened video's. What the user watched or hid is - never in it. Over it, a two-way switch: on topic or Hungarian trending, kept on the account. + a check before the channel the user follows (named on hover). A click plays it in the same player + (Back returns to the opened video), and the list stays the opened video's. Played, it is the item it + now is (owner's pick D1 a): its title, its own watched / playlist / download controls, its progress + kept, marked watched at its end — and its end does not step the queue. What the user watched or hid + is never in it; a list read so far with nothing unseen says so above Show more. Over it, a two-way + switch: on topic or Hungarian trending, kept on the account. Turned off on the instance (a daily + limit of 0, owner's pick D2 a), there is no tab and no button; a refusal a retry cannot answer (the + day's cap, turned off, the shared budget) offers no Retry. - **Clip mode (both players, owner's picks A + P1 + a):** a scissors in the control row (Plex: the C5 row, before the ⋯; YouTube: the actions beside Download, and the docked bar) or `C` opens a clip panel IN the player; `[` / `]` mark the start / end at the playhead and open it when shut. The video being watched is the @@ -806,3 +811,4 @@ of a full-width panel and nothing on the right. | 2026-10-18 | Scheduler run history (owner D4, E1, E2 ★) | A row's last three runs as dots, newest first (interrupted: a hollow warn ring, set apart from skipped and failed), and a history button that opens the job's runs on the Activity page; a user's ad-hoc runs in their own block under the jobs, with no interval and no run button | An inline expandable history per row; a dark-grey interrupted dot; ad-hoc runs only on the Activity page | Three dots answer "did it run, did it work" at a glance; the whole history needs filters, which the Activity page has | | 2026-10-18 | Activity page (owner D1, D2, G1–G3 ★) | One page, "Napló" / "Activity": four tabs (Runs, Admin actions, Server log, Quota) under one filter bar (text, a period of 1 h · 24 h · 7 d · 30 d · 90 d defaulting to 7 days, an account), the view in the URL; runs and admin actions as dense rows that open in place as a log line does, filtered and paged on the server; a run's or a request's log lines one pivot away, as a history entry | One merged timeline; a page per source; a default period per tab; card rows; the audit log's client-side table | Each source keeps its own table and retention, read in one place; one bar keeps what you narrowed across tabs; about 1.8k runs a day need a log's density and a server-side filter | | 2026-10-24 | Related videos (owner U1–U3 ★, D1 A, D3) | On demand: a tab beside the description (wide), a drawer (stacked); the narrow thumbnailed row, its menu alone, a followed check; plays in place and the list stays the opened video's; an on-topic / Hungarian-trending switch kept on the account | Always shown; the end-of-video grid alone; a dialog; a big card or a text row; a list that follows the playing video | The modal is full on a small screen; a picture is what a choice among strangers rests on; a list that stays put is tried one by one, and spends no more of the day's cap | +| 2026-10-25 | Related: one played; off (owner D1 a, D2 a) | Played from the list, it is the player's item: its title, controls and progress, marked watched at its end, the queue left where it was; a daily limit of 0 takes the tab and the button away | A linked video's in-place play (no controls, nothing kept); a tab that only shows a refusal | What is watched should count as watched, wherever it was started from; a control that can only be refused is not offered | diff --git a/frontend/src/components/ui/KeyboardHelp.dom.test.tsx b/frontend/src/components/ui/KeyboardHelp.dom.test.tsx index 242a5f90..c7b614e4 100644 --- a/frontend/src/components/ui/KeyboardHelp.dom.test.tsx +++ b/frontend/src/components/ui/KeyboardHelp.dom.test.tsx @@ -16,7 +16,8 @@ import { } from "../../lib/keymap"; import { pushLayer, removeLayer } from "../../lib/layerStack"; import { useMediaKeys } from "../../lib/mediaKeys"; -import { useState } from "react"; +import { useRef, useState } from "react"; +import { navItemAttrs, useListNav } from "../../lib/listNav"; vi.mock("react-i18next", async (importOriginal) => { const actual = await importOriginal(); @@ -144,6 +145,40 @@ describe("KeyboardHelp", () => { expect(descriptions()).toContain("keys.help"); }); + // The player's related videos are a list inside the layer: their keys work there (`inTopLayer`), so + // the sheet opened over the player lists them — and over a layer with no list, still does not. + it("lists the list and menu keys over a layer that holds a list of its own", () => { + function LayerList() { + const ref = useRef(null); + useListNav(ref, { enabled: true }); + useContextMenu(); + return ( +
+
+
row
+
+
+ ); + } + render( + + + , + ); + const withList = pushLayer({ root: () => screen.getByTestId("layer") }); + pressH(); + expect(descriptions()).toContain("keys.listStep"); + expect(descriptions()).toContain("keys.contextMenu"); + act(() => closeKeyboardHelp()); + removeLayer(withList); + + const bare = pushLayer(); + pressH(); + removeLayer(bare); + expect(descriptions()).not.toContain("keys.listStep"); + expect(descriptions()).not.toContain("keys.contextMenu"); + }); + // Two live context menus (the Playlists page mounts one, its rail another) must not double the row. it("lists the context-menu chords once however many surfaces register", () => { function WithMenu() { diff --git a/frontend/src/components/ui/KeyboardHelp.tsx b/frontend/src/components/ui/KeyboardHelp.tsx index f62bd219..6b509ed7 100644 --- a/frontend/src/components/ui/KeyboardHelp.tsx +++ b/frontend/src/components/ui/KeyboardHelp.tsx @@ -41,11 +41,13 @@ export default function KeyboardHelp() { // Opened over a player or a dialog, the page-level keys (`s`, `g …`, `j`/`k`) are quiet — listing // them there would promise keys that do nothing until the overlay closes. And a `g` chord is listed // only for a module this account can actually reach. - // The context-menu key goes too: every `useContextMenu` is a page list, and with an overlay up the - // focus Shift+F10 needs is inside the overlay (found in UAT). - const live = overLayer - ? scopes.filter((s) => !isPageLevelScope(s) && s !== "contextMenu") - : scopes; + // The context-menu key goes too, with the page's lists. Over a layer that holds a list of its own + // (the player's related videos) the list keys and the menu key work on THAT list, so they stay — + // but not Delete: no list inside a layer takes it, and a page's remover behind does not reach. + const inLayer = (s: (typeof scopes)[number]) => + (!isPageLevelScope(s) && s !== "contextMenu") || + (overLayer === "layerWithList" && (s === "list" || s === "contextMenu")); + const live = overLayer === "page" ? scopes : scopes.filter(inLayer); const lines = helpLines(live, pages).filter((l) => !l.when || !!conditions[l.when]); const words = { Space: t("keys.cap.space"), ContextMenu: t("keys.cap.menu") }; return ( diff --git a/frontend/src/features/player/PlayerModal.dom.test.tsx b/frontend/src/features/player/PlayerModal.dom.test.tsx index 748e1b80..08bbb306 100644 --- a/frontend/src/features/player/PlayerModal.dom.test.tsx +++ b/frontend/src/features/player/PlayerModal.dom.test.tsx @@ -27,14 +27,19 @@ vi.mock("../navigation/navigationContext", () => ({ useNavigationActions: () => ({ openChannel: vi.fn() }), })); // Their own queries and popovers are not what is under test here. -vi.mock("../video/AddToPlaylist", () => ({ default: () => null })); -vi.mock("../downloads/DownloadButton", () => ({ default: () => null })); +vi.mock("../video/AddToPlaylist", () => ({ + default: ({ videoId }: { videoId: string }) => , +})); +vi.mock("../downloads/DownloadButton", () => ({ + default: ({ videoId }: { videoId: string }) => , +})); import PlayerModal from "./PlayerModal"; import { ConfirmProvider } from "../../components/ui/ConfirmProvider"; import { useConfirm } from "../../components/ui/confirmContext"; -import { api, type Video } from "../../lib/api"; +import { api, type Me, type Video } from "../../lib/api"; import { clearAll, notify } from "../../lib/notifications"; +import { qk } from "../../lib/queryKeys"; beforeAll(() => { class Stub { @@ -96,6 +101,7 @@ const vid = (id: string, position_seconds: number): Video => function renderModal(ui: ReactNode) { const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + qc.setQueryData(qk.me(), me); return render({ui}); } @@ -108,6 +114,12 @@ afterEach(() => { volumes.length = 0; }); +// The signed-in account the player reads; related videos are on unless a test turns them off. +let me = { related_enabled: true } as Me; +beforeEach(() => { + me = { related_enabled: true } as Me; +}); + const lastStart = () => started[started.length - 1]; describe("PlayerModal — where each item starts", () => { @@ -434,7 +446,7 @@ describe("PlayerModal — the watch toggle after a queue advance", () => { await settle(); // A really is marked watched — that part is correct and must keep working. - expect(onState.mock.calls).toContainEqual(["A", "watched"]); + expect(onState.mock.calls.map((c) => c.slice(0, 2))).toContainEqual(["A", "watched"]); // B is not, and its toggle says so. Before the fix both toggles read "watchedUnmark" here. expect(screen.queryAllByTitle("player.watchedUnmark")).toHaveLength(0); expect(screen.queryAllByTitle("player.markWatched").length).toBeGreaterThan(0); @@ -449,7 +461,7 @@ describe("PlayerModal — the watch toggle after a queue advance", () => { await waitFor(() => expect(lastStart()?.videoId).toBe("A")); fireEvent.click(screen.getAllByTitle("player.markWatched")[0]!); - expect(onState.mock.calls).toContainEqual(["A", "watched"]); + expect(onState.mock.calls.map((c) => c.slice(0, 2))).toContainEqual(["A", "watched"]); expect(screen.queryAllByTitle("player.watchedUnmark").length).toBeGreaterThan(0); }); }); @@ -813,13 +825,95 @@ describe("PlayerModal — the side panel on a wide window", () => { fireEvent.click(screen.getByRole("tab", { name: "player.related.title" })); fireEvent.click(titleOf(await screen.findAllByRole("link", { name: "Video rel2" }))); // In the same player, from where the user left it. - expect(loaded).toEqual([{ videoId: "rel2", startSeconds: 120 }]); + await waitFor(() => expect(lastStart()).toEqual({ videoId: "rel2", start: 120 })); // The list stays the opened video's, to try the next one; it is not asked for again. expect(screen.getAllByRole("link", { name: "Video rel1" }).length).toBeGreaterThan(0); expect(asked.mock.calls).toEqual([["A", "topical", null]]); expect(screen.getByRole("button", { name: "player.back" })).toBeTruthy(); }); + // A related video played in place is the item it now is: tracked like a queue item, while the + // queue stays where it was and Back returns to the opened one. + const playRel2 = async ( + onState: (id: string, status: string, v?: Video) => void, + queue?: Video[], + ) => { + wideWindow(true); + vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); + vi.spyOn(api, "related").mockResolvedValue({ + items: [related("rel1"), related("rel2", 120)], + has_more: false, + next_cursor: null, + }); + const A = vid("A", 40); + renderModal( + {}} + onState={onState} + />, + ); + act(() => playerEvents.onReady?.()); + fireEvent.click(screen.getByRole("tab", { name: "player.related.title" })); + fireEvent.click(titleOf(await screen.findAllByRole("link", { name: "Video rel2" }))); + await waitFor(() => expect(lastStart()?.videoId).toBe("rel2")); + }; + + it("wide: a related video played in place shows its own title and controls", async () => { + await playRel2(() => {}); + expect(screen.getByRole("heading", { name: "Video rel2" })).toBeTruthy(); + expect(screen.getAllByTestId("add-rel2").length).toBeGreaterThan(0); + expect(screen.getAllByTestId("download-rel2").length).toBeGreaterThan(0); + expect(screen.getAllByTitle("player.markWatched").length).toBeGreaterThan(0); + }); + + it("wide: a related video's progress is checkpointed, and Back resumes the opened one", async () => { + const saved = vi.spyOn(api, "saveProgress").mockImplementation(async (id: string) => ({ + video_id: id, + position_seconds: id === "rel2" ? 500 : 77, + })); + clock.current = 77; + await playRel2(() => {}); + clock.current = 500; + fireEvent.click(screen.getByRole("button", { name: "player.back" })); + // Where leaving it for the related one checkpointed it, not the queue's snapshot (40). + await waitFor(() => expect(lastStart()).toEqual({ videoId: "A", start: 77 })); + expect(saved.mock.calls.map((c) => c[0])).toContain("rel2"); + expect(screen.queryByRole("button", { name: "player.back" })).toBeNull(); + }); + + it("wide: a related video watched to its end is marked watched, and the queue stays put", async () => { + const onState = vi.fn(); + const B = vid("B", 0); + await playRel2(onState, [vid("A", 40), B]); + act(() => playerEvents.onStateChange?.({ data: 0 })); + expect(onState.mock.calls.map((c) => c.slice(0, 2))).toContainEqual(["rel2", "watched"]); + // Named for the host's notice: it is none of the host's own rows. + expect(onState.mock.calls[0]![2]).toMatchObject({ id: "rel2", title: "Video rel2" }); + await settle(); + expect(lastStart()?.videoId).toBe("rel2"); + }); + + it("leaves the related videos out when the instance turned them off", () => { + me = { related_enabled: false } as Me; + vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); + const A = vid("A", 0); + wideWindow(true); + const wideView = renderModal( + {}} onState={() => {}} />, + ); + expect(screen.queryByRole("tab", { name: "player.related.title" })).toBeNull(); + expect(screen.getByTestId("player-description")).toBeTruthy(); + wideView.unmount(); + wideWindow(false); + renderModal( + {}} onState={() => {}} />, + ); + expect(screen.queryByRole("button", { name: "player.related.title" })).toBeNull(); + }); + // The list keys are the page's, silenced while a layer is up — but this list is IN the layer. const press = (key: string) => act(() => void fireEvent.keyDown(document.activeElement ?? document.body, { key })); @@ -858,7 +952,7 @@ describe("PlayerModal — the side panel on a wide window", () => { expect(volumes).toEqual([]); // Enter on the row plays it, like its title's click. press("Enter"); - expect(loaded).toEqual([{ videoId: "rel2", startSeconds: 0 }]); + await waitFor(() => expect(lastStart()?.videoId).toBe("rel2")); }); it("stacked: the keyboard walks the list in the drawer too", async () => { @@ -902,7 +996,7 @@ describe("PlayerModal — the side panel on a wide window", () => { fireEvent.click(screen.getByRole("button", { name: "player.related.title" })); const drawer = await screen.findByRole("dialog", { name: "player.related.title" }); fireEvent.click(titleOf(await within(drawer).findAllByRole("link", { name: "Video rel1" }))); - expect(loaded).toEqual([{ videoId: "rel1", startSeconds: 0 }]); + await waitFor(() => expect(lastStart()?.videoId).toBe("rel1")); await waitFor(() => expect(screen.queryByRole("dialog", { name: "player.related.title" })).toBeNull(), ); @@ -911,6 +1005,89 @@ describe("PlayerModal — the side panel on a wide window", () => { expect(onClose).not.toHaveBeenCalled(); }); + const openDrawer = async () => { + vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); + vi.spyOn(api, "related").mockResolvedValue({ + items: [related("rel1")], + has_more: false, + next_cursor: null, + }); + const A = vid("A", 0); + renderModal( + {}} onState={() => {}} />, + ); + act(() => playerEvents.onReady?.()); + fireEvent.click(screen.getByRole("button", { name: "player.related.title" })); + const drawer = await screen.findByRole("dialog", { name: "player.related.title" }); + await within(drawer).findAllByRole("link", { name: "Video rel1" }); + return { drawer }; + }; + + it("stacked: the video's keys are the drawer's while it is open", async () => { + wideWindow(false); + const { drawer } = await openDrawer(); + act(() => within(drawer).getByRole("button", { name: "common.close" }).focus()); + press("ArrowUp"); + press("ArrowDown"); + press("ArrowLeft"); + press("ArrowRight"); + expect(volumes).toEqual([]); + expect(seeks).toEqual([]); + }); + + it("stacked: the window regaining focus leaves it in the drawer", async () => { + wideWindow(false); + const { drawer } = await openDrawer(); + const row = titleOf(within(drawer).getAllByRole("link", { name: "Video rel1" })); + act(() => row.focus()); + act(() => void window.dispatchEvent(new Event("focus"))); + expect(document.activeElement).toBe(row); + }); + + it("going wide moves the drawer's list into the panel's tab, and narrow again reopens nothing", async () => { + vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); + vi.spyOn(api, "related").mockResolvedValue({ + items: [related("rel1")], + has_more: false, + next_cursor: null, + }); + const A = vid("A", 0); + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + qc.setQueryData(qk.me(), me); + const ui = () => ( + + {}} onState={() => {}} /> + + ); + wideWindow(false); + const view = render(ui()); + fireEvent.click(screen.getByRole("button", { name: "player.related.title" })); + await screen.findByRole("dialog", { name: "player.related.title" }); + wideWindow(true); + view.rerender(ui()); + await waitFor(() => + expect(screen.queryByRole("dialog", { name: "player.related.title" })).toBeNull(), + ); + // The list the user was in is the panel's open tab now. + expect( + screen.getByRole("tab", { name: "player.related.title" }).getAttribute("aria-selected"), + ).toBe("true"); + wideWindow(false); + view.rerender(ui()); + expect(screen.queryByRole("dialog", { name: "player.related.title" })).toBeNull(); + }); + + it("wide: the panel's tabs keep the row's gap", () => { + wideWindow(true); + vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); + const A = vid("A", 0); + renderModal( + {}} onState={() => {}} />, + ); + const row = screen.getByRole("tablist").className.split(" "); + expect(row).toEqual(expect.arrayContaining(["flex", "flex-wrap", "gap-1"])); + }); + it("stacked: the card scrolls under the wheel, the video in it is still the volume", () => { wideWindow(false); vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); diff --git a/frontend/src/features/player/PlayerModal.tsx b/frontend/src/features/player/PlayerModal.tsx index 51bb6d24..9d2c0429 100644 --- a/frontend/src/features/player/PlayerModal.tsx +++ b/frontend/src/features/player/PlayerModal.tsx @@ -85,6 +85,7 @@ import { SURFACE } from "../../components/ui/surfaces"; import { IconLink, LinkButton } from "../../components/ui/LinkButton"; import { SectionLabel } from "../../components/ui/Headings"; import { useMediaQuery } from "../../lib/useMediaQuery"; +import { useMe } from "../../lib/useMe"; import Modal from "../../components/ui/Modal"; import Tabs from "../../components/ui/Tabs"; import { useTabPanel } from "../../components/ui/tabPanel"; @@ -168,6 +169,7 @@ export default function PlayerModal({ startIndex, onClose, onState, + onWrite, onRemoveFromList, }: { video: Video; @@ -180,7 +182,12 @@ export default function PlayerModal({ queue?: Video[]; startIndex?: number; onClose: () => void; - onState: (id: string, status: VideoStatus) => void; + /** The player's own marks (its watched toggle, the auto-mark at the end), for the item it is on — + * named by `video`, which may be none of the host's (a related video played in place). */ + onState: (id: string, status: VideoStatus, video?: Video) => void; + /** A write the related list made itself, for a host that refetches once the player closes and its + * writes have landed (the feed's player session). */ + onWrite?: (write: Promise) => void; /** Remove the playing video from the list it was opened from (the Playlists module passes its own * `removeItem`; the feed passes nothing — there is no list to remove it from). Returns whether it * actually happened, because the CONSEQUENCE is the player's: its launch queue is frozen, so it @@ -236,6 +243,19 @@ export default function PlayerModal({ // startAt only applies to the item we opened with; advanced items resume their own pos. const resumeAt = startAt != null && active.id === video.id ? startAt : active.position_seconds || 0; + // A related video played in place (the owner's pick) is the item it now is: its progress is + // checkpointed, it is marked watched at its end, and its own watched / playlist / download + // controls show — while the queue stays where it was, and Back returns to the opened item. It is + // the SUBJECT of the player from here down; `active` stays the queue's item, which the related + // list, the counter and prev/next belong to. A queue step leaves it. + const [related, setRelated] = useState<{ video: Video; startAt: number } | null>(null); + const [relatedOf, setRelatedOf] = useState(active.id); + if (relatedOf !== active.id) { + setRelatedOf(active.id); + setRelated(null); + } + const subject = related?.video ?? active; + const subjectResume = related ? related.startAt : resumeAt; const mountRef = useRef(null); const playerRef = useRef(null); // Keyboard/wheel controls. We keep focus on the modal card (not the cross-origin player @@ -251,11 +271,16 @@ export default function PlayerModal({ // useYtNativeControls, useYtBigView) where a fresh identity each render would spread into their // dep lists too. const focusModal = useCallback(() => cardRef.current?.focus(), []); + // The player's place on the layer stack (registered below), for what must defer to a layer above. + const layerIdRef = useRef(null); // What the player does on its own (turning ready, the pointer leaving the video, the window // regaining focus, a wheel over it, leaving fullscreen) takes the focus back only when it may have // slipped into the iframe or off the card. Taking it off a control the viewer tabbed to restarted - // their Tab from the first button. + // their Tab from the first button. Nor while a layer that owns the keyboard is open above the + // player (the related drawer, a dialog): its focus is outside the card by design, and the window + // regaining focus pulled it onto the card behind the scrim. const reclaimFocus = useCallback(() => { + if (layerIdRef.current != null && anyBlockingLayerAbove(layerIdRef.current)) return; const card = cardRef.current; const active = document.activeElement; if ( @@ -351,7 +376,7 @@ export default function PlayerModal({ // currently-playing id may differ from the active item. const [currentVideoId, setCurrentVideoId] = useState(active.id); const currentIdRef = useRef(active.id); - const navigated = currentVideoId !== active.id; + const navigated = currentVideoId !== subject.id; // Clip mode: the range being marked of the video that is playing, or null. Its panel sits under // the video, which is its preview; another video starts without one. const [clip, setClip] = useState(null); @@ -371,7 +396,7 @@ export default function PlayerModal({ // The picture the crop frame is drawn on. The iframe never tells its video's size: a short is // 9:16, anything else is taken as the player's 16:9, and the cut fits the frame to the real picture // (downloads/clip) — so a 4:3 video's frame drawn over its bars loses only what lies on the bars. - const picture = !navigated && active.is_short ? 9 / 16 : 16 / 9; + const picture = !navigated && subject.is_short ? 9 / 16 : 16 / 9; // The docked clip panel's height, measured (it wraps with the window and the text size): the video // above it ends there. const clipDockRef = useRef(null); @@ -488,7 +513,7 @@ export default function PlayerModal({ // Title/author of a navigated-to video, read from the player (free, no API call). const [liveData, setLiveData] = useState<{ title?: string; author?: string } | null>(null); // What is playing, by name: the panel's title, and the dialog's name (`titleId`) in either layout. - const heading = navigated ? (liveData?.title ?? t("player.loading")) : active.title; + const heading = navigated ? (liveData?.title ?? t("player.loading")) : subject.title; // IFrame Player API error code, when the current video can't play in the embed (e.g. an // auto-generated Topic "Art Track" with embedding disabled → 101/150, or removed/private → // 100). We overlay a friendly message + "Open on YouTube" instead of YouTube's bare error. @@ -499,9 +524,13 @@ export default function PlayerModal({ const loadVideo = useStableCallback((id: string, start: number | null) => { const p = playerRef.current; if (!p || typeof p.loadVideoById !== "function") return; - // Explicit start wins; otherwise resume the active item, start others at 0. + // Explicit start wins; otherwise resume the item the player is on, start others at 0. const startSeconds = - start != null ? start : id === active.id ? (sessionPosRef.current.get(id) ?? resumeAt) : 0; + start != null + ? start + : id === subject.id + ? (sessionPosRef.current.get(id) ?? subjectResume) + : 0; setPlayerError(null); p.loadVideoById({ videoId: id, startSeconds: startSeconds || 0 }); currentIdRef.current = id; @@ -515,18 +544,18 @@ export default function PlayerModal({ // one writer arrives late by construction: useYtProgress's cleanup for the OUTGOING item runs // after the commit that already advanced `active`, and near the end of a clip it auto-marks that // item watched (playerModalHooks.ts `maybeAutoWatch`). Keyed, that stray write is simply not this - // item's, so the toggle falls back to `active.status` and no ordering has to be argued about. + // item's, so the toggle falls back to `subject.status` and no ordering has to be argued about. // Unkeyed it landed last and painted the NEXT video as watched; a corrective click then sent // onState(next, "new") for an item that was already new. const [status, setStatus] = useState<{ id: string; value: VideoStatus }>({ id: active.id, value: active.status, }); - const watched = (status.id === active.id ? status.value : active.status) === "watched"; + const watched = (status.id === subject.id ? status.value : subject.status) === "watched"; const setWatched = (on: boolean) => { const next = on ? "watched" : "new"; - setStatus({ id: active.id, value: next }); - onState(active.id, next); + setStatus({ id: subject.id, value: next }); + onState(subject.id, next, subject); }; // On queue advance (active changes), point `currentVideoId` at the new item — DURING that render, @@ -537,9 +566,9 @@ export default function PlayerModal({ // The watch status is NOT synced here: it is keyed by id above, which is both stale-proof and // immune to the late writer this block cannot see (see that comment). const [syncedTo, setSyncedTo] = useState(active.id); - if (syncedTo !== active.id) { - setSyncedTo(active.id); - setCurrentVideoId(active.id); + if (syncedTo !== subject.id) { + setSyncedTo(subject.id); + setCurrentVideoId(subject.id); } // `currentIdRef` must NOT move into the block above, and the reason is EFFECT ORDERING, which the @@ -553,8 +582,8 @@ export default function PlayerModal({ // frozen launch queue's stale position — which is the exact bug the session-position map exists // to prevent (748 instead of 830, in the test's numbers). useEffect(() => { - currentIdRef.current = active.id; - }, [active.id]); + currentIdRef.current = subject.id; + }, [subject.id]); const seekTo = useStableCallback((seconds: number) => { const p = playerRef.current; @@ -646,6 +675,8 @@ export default function PlayerModal({ if (clipOpenRef.current) return; const { autoMode: a, loopMode: l } = modeRef.current; 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 wrap = l === "all"; @@ -681,10 +712,19 @@ export default function PlayerModal({ // list in place, to try the next one (the owner's pick). const [panelTab, setPanelTab] = useState<"description" | "related">("description"); const [relatedOpen, setRelatedOpen] = useState(false); + // The drawer is the stacked layout's: going wide while it is open, the list moves into the panel's + // tab instead. Left open, it reopened unasked when the window narrowed again. + if (wide && relatedOpen) { + setRelatedOpen(false); + setPanelTab("related"); + } const panelTabs = useTabPanel(); + // 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; const playRelated = (v: Video, startAt: number) => { setRelatedOpen(false); - loadVideo(v.id, startAt); + setRelated({ video: v, startAt }); }; const descPop = useAnchoredPopover({ open: showDesc, @@ -712,7 +752,6 @@ export default function PlayerModal({ // This player is a dismissable layer: register it so a Modal or popover opened ABOVE it (e.g. the // download dialog, or an anchored menu) takes Escape first and this player defers. Its root lets // the keyboard walk a list inside it (the related videos). - const layerIdRef = useRef(null); useEffect(() => { const id = pushLayer({ root: () => dialogRef.current }); layerIdRef.current = id; @@ -764,6 +803,15 @@ export default function PlayerModal({ if (dialogRef.current) trapTab(e, dialogRef.current, cardRef.current); return; } + // A layer that owns the keyboard above the player (the related drawer, a menu, a dialog) has the + // keys: on its ✕ the arrows changed the volume and seeked the video behind it. Escape is the + // exception, and `close` decides for itself which layer it closes. + if ( + e.key !== "Escape" && + layerIdRef.current != null && + anyBlockingLayerAbove(layerIdRef.current) + ) + return; dispatch(e, "ytPlayer", { clip: () => { if (clip) closeClip(); @@ -838,8 +886,8 @@ export default function PlayerModal({ // the end — recreated per active item. The once-bound event closures read fresh queue/mode/volume via // the refs passed in. See lib/playerModalHooks. useYtProgress({ - activeId: active.id, - resumeAt, + activeId: subject.id, + resumeAt: subjectResume, mountRef, playerRef, currentIdRef, @@ -1051,9 +1099,9 @@ export default function PlayerModal({ {/* 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 && } {!navigated && ( - + )} {/* Clip mode here: the clip panel takes this bar's place. */} {playerError == null && ( @@ -1266,7 +1314,7 @@ export default function PlayerModal({ )} - {!wide && ( + {!wide && relatedOn && ( setRelatedOpen(true)} @@ -1286,14 +1334,16 @@ export default function PlayerModal({ maxWidth="max-w-sm" initialFocus="close" > - + )} - {navigated && ( + {(navigated || related) && ( + + ))} + + + ); + } + withLayout( + (_, n) => ({ left: 0, top: n * 100 }), + () => { + render(); + const layer = pushLayer({ root: () => screen.getByTestId("layer") }); + try { + act(() => screen.getByTestId("r1-hide").focus()); + press("j"); + expect(focused()).toBe("r2"); + act(() => screen.getByTestId("r2-hide").focus()); + fireEvent.click(screen.getByTestId("r2-hide")); + expect(document.activeElement).toBe(document.body); + press("j"); + expect(focused()).toBe("r3"); + } finally { + removeLayer(layer); + } + }, + ); + }); }); describe("useListNav · leaving the list, and naming the item", () => { diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index a0db0539..6d688fea 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -1,5 +1,5 @@ import { useEffect, useEffectEvent, useState, type RefObject } from "react"; -import { dispatch, useKeyScope } from "./keymap"; +import { dispatch, setListInTopLayerProbe, useKeyScope } from "./keymap"; import { anyBlockingLayer, topBlockingLayerRoot } from "./layerStack"; import { scrollParent } from "./listPaging"; import { focusIsUnclaimed } from "./useFocusKeeper"; @@ -101,6 +101,12 @@ function current(): { list: NavList; index: number; item: HTMLElement } | null { * move between the items, so a surface that also binds them (the player's seek) leaves them alone. */ export const focusInNavList = (): boolean => current() !== null; +// The help sheet lists the list keys over a layer only when this says there is a list to walk in it. +setListInTopLayerProbe(() => { + const layer = topBlockingLayerRoot(); + return !!layer && [...lists.keys()].some((el) => layer.contains(el)); +}); + /** The mounted items that belong to `el` itself (not to a list nested inside it), in index order. */ function ownItems(el: HTMLElement): HTMLElement[] { return [...el.querySelectorAll(`[${INDEX}]`)] @@ -335,10 +341,16 @@ function onKey(e: KeyboardEvent): void { if (move(dx, dy)) ev.preventDefault(); }; // The list keys act on the page, so an open layer silences them — unless the focus is in a list - // INSIDE the topmost layer: that list is what the user is in (the player's related videos). + // INSIDE the topmost layer: that list is what the user is in (the player's related videos). Or was, + // a moment ago: a row marked from its menu leaves the list and the focus falls to , and `j` + // goes on from its slot (`slotOfRemoved`) as it does on the page. const layer = topBlockingLayerRoot(); const cur = current(); - const inTopLayer = !!layer && !!cur && layer.contains(cur.item); + const inTopLayer = + !!layer && + (cur + ? layer.contains(cur.item) + : focusIsUnclaimed(document.activeElement) && !!last && layer.contains(last.list.el)); dispatch( e, "list", diff --git a/frontend/src/lib/playerModalHooks.progress.dom.test.tsx b/frontend/src/lib/playerModalHooks.progress.dom.test.tsx index d4043ad0..6fcc56e1 100644 --- a/frontend/src/lib/playerModalHooks.progress.dom.test.tsx +++ b/frontend/src/lib/playerModalHooks.progress.dom.test.tsx @@ -186,7 +186,7 @@ describe("patchCachedProgress", () => { expect(after.pages[0]?.items[0]?.position_seconds).toBe(830); }); - it("patches a related list too — a row played from it resumes where it was left", () => { + it("patches a related list too — its rows open the same player", () => { const qc = new QueryClient(); const key = qk.related("SRC", "topical"); qc.setQueryData(key, { pages: [{ items: [video()], next_cursor: null }], pageParams: [null] }); diff --git a/frontend/src/lib/progressCache.ts b/frontend/src/lib/progressCache.ts index 673bbf99..46bc1351 100644 --- a/frontend/src/lib/progressCache.ts +++ b/frontend/src/lib/progressCache.ts @@ -84,3 +84,21 @@ export function patchCachedProgress(qc: QueryClient, positions: Map({ queryKey: qk.ytSearch() }, patch); qc.setQueriesData({ queryKey: qk.related() }, patch); } + +/** Write one video's change (its status, its Watch-later mark) into every cached related list — + * both regions', every opened video's. Those lists are never refetched (another scrape and a slot + * of the day's cap), and they leave out what the user has seen: a video marked anywhere else — a + * feed card, a playlist, the player itself — still showed in one opened again later. So every + * state write calls it once the server has the change. */ +export function patchRelated(qc: QueryClient, id: string, change: Partial