fix(plex): the player's ⋯ and tracks lists are real menus, and every close returns the focus

They were hand-built divs: no menu roles, no arrow keys, and Escape left the
focus on <body>. Both run on the popover engine now (media-tone rows), and the
tuning panel is a labelled dialog that takes the focus and hands it back to ⋯.
This commit is contained in:
2026-09-27 14:22:37 +02:00
parent 9bedd96774
commit a975ab627a
8 changed files with 224 additions and 90 deletions
@@ -183,6 +183,17 @@ function pressPicture(pointerType: "touch" | "mouse", x = 450) {
fireEvent.click(v, { clientX: x, clientY: 250 });
}
// A menu's rows are named by their text: the surface is `visibility: hidden` until floating-ui has
// measured it (never, under jsdom), and a hidden element's accessible name is empty.
const menuItem = (label: string) =>
waitFor(() => {
const row = screen
.queryAllByRole("menuitem", { hidden: true })
.find((b) => b.textContent === label);
expect(row).toBeDefined();
return row!;
});
describe("a tap on the picture (C3)", () => {
it("a mouse click plays or pauses; a finger's tap only shows or hides the controls", async () => {
mount({ details: { A: item("A") } });
@@ -277,13 +288,58 @@ describe("the C5 row", () => {
"plex.player.help",
"plex.player.download",
];
for (const r of rows) expect(screen.getByRole("button", { name: r }), r).toBeTruthy();
const menu = await screen.findByRole("menu", { hidden: true });
expect(
[...menu.querySelectorAll('[role="menuitem"]')].map(
(b) => b.getAttribute("aria-label") ?? b.textContent,
),
).toEqual(rows);
// A row closes the menu and opens its tool: Settings → the tuning panel.
fireEvent.click(screen.getByRole("button", { name: "plex.player.settings" }));
expect(screen.queryByRole("button", { name: "plex.player.stats.title" })).toBeNull();
fireEvent.click(await menuItem("plex.player.settings"));
await screen.findByRole("dialog", { name: "plex.player.settings" });
expect(
screen
.queryAllByRole("menuitem", { hidden: true })
.find((b) => b.textContent === "plex.player.stats.title") ?? null,
).toBeNull();
expect(screen.getByLabelText("plex.player.more").getAttribute("aria-expanded")).toBe("true");
});
// E28 review D3: the ⋯ was a hand-built <div> of buttons — no menu role, no arrow keys, and Escape
// closed it with the focus left on <body>. It is a menu on the popover engine now.
it("walks the ⋯ with the arrows, and Escape gives the focus back to the ⋯", async () => {
mount({ details: { A: item("A") } });
await playingAt("/media/A", 100);
const more = screen.getByLabelText("plex.player.more");
more.focus();
fireEvent.click(more);
const menu = await screen.findByRole("menu", { hidden: true });
const rows = [...menu.querySelectorAll<HTMLElement>('[role="menuitem"]')];
await waitFor(() => expect(document.activeElement).toBe(rows[0]));
fireEvent.keyDown(document.activeElement!, { key: "ArrowDown" });
expect(document.activeElement).toBe(rows[1]);
fireEvent.keyDown(document.activeElement!, { key: "End" });
expect(document.activeElement).toBe(rows[rows.length - 1]);
fireEvent.keyDown(document.activeElement!, { key: "Escape" });
await waitFor(() => expect(screen.queryByRole("menu")).toBeNull());
await waitFor(() => expect(document.activeElement).toBe(more));
});
it("gives the focus back to the ⋯ when the tuning panel closes from inside", async () => {
mount({ details: { A: item("A") } });
await playingAt("/media/A", 100);
const more = screen.getByLabelText("plex.player.more");
fireEvent.click(more);
fireEvent.click(await menuItem("plex.player.settings"));
const panel = await screen.findByRole("dialog", { name: "plex.player.settings" });
// Into the panel on open…
await waitFor(() => expect(panel.contains(document.activeElement)).toBe(true));
fireEvent.keyDown(document.activeElement!, { key: "Escape" });
await waitFor(() => expect(screen.queryByRole("dialog")).toBeNull());
// …and back on the ⋯ when it goes, not on <body>.
await waitFor(() => expect(document.activeElement).toBe(more));
});
it("Next shows only when there is a next item", async () => {
mount({ details: { A: item("A") } });
await playingAt("/media/A", 100);
@@ -261,9 +261,9 @@ describe("the subtitle poll and notice", () => {
await screen.findByText("Title A");
fireEvent.click(screen.getByLabelText("plex.player.tracks"));
const off = await screen.findByText("plex.player.subOff");
expect(off.closest("button")?.getAttribute("aria-pressed")).toBe("false");
const pressed = document.querySelectorAll('[aria-pressed="true"]');
expect([...pressed].map((b) => b.textContent)).toEqual(["English"]); // the saved subtitle
expect(off.closest('[role="menuitemradio"]')?.getAttribute("aria-checked")).toBe("false");
const checked = document.querySelectorAll('[role="menuitemradio"][aria-checked="true"]');
expect([...checked].map((b) => b.textContent)).toEqual(["English"]); // the saved subtitle
});
});
@@ -8,7 +8,7 @@
// the round-3 comment that handed the step prompt's trap to "the dialog below" assumed this one did
// too. Its own file because `layerStack` is module state and these tests push into it.
import { describe, it, expect, vi, beforeAll, afterEach } from "vitest";
import { render, cleanup, waitFor, act } from "@testing-library/react";
import { render, cleanup, waitFor, act, screen } from "@testing-library/react";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
vi.mock("react-i18next", async (importOriginal) => ({
@@ -116,6 +116,17 @@ const press = (key: string, shiftKey = false) =>
);
});
// A menu's rows are named by their text: the surface is `visibility: hidden` until floating-ui has
// measured it (never, under jsdom), and a hidden element's accessible name is empty.
const menuItem = (label: string) =>
waitFor(() => {
const row = screen
.queryAllByRole("menuitem", { hidden: true })
.find((b) => b.textContent === label);
expect(row).toBeDefined();
return row!;
});
describe("PlexPlayer · Tab stays inside the player", () => {
it("pulls the focus back in when it is on the page behind", async () => {
await mount();
@@ -160,9 +171,10 @@ describe("PlexPlayer · Tab stays inside the player", () => {
const more = buttons().find((b) => b.getAttribute("aria-label") === "plex.player.more");
expect(more).toBeDefined();
act(() => more!.click());
const settings = buttons().find((b) => b.textContent === "plex.player.settings");
expect(settings).toBeDefined();
act(() => settings!.click());
// The ⋯ is a menu on the popover engine: it portals out of the player and traps for itself.
const settings = await menuItem("plex.player.settings");
act(() => settings.click());
await screen.findByRole("dialog", { name: "plex.player.settings" });
const items = buttons();
// The panel really opened — the trap now has to span its controls too.
expect(items.length).toBeGreaterThan(before + 3);
+107 -72
View File
@@ -82,7 +82,8 @@ import {
} from "../lib/layerStack";
import { Button } from "./ui/Button";
import { IconButton } from "./ui/IconButton";
import { Pressable } from "./ui/Pressable";
import { PopoverItem, PopoverSurface } from "./ui/AnchoredPopover";
import { closeThenAct, useAnchoredPopover } from "./ui/anchoredPopoverHooks";
import { Slider } from "./ui/NativeControls";
import { formatUnit } from "../lib/format";
@@ -104,6 +105,10 @@ const RECLAIM_WINDOW_MS = 120_000;
const BACK_TO_PREV_S = 3;
// C5: a touch double-tap on the picture's left or right third skips this far; each further tap
// inside the window skips again (the keyboard keeps ← / → at the viewer's own step).
// The player's menus sit on the always-dark media chrome, under white text, in every theme.
const PLAYER_MENU =
"rounded-xl border border-white/15 bg-media-chrome/95 text-sm text-white shadow-2xl";
const PLAYER_ROW = "block w-full rounded-lg px-2 py-1.5 text-left hover:bg-white/10";
const TAP_SKIP_S = 10;
const DOUBLE_TAP_MS = 300;
@@ -236,7 +241,6 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
const [menuOpen, setMenuOpen] = useState(false); // the tuning panel (tabbed), from ⋯ → Settings
// The fixed ⋯ (C5): the rare tools — settings, media info, stats, shortcuts, the original file.
const [moreOpen, setMoreOpen] = useState(false);
const moreRef = useRef<HTMLDivElement>(null);
const [tracksOpen, setTracksOpen] = useState(false); // separate quick menu = audio/subtitle tracks
const [settingsTab, setSettingsTab] = useState<"sync" | "playback" | "subtitle" | "clock">(
"sync",
@@ -244,9 +248,7 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
const audioRef = useRef<number | null>(null);
const menuOpenRef = useRef(false);
const menuRef = useRef<HTMLDivElement>(null);
const menuTriggerRef = useRef<HTMLButtonElement>(null);
const tracksRef = useRef<HTMLDivElement | null>(null);
const tracksTriggerRef = useRef<HTMLButtonElement>(null);
const menuTriggerRef = useRef<HTMLButtonElement | null>(null);
menuOpenRef.current = menuOpen || tracksOpen || moreOpen; // keep the control bar up while one is open
// Auto-hiding control bar (reveal on mouse move / any key; hide after 3s while playing).
const {
@@ -265,21 +267,40 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
onPointerEnter: () => holdOnHover(true),
onPointerLeave: () => holdOnHover(false),
};
// Each menu stays open until a click/Escape OUTSIDE it (so sliders can be dragged freely).
// The tuning panel stays open until a click/Escape OUTSIDE it (so sliders can be dragged freely).
// It renders inside the player, whose Tab trap spans it; the ⋯ counts as inside, since the ⋯ is
// what closes it (below).
useDismiss(menuOpen, () => setMenuOpen(false), [menuRef, menuTriggerRef]);
useDismiss(tracksOpen, () => setTracksOpen(false), [tracksRef, tracksTriggerRef]);
useDismiss(moreOpen, () => setMoreOpen(false), [moreRef, menuTriggerRef]);
// The tracks menu is both dismiss-tracked and edge-faded, so its two refs are merged here.
// Must be stable: an inline arrow would detach (null) and re-attach the node on every render,
// and the fade's node-state ref would re-render in response — a loop. `fade.ref` is a setState.
const tracksFade = useScrollFade();
const tracksFadeRef = tracksFade.ref;
const setTracksNode = useCallback(
(node: HTMLElement | null) => {
tracksRef.current = node as HTMLDivElement | null;
tracksFadeRef(node);
// The ⋯ and the tracks list are real menus on the popover engine (E28 review D3): roving arrows,
// Home/End, Escape as a layer, and the focus back on the trigger however they close. They were
// hand-built <div>s — no roles, no arrows, and Escape left the focus on <body>. The engine portals
// them into the fullscreen element when there is one, so they show in fullscreen too.
const morePop = useAnchoredPopover({
open: moreOpen,
// The ⋯ also CLOSES the tuning panel it opened, rather than stacking the menu over it.
onOpenChange: (open) => {
if (open && menuOpen) setMenuOpen(false);
else setMoreOpen(open);
},
[tracksFadeRef],
placement: "top-end",
offset: 8,
});
const tracksPop = useAnchoredPopover({
open: tracksOpen,
onOpenChange: setTracksOpen,
placement: "top-end",
offset: 8,
fitHeight: true,
});
const tracksFade = useScrollFade();
// The ⋯ is both the menu's trigger and the tuning panel's dismiss exception and focus return.
const setMoreReference = morePop.refs.setReference;
const moreTriggerRef = useCallback(
(node: HTMLButtonElement | null) => {
menuTriggerRef.current = node;
setMoreReference(node);
},
[setMoreReference],
);
// The keyboard cheat sheet is app-wide now (ui/KeyboardHelp, opened by the global "h"), not this
// player's own overlay. Mirror its open state into a ref because usePlayerVolume still needs to
@@ -1593,74 +1614,89 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
</div>
<div className="ml-auto flex items-center gap-2">
{/* Quick TRACKS menu (audio / subtitle) — the frequent action, on its own button. */}
{/* Quick TRACKS menu (audio / subtitle) — the frequent action, on its own button. Picking
a track keeps it open: the change is seen where it was made, and the other group is
one arrow away. */}
{detail && (detail.audio_streams.length > 1 || textSubs.length > 0) && (
<div className="relative">
<>
<IconButton
variant="overlay"
size="player"
round
ref={tracksTriggerRef}
onClick={() => setTracksOpen((o) => !o)}
ref={tracksPop.refs.setReference}
{...tracksPop.getReferenceProps()}
aria-expanded={tracksOpen}
label={t("plex.player.tracks")}
>
<Languages />
</IconButton>
{tracksOpen && (
<PopoverSurface
popover={tracksPop}
ariaLabel={t("plex.player.tracks")}
className={`${PLAYER_MENU} w-56`}
>
{/* Portaled, but a React child of the player: a click or a wheel here must not
reach the stage's play-toggle and volume handlers. */}
<div
ref={setTracksNode}
style={tracksFade.style}
ref={tracksFade.ref}
onClick={(e) => e.stopPropagation()}
onWheel={(e) => e.stopPropagation()}
className="border border-white/15 bg-media-chrome/95 absolute bottom-full right-0 z-menu mb-2 w-56 max-h-[60vh] overflow-auto no-scrollbar rounded-xl p-2 text-sm text-white shadow-2xl"
style={{ ...tracksFade.style, maxHeight: "min(60dvh, var(--fit-max-h))" }}
className="overflow-auto no-scrollbar p-2"
>
{detail.audio_streams.length > 1 && (
<>
<div role="group" aria-label={t("plex.player.audio")}>
<PanelHead>{t("plex.player.audio")}</PanelHead>
{detail.audio_streams.map((a) => (
<Pressable
<PopoverItem
key={a.ord}
onClick={() => changeAudio(a.ord)}
aria-pressed={(audioOrd ?? 0) === a.ord}
className={`block w-full truncate rounded px-2 py-1 hover:bg-white/10 ${
tone="media"
role="menuitemradio"
checked={(audioOrd ?? 0) === a.ord}
label={a.label}
onSelect={() => changeAudio(a.ord)}
className={`${PLAYER_ROW} truncate ${
(audioOrd ?? 0) === a.ord ? "text-accent" : "text-white"
}`}
>
{a.label}
</Pressable>
</PopoverItem>
))}
</>
</div>
)}
{textSubs.length > 0 && (
<>
<div role="group" aria-label={t("plex.player.subtitles")}>
<PanelHead>{t("plex.player.subtitles")}</PanelHead>
<Pressable
onClick={() => changeSubtitle(null)}
aria-pressed={subOrd == null}
className={`block w-full rounded px-2 py-1 hover:bg-white/10 ${
subOrd == null ? "text-accent" : "text-white"
}`}
<PopoverItem
tone="media"
role="menuitemradio"
checked={subOrd == null}
label={t("plex.player.subOff")}
onSelect={() => changeSubtitle(null)}
className={`${PLAYER_ROW} ${subOrd == null ? "text-accent" : "text-white"}`}
>
{t("plex.player.subOff")}
</Pressable>
{textSubs.map((s) => (
<Pressable
key={s.ord}
onClick={() => changeSubtitle(s.ord)}
aria-pressed={subOrd === s.ord}
className={`block w-full truncate rounded px-2 py-1 hover:bg-white/10 ${
subOrd === s.ord ? "text-accent" : "text-white"
</PopoverItem>
{textSubs.map((sub) => (
<PopoverItem
key={sub.ord}
tone="media"
role="menuitemradio"
checked={subOrd === sub.ord}
label={sub.label}
onSelect={() => changeSubtitle(sub.ord)}
className={`${PLAYER_ROW} truncate ${
subOrd === sub.ord ? "text-accent" : "text-white"
}`}
>
{s.label}
</Pressable>
{sub.label}
</PopoverItem>
))}
</>
</div>
)}
</div>
)}
</div>
</PopoverSurface>
</>
)}
{/* The fixed ⋯ (C5): the rare tools. Settings opens the tuning panel from the same spot,
@@ -1670,40 +1706,39 @@ export default function PlexPlayer({ itemId, onClose, queue, fromStart = false }
variant="overlay"
size="player"
round
ref={menuTriggerRef}
onClick={() => (menuOpen ? setMenuOpen(false) : setMoreOpen((o) => !o))}
ref={moreTriggerRef}
{...morePop.getReferenceProps()}
aria-expanded={moreOpen || menuOpen}
label={t("plex.player.more")}
>
<MoreHorizontal />
</IconButton>
{moreOpen && (
<div
ref={moreRef}
onClick={(e) => e.stopPropagation()}
className="border border-white/15 bg-media-chrome/95 absolute bottom-full right-0 z-menu mb-2 flex w-60 flex-col rounded-xl p-1.5 text-sm text-white shadow-2xl"
>
<PopoverSurface
popover={morePop}
ariaLabel={t("plex.player.more")}
className={`${PLAYER_MENU} w-60 p-1.5`}
>
<div onClick={(e) => e.stopPropagation()} className="flex flex-col">
{moreItems.map((it) => (
<Pressable
<PopoverItem
key={it.id}
onClick={() => {
setMoreOpen(false);
// Back on the ⋯ before the tool opens: its row is about to unmount, and a
// dialog opened from here returns the focus to where it found it.
menuTriggerRef.current?.focus();
it.run();
}}
className="touch-target flex min-h-9 w-full items-center gap-2.5 rounded-lg px-2.5 text-left hover:bg-white/10"
tone="media"
label={it.label}
// Closes first, so the focus is back on the ⋯ before the tool opens: a dialog
// opened from here returns the focus to where it found it.
onSelect={() => closeThenAct(() => setMoreOpen(false), it.run)}
className={`${PLAYER_ROW} flex min-h-9 items-center gap-2.5 text-white`}
>
{it.icon}
{it.label}
</Pressable>
</PopoverItem>
))}
</div>
)}
</PopoverSurface>
{menuOpen && (
<PlexPlayerSettings
menuRef={menuRef}
returnFocusTo={menuTriggerRef}
settingsTabs={settingsTabs}
activeTab={activeTab}
setSettingsTab={setSettingsTab}
+19 -1
View File
@@ -1,4 +1,4 @@
import { type RefObject } from "react";
import { useLayoutEffect, type RefObject } from "react";
import { useTranslation } from "react-i18next";
import { Slider, Toggle } from "./plexPlayerControls";
@@ -18,6 +18,9 @@ export function PlexPlayerSettings(props: {
// which is the honest shape — the element is null until the div mounts. The old `RefObject<T>` lied
// about that and 18's types let it.
menuRef: RefObject<HTMLDivElement | null>;
/** The ⋯ that opened the panel: the focus goes back to it when the panel closes with the focus
* inside — by Escape, Back or the ⋯ itself — instead of falling to <body> (E28 review D3). */
returnFocusTo: RefObject<HTMLElement | null>;
settingsTabs: { key: SettingsTab; label: string }[];
activeTab: SettingsTab;
setSettingsTab: (tab: SettingsTab) => void;
@@ -28,6 +31,7 @@ export function PlexPlayerSettings(props: {
}) {
const {
menuRef,
returnFocusTo,
settingsTabs,
activeTab,
setSettingsTab,
@@ -37,9 +41,23 @@ export function PlexPlayerSettings(props: {
now,
} = props;
const { t } = useTranslation();
// Into the panel on open (its first tab), and back to the ⋯ on close. The cleanup runs while the
// panel is still in the DOM, so "the focus was inside" is read before its removal drops it to
// <body>; focus the reader moved elsewhere (a click on the video) stays where they put it.
useLayoutEffect(() => {
const panel = menuRef.current;
const back = returnFocusTo.current; // the ⋯ outlives the panel: it is the panel's own trigger
panel?.querySelector<HTMLElement>("button")?.focus({ preventScroll: true });
return () => {
if (panel?.contains(document.activeElement))
queueMicrotask(() => back?.focus({ preventScroll: true }));
};
}, [menuRef, returnFocusTo]);
return (
<div
ref={menuRef}
role="dialog"
aria-label={t("plex.player.settings")}
onWheel={(e) => e.stopPropagation()}
onClick={(e) => e.stopPropagation()}
className="border border-white/15 bg-media-chrome/95 absolute bottom-full right-0 z-menu mb-2 w-80 rounded-xl p-2 text-sm text-white shadow-2xl"
+14 -5
View File
@@ -104,6 +104,10 @@ const HIGHLIGHT_SELECTED =
"data-active:bg-accent/35 data-active:text-fg data-active:ring-2 data-active:ring-inset data-active:ring-accent/80";
const HIGHLIGHT_SELECTED_RING_ONLY =
"data-active:ring-2 data-active:ring-inset data-active:ring-accent/70";
// On the always-dark media chrome (a player's menus, `bg-media-chrome` under white text) the theme's
// `text-fg` is dark in the light theme: the highlighted row would go dark-on-dark. White instead.
const MEDIA_HIGHLIGHT =
"data-active:bg-white/15 data-active:text-white data-active:ring-1 data-active:ring-inset data-active:ring-white/40";
export function PopoverItem({
children,
@@ -117,6 +121,7 @@ export function PopoverItem({
elementRef,
/** Typeahead match string; defaults to the text child. Set explicitly when children aren't plain text. */
label,
tone,
...rest
}: {
children: ReactNode;
@@ -134,6 +139,8 @@ export function PopoverItem({
* aria-haspopup/aria-expanded belong on that button — not on a wrapper div around it. */
elementRef?: (node: HTMLButtonElement | null) => void;
label?: string;
/** `media`: the row sits on the always-dark media chrome (a player's menu), not a theme surface. */
tone?: "media";
} & Omit<ButtonHTMLAttributes<HTMLButtonElement>, "onSelect" | "role" | "children" | "className">) {
const ctx = useContext(PopoverListContext);
const virtual = ctx?.virtual ?? false;
@@ -212,11 +219,13 @@ export function PopoverItem({
disabled={disabled}
// touch-target: a menu row is a control primitive, 44 tall on a coarse pointer (A3).
className={`outline-hidden touch-target ${
checked || selected
? keepSelectedPaint
? HIGHLIGHT_SELECTED_RING_ONLY
: HIGHLIGHT_SELECTED
: ROVING_HIGHLIGHT
tone === "media"
? MEDIA_HIGHLIGHT
: checked || selected
? keepSelectedPaint
? HIGHLIGHT_SELECTED_RING_ONLY
: HIGHLIGHT_SELECTED
: ROVING_HIGHLIGHT
}${className ? ` ${className}` : ""}`}
>
{children}
@@ -27,7 +27,6 @@ const PLAIN: Record<string, [number, string]> = {
"PlaylistsRail.tsx": [1, "the load-error retry"],
"PlexCollectionEditor.tsx": [1, "a checkbox row in a picker"],
"PlexInfo.tsx": [1, "the rating filter (a number)"],
"PlexPlayer.tsx": [4, "track picker rows and the ⋯ menu's rows"],
"PlexPlayerSettings.tsx": [2, "setting options"],
"PlexPlaylistAdd.tsx": [1, "a checkbox row in a picker"],
"PlexPlaylistView.tsx": [1, "a show's poster stack (aria-hidden)"],
+6 -1
View File
@@ -46,8 +46,13 @@ describe("the Plex player's layers", () => {
const popovers = classes(
/\bbottom-full\b.*\bbg-media-chrome\b|\bbg-media-chrome\b.*\bbottom-full\b/,
);
expect(popovers).toHaveLength(3); // tracks, the ⋯ menu (C5), the settings panel
expect(popovers).toHaveLength(1); // the settings panel, which renders inside the player
for (const c of popovers) expect(c).toMatch(/\bz-menu\b/);
// The tracks list and the ⋯ menu are on the popover engine (E28 review D3), which lifts them to
// z-popover — above the player's own z-overlay — and into the fullscreen element.
expect([...src.matchAll(/<PopoverSurface\b[^>]*?className=\{`\$\{PLAYER_MENU\}/g)]).toHaveLength(
2,
);
});
it("keeps the notices a level below", () => {