From b3402d158fa8584656bb24797a2b9e4f3fca5f8f Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 1 Nov 2026 17:12:05 +0100 Subject: [PATCH] fix(player): the card under the big view is out of the Tab order, and the focus leaves it and comes back --- .../components/ui/dialogFocus.dom.test.tsx | 21 ++++++- frontend/src/components/ui/dialogFocus.ts | 7 ++- .../features/player/PlayerModal.dom.test.tsx | 58 +++++++++++++++++++ frontend/src/features/player/PlayerModal.tsx | 37 ++++++++++-- 4 files changed, 115 insertions(+), 8 deletions(-) diff --git a/frontend/src/components/ui/dialogFocus.dom.test.tsx b/frontend/src/components/ui/dialogFocus.dom.test.tsx index 3f1d14e6..2f5ef5df 100644 --- a/frontend/src/components/ui/dialogFocus.dom.test.tsx +++ b/frontend/src/components/ui/dialogFocus.dom.test.tsx @@ -5,7 +5,7 @@ import { useRef } from "react"; import { afterEach, describe, expect, it, vi } from "vitest"; import { cleanup, render } from "@testing-library/react"; -import { useDialogChrome } from "./dialogFocus"; +import { focusablesIn, useDialogChrome } from "./dialogFocus"; afterEach(cleanup); @@ -36,6 +36,25 @@ describe("useDialogChrome · the page scroll lock", () => { }); }); +describe("focusablesIn", () => { + // A trap that counted them had its "last" control where Tab never lands, and let the focus out. + it("leaves out what an inert part holds", () => { + const offsetParent = vi + .spyOn(HTMLElement.prototype, "offsetParent", "get") + .mockReturnValue(document.body); + const { container } = render( +
+ +
+ +
+
, + ); + 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/features/player/PlayerModal.dom.test.tsx b/frontend/src/features/player/PlayerModal.dom.test.tsx index 5d008ef3..790cdc44 100644 --- a/frontend/src/features/player/PlayerModal.dom.test.tsx +++ b/frontend/src/features/player/PlayerModal.dom.test.tsx @@ -1699,6 +1699,64 @@ describe("PlayerModal — the docked bar's fullscreen", () => { }); }); +// The big view covers the card, and its controls stayed in the Tab order under it: past the bar's +// last button the focus went on, unseen, to the card's Previous, Download and Remove. +describe("PlayerModal — the card under the big view", () => { + const open = () => { + vi.spyOn(api, "videoDetail").mockReturnValue(new Promise(() => {})); + 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(); + act(() => enter.focus()); + const matches = vi.spyOn(enter, "matches").mockReturnValue(false); + fireEvent.click(enter); + matches.mockRestore(); + const card = screen.getByRole("dialog").querySelector("[tabindex='-1']"); + expect(document.activeElement).toBe(card); + fireEvent.click(screen.getByLabelText("player.exitMaximize")); + expect(document.activeElement).toBe(card); + }); + + 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 // not the player, the scissors gets the focus back; in the maximised view the panel takes the docked // bar's place; a crop frame lies over the video and Escape turns it off before the panel closes. diff --git a/frontend/src/features/player/PlayerModal.tsx b/frontend/src/features/player/PlayerModal.tsx index d5da4bb0..8e4ffa8f 100644 --- a/frontend/src/features/player/PlayerModal.tsx +++ b/frontend/src/features/player/PlayerModal.tsx @@ -450,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. @@ -533,14 +534,37 @@ 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. A focus the view's end took with it (the bar's + // controls go with the bar) goes back to the card. After the clip panel's move above, which keeps + // its own focus. + const cardPanelRef = useRef(null); + const bigExitRef = useRef(null); + const focusBeforeBig = useRef(null); + useLayoutEffect(() => { + if (bigView) { + const active = document.activeElement; + if (!(active instanceof HTMLElement) || !cardPanelRef.current?.contains(active)) return; + const byKey = active.matches(":focus-visible"); + focusBeforeBig.current = byKey ? active : null; + if (byKey && bigExitRef.current) bigExitRef.current.focus(); + else focusModal(); + return; + } + const back = focusBeforeBig.current; + focusBeforeBig.current = null; + if (back?.isConnected) back.focus(); + else 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. */ @@ -1151,7 +1175,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 gap-x-3 border-t border-white/10 bg-media-chrome/95 media-marks px-3 text-sm text-white" @@ -1287,6 +1311,7 @@ export default function PlayerModal({ )} { e.stopPropagation(); @@ -1356,6 +1381,8 @@ 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. */}