diff --git a/frontend/src/components/ui/ActionMenu.dom.test.tsx b/frontend/src/components/ui/ActionMenu.dom.test.tsx index 732f0454..b5b54ace 100644 --- a/frontend/src/components/ui/ActionMenu.dom.test.tsx +++ b/frontend/src/components/ui/ActionMenu.dom.test.tsx @@ -130,3 +130,67 @@ describe("ActionMenu", () => { expect(screen.getByTestId("custom-trigger")).toBeTruthy(); }); }); + +// PageUp/PageDown in a roving menu (E11, owner request) — the same page as the palette's and the +// combobox's (lib/listPaging); here the keyboard's row holds real DOM focus (the `tabindex=0` rover). +describe("ActionMenu · paging", () => { + it("moves the roving focus a page at a time, over disabled rows, stopping at the ends", () => { + renderMenu( + Array.from({ length: 25 }, (_, i) => ({ + id: `r${i}`, + label: `Row ${i}`, + onSelect: vi.fn(), + disabled: i === 20, + })), + ); + fireEvent.click(screen.getByRole("button", { name: "Catalog actions" })); + const menu = screen.getByRole("menu", { hidden: true }); + const rover = () => + screen + .getAllByRole("menuitem", { hidden: true }) + .find((el) => el.getAttribute("tabindex") === "0")?.textContent; + + expect(fireEvent.keyDown(menu, { key: "PageDown" })).toBe(false); + expect(rover()).toBe("Row 10"); + fireEvent.keyDown(menu, { key: "PageDown" }); + expect(rover()).toBe("Row 21"); // Row 20 is disabled + fireEvent.keyDown(menu, { key: "PageDown" }); + expect(rover()).toBe("Row 24"); + fireEvent.keyDown(menu, { key: "PageUp" }); + expect(rover()).toBe("Row 14"); + }); + + it("pages only the submenu the key was pressed in, not the menu around it", () => { + // A submenu's surface sits inside its parent's in the React tree, so the key bubbles through the + // parent's handler too; paging both would drag the focus back out to the parent's row. + renderMenu([ + { + id: "move", + label: "Move to", + items: Array.from({ length: 25 }, (_, i) => ({ + id: `s${i}`, + label: `Sub ${i}`, + onSelect: vi.fn(), + })), + }, + ...Array.from({ length: 12 }, (_, i) => ({ + id: `p${i}`, + label: `Top ${i}`, + onSelect: vi.fn(), + })), + ]); + fireEvent.click(screen.getByRole("button", { name: "Catalog actions" })); + fireEvent.click(screen.getByText("Move to")); + const rovers = () => + screen + .getAllByRole("menuitem", { hidden: true }) + .filter((el) => el.getAttribute("tabindex") === "0") + .map((el) => el.textContent ?? ""); + const outer = () => rovers().filter((label) => !label.startsWith("Sub")); + const before = outer(); + + fireEvent.keyDown(screen.getByText("Sub 0").closest("button")!, { key: "PageDown" }); + expect(rovers()).toContain("Sub 10"); + expect(outer()).toEqual(before); + }); +}); diff --git a/frontend/src/components/ui/AnchoredPopover.tsx b/frontend/src/components/ui/AnchoredPopover.tsx index 05323d9d..1b87842f 100644 --- a/frontend/src/components/ui/AnchoredPopover.tsx +++ b/frontend/src/components/ui/AnchoredPopover.tsx @@ -3,6 +3,7 @@ import { useContext, useEffect, useId, + useMemo, useRef, useState, type ButtonHTMLAttributes, @@ -37,6 +38,7 @@ import { } from "@floating-ui/react"; import { useLayer } from "../../lib/layerStack"; import { useFullscreenPortalHost } from "../../lib/fullscreen"; +import { pageStep, pageTarget, scrollParent } from "../../lib/listPaging"; // R8 popover platform. ONE anchored-popover primitive over @floating-ui/react, replacing the ~9 // hand-rolled positioning engines (measured flip / width clamp / rAF scroll-follow / scroller-relative @@ -52,7 +54,8 @@ import { useFullscreenPortalHost } from "../../lib/fullscreen"; // // S2 (this layer) adds the keyboard/ARIA model the roles promise, mirroring `ViewSwitcher` (the house's // textbook menu) but via Floating UI's idioms rather than a second hand-rolled roving engine: -// - useListNavigation → roving focus over the registered items (↑/↓/Home/End, loop), menu/listbox only +// - useListNavigation → roving focus over the registered items (↑/↓/Home/End, loop), menu/listbox only; +// PageUp/PageDown (a page, no loop) are ours, for every list surface incl. the combobox // - useTypeahead → type-to-focus a matching item by its label // - FloatingFocusManager → focus moves into the surface on open and RETURNS to the trigger on close // (a body-portaled popover sits at the end of the document; without this, Tab can't reach it and @@ -272,7 +275,48 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { }, }); - const interactions = useInteractions([click, hover, focus, dismiss, role, listNav, typeahead]); + // PageUp/PageDown: a page of rows at a time (lib/listPaging, shared with the palette). floating-ui's + // list navigation knows only the arrows and Home/End, and the arrows alone crawl through the sidebar's + // channel filter or a Plex studio list. On BOTH elements: a combobox's keys arrive on its input, a + // menu's on the focused row inside the surface. Last in the list, so floating-ui's own keydown has + // already switched to keyboard modality — which is what makes it scroll the new row into view. A + // nested menu's surface bubbles through its parent's in the React tree; the inner one handles the + // key first and marks it, so the outer one stands down. + const onPageKey = (e: React.KeyboardEvent) => { + if ((e.key !== "PageDown" && e.key !== "PageUp") || e.defaultPrevented) return; + const rows = elementsRef.current; + const live = rows.filter((el): el is HTMLElement => el != null); + // No mounted rows — a closed surface, a hover card, a dialog — and the key keeps its own meaning. + if (!live.length) return; + e.preventDefault(); + const step = pageStep(scrollParent(live[0]!), live); + const to = pageTarget( + activeIndex ?? 0, + e.key === "PageDown" ? step : -step, + rows.length, + (i) => !!rows[i] && !rows[i]!.hasAttribute("disabled"), + ); + if (to === null) return; + setActiveIndex(to); + setHoverIndex(null); + }; + const pageKeyRef = useRef(onPageKey); + pageKeyRef.current = onPageKey; + const paging = useMemo(() => { + const onKeyDown = (e: React.KeyboardEvent) => pageKeyRef.current(e); + return { reference: { onKeyDown }, floating: { onKeyDown } }; + }, []); + + const interactions = useInteractions([ + click, + hover, + focus, + dismiss, + role, + listNav, + typeahead, + paging, + ]); // `hide()` flips this true once the trigger is scrolled out of its clipping scroller; the surface is // then painted invisible (PopoverSurface) but stays open so it can reappear on scroll-back. diff --git a/frontend/src/components/ui/Combobox.dom.test.tsx b/frontend/src/components/ui/Combobox.dom.test.tsx index 19401cc5..30dab7a6 100644 --- a/frontend/src/components/ui/Combobox.dom.test.tsx +++ b/frontend/src/components/ui/Combobox.dom.test.tsx @@ -247,3 +247,44 @@ describe("Combobox", () => { expect(screen.getByRole("combobox").getAttribute("aria-expanded")).toBe("false"); // closed }); }); + +// PageUp/PageDown (E11, owner request): the sidebar's channel filter and the Plex pickers run to +// hundreds of rows, and floating-ui's list navigation has only the arrows. jsdom has no layout, so a +// page is the fixed fallback of 10 rows; the measured page is pinned in lib/listPaging. +describe("Combobox · paging", () => { + const rows = (disabled: number[] = []) => + Array.from({ length: 25 }, (_, i) => ({ value: `Row ${i}`, disabled: disabled.includes(i) })); + const activeValue = (input: HTMLElement) => + document.getElementById(input.getAttribute("aria-activedescendant") ?? "")?.textContent; + + it("moves the active option a page at a time, stopping at both ends", () => { + wrap(); + const input = screen.getByRole("combobox"); + input.focus(); + fireEvent.focus(input); + expect(fireEvent.keyDown(input, { key: "PageDown" })).toBe(false); // the page does not scroll + expect(activeValue(input)).toBe("Row 10"); + expect(document.activeElement).toBe(input); + fireEvent.keyDown(input, { key: "PageDown" }); + fireEvent.keyDown(input, { key: "PageDown" }); + expect(activeValue(input)).toBe("Row 24"); // no wrap to the top + fireEvent.keyDown(input, { key: "PageUp" }); + expect(activeValue(input)).toBe("Row 14"); + }); + + it("never lands on a disabled option, and Enter commits where it landed", () => { + const onChange = vi.fn(); + wrap(); + const input = screen.getByRole("combobox"); + input.focus(); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: "PageDown" }); + fireEvent.keyDown(input, { key: "Enter" }); + expect(onChange).toHaveBeenCalledWith(["Row 11"]); + }); + + it("leaves the key alone while the list is closed", () => { + wrap(); + expect(fireEvent.keyDown(screen.getByRole("combobox"), { key: "PageDown" })).toBe(true); + }); +}); diff --git a/frontend/src/components/ui/CommandPalette.tsx b/frontend/src/components/ui/CommandPalette.tsx index 1caea4db..d21fddbc 100644 --- a/frontend/src/components/ui/CommandPalette.tsx +++ b/frontend/src/components/ui/CommandPalette.tsx @@ -3,6 +3,7 @@ import { useTranslation } from "react-i18next"; import { Search } from "lucide-react"; import Modal from "./Modal"; import { closeTopOverlayThenRun } from "../../lib/history"; +import { pageStep, pageTarget } from "../../lib/listPaging"; // The app's jump-to bar: one text field over every place you can go and every app-wide thing you can // do, opened with Ctrl/Cmd+K. It is the "one-layer operation" half of E10 — reaching a channel three @@ -71,24 +72,6 @@ function rankItem(item: CommandItem, groupLabel: string, words: string[]): Rank * every keystroke queues one announcement per letter. */ const ANNOUNCE_DELAY_MS = 400; -/** PageUp/PageDown step without layout to measure (jsdom, or a list not painted yet). */ -const PAGE_FALLBACK = 10; - -/** How far PageUp/PageDown move: the rows that fit in the scrolling list right now, minus one, so the - * row you paged from stays in sight as the new page's edge (a native listbox does the same). Measured - * rather than fixed: the list is 50vh tall and group headings eat into it. With 461 rows and ~11 in - * view, the arrows alone meant forty presses to cross the channel group (E11, owner request). */ -function pageStep(list: HTMLElement | null): number { - const box = list?.getBoundingClientRect(); - if (!list || !box || box.height === 0) return PAGE_FALLBACK; - let fits = 0; - for (const option of list.querySelectorAll('[role="option"]')) { - const r = option.getBoundingClientRect(); - if (r.top >= box.top - 0.5 && r.bottom <= box.bottom + 0.5) fits++; - } - return Math.max(1, fits - 1); -} - export default function CommandPalette({ open, onClose, @@ -186,14 +169,17 @@ export default function CommandPalette({ const from = Math.max(activeIndex, 0); setPicked(flat[(from + step + flat.length) % flat.length]!.id); } else if (e.key === "PageDown" || e.key === "PageUp") { - // A page at a time, and NO wrap: the arrows wrap because one step past the end is a small, - // obvious move, but a page-sized jump that lands back at the top reads as "it lost my place". - // Home/End stay the field's own (caret to the start/end of the query). + // A page at a time, stopping at the ends (lib/listPaging). The list is 50vh and group headings + // eat into it, so the page is measured. Home/End stay the field's own (caret to the query's ends). e.preventDefault(); - if (flat.length === 0) return; - const step = pageStep(listRef.current) * (e.key === "PageDown" ? 1 : -1); - const to = Math.min(flat.length - 1, Math.max(0, Math.max(activeIndex, 0) + step)); - setPicked(flat[to]!.id); + const list = listRef.current; + const step = pageStep(list, list?.querySelectorAll('[role="option"]') ?? []); + const to = pageTarget( + Math.max(activeIndex, 0), + e.key === "PageDown" ? step : -step, + flat.length, + ); + if (to !== null) setPicked(flat[to]!.id); } else if (e.key === "Enter") { const item = flat[activeIndex]; if (item) { diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index d8be132a..369ef486 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -1,5 +1,6 @@ import { useEffect, useRef, type RefObject } from "react"; import { dispatch, useKeyScope } from "./keymap"; +import { scrollParent } from "./listPaging"; // ── Keyboard navigation over lists and grids (E11 S4, UX-6) ────────────────────────────────────── // Before this, a feed card cost ~10 Tab stops (thumb, play, info, title, channel, five actions), so @@ -64,14 +65,6 @@ function current(): { list: NavList; index: number; item: HTMLElement } | null { return item && list ? { list, index: indexOf(item), item } : null; } -function scrollParent(node: HTMLElement): HTMLElement | null { - for (let el = node.parentElement; el; el = el.parentElement) { - const oy = getComputedStyle(el).overflowY; - if (oy === "auto" || oy === "scroll") return el; - } - return null; -} - /** Where `j` lands when nothing in a list has focus yet: the first item the reader can see, not the * top of the list — after scrolling down a feed, the first press must not throw them back up. "Can * see" = its bottom edge is below the top of the scroller's visible band, which starts under the diff --git a/frontend/src/lib/listPaging.test.ts b/frontend/src/lib/listPaging.test.ts new file mode 100644 index 00000000..f8900af5 --- /dev/null +++ b/frontend/src/lib/listPaging.test.ts @@ -0,0 +1,43 @@ +// One definition of a PageUp/PageDown page for the palette and every popover list (E11, owner +// request). The DOM-bound halves are covered where they are wired (CommandPalette, Combobox, +// ActionMenu); this pins the arithmetic both lean on. +import { describe, expect, it } from "vitest"; +import { PAGE_FALLBACK, pageStep, pageTarget } from "./listPaging"; + +const box = (top: number, height: number) => + ({ getBoundingClientRect: () => ({ top, bottom: top + height, height }) }) as unknown as Element; + +describe("pageStep", () => { + it("is the rows wholly in view minus one", () => { + // A 100px list with 30px rows: 0–30, 30–60, 60–90 fit; 90–120 is cut off. + const rows = [0, 30, 60, 90, 120].map((top) => box(top, 30)); + expect(pageStep(box(0, 100), rows)).toBe(2); + }); + + it("never stands still, and falls back without layout", () => { + expect(pageStep(box(0, 20), [box(0, 30)])).toBe(1); + expect(pageStep(box(0, 0), [box(0, 30)])).toBe(PAGE_FALLBACK); + expect(pageStep(null, [])).toBe(PAGE_FALLBACK); + }); +}); + +describe("pageTarget", () => { + it("moves a page and stops at both ends instead of wrapping", () => { + expect(pageTarget(0, 10, 25)).toBe(10); + expect(pageTarget(20, 10, 25)).toBe(24); + expect(pageTarget(24, 10, 25)).toBe(24); + expect(pageTarget(3, -10, 25)).toBe(0); + expect(pageTarget(0, 10, 0)).toBeNull(); + }); + + it("passes over a row it may not land on, then looks back — never past the start", () => { + const off = new Set([10, 11]); + const usable = (i: number) => !off.has(i); + expect(pageTarget(0, 10, 25, usable)).toBe(12); // on past the landing row first + off.add(24); + off.add(23); + expect(pageTarget(15, 10, 25, usable)).toBe(22); // at the end: back toward the start + expect(pageTarget(22, 10, 25, usable)).toBeNull(); // nothing between the start and the end + expect(pageTarget(15, -10, 25, (i) => i > 12)).toBe(13); + }); +}); diff --git a/frontend/src/lib/listPaging.ts b/frontend/src/lib/listPaging.ts new file mode 100644 index 00000000..78f9c033 --- /dev/null +++ b/frontend/src/lib/listPaging.ts @@ -0,0 +1,49 @@ +// ── PageUp/PageDown over a list the keyboard otherwise walks row by row (E11, owner request) ───── +// The jump-to palette (461 rows, ~11 in view) and every popover list — the sidebar's channel filter +// over hundreds of channels, the Plex pickers, the filter and sort menus — had only the arrows. One +// definition of "a page" here, so the palette and the dropdowns cannot drift apart. + +/** A page when there is no layout to measure (jsdom, or a list not painted yet). */ +export const PAGE_FALLBACK = 10; + +/** The nearest ancestor that scrolls vertically. */ +export function scrollParent(node: Element): HTMLElement | null { + for (let el = node.parentElement; el; el = el.parentElement) { + const oy = getComputedStyle(el).overflowY; + if (oy === "auto" || oy === "scroll") return el; + } + return null; +} + +/** How far one PageUp/PageDown moves: the rows that fit wholly inside `scroller` right now, minus one, + * so the row you paged from stays in sight at the new page's edge (a native listbox does the same). + * Measured rather than fixed: a list's height follows the viewport, and group headings eat into it. */ +export function pageStep(scroller: Element | null, rows: Iterable): number { + const box = scroller?.getBoundingClientRect(); + if (!box || box.height === 0) return PAGE_FALLBACK; + let fits = 0; + for (const row of rows) { + const r = row.getBoundingClientRect(); + if (r.top >= box.top - 0.5 && r.bottom <= box.bottom + 0.5) fits++; + } + return Math.max(1, fits - 1); +} + +/** Where a page move of `delta` from row `from` lands among `count` rows. It stops at both ends — no + * wrap, unlike the arrows: one step past the end is a small, obvious move, but a page-sized jump that + * lands back at the top reads as having lost your place. A row `usable` refuses (a disabled menu row) + * is never the target: the search goes on past the landing row first, then back toward the start, + * never beyond it. `null` = nowhere to go. */ +export function pageTarget( + from: number, + delta: number, + count: number, + usable: (index: number) => boolean = () => true, +): number | null { + if (count <= 0) return null; + const to = Math.min(count - 1, Math.max(0, from + delta)); + const dir = delta < 0 ? -1 : 1; + for (let i = to; i >= 0 && i < count; i += dir) if (usable(i)) return i; + for (let i = to - dir; (i - from) * dir > 0; i -= dir) if (usable(i)) return i; + return null; +}