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