diff --git a/webapp/e2e/smart-playlists-and-podcasts.spec.ts b/webapp/e2e/smart-playlists-and-podcasts.spec.ts index e1ac3ef8..61565773 100644 --- a/webapp/e2e/smart-playlists-and-podcasts.spec.ts +++ b/webapp/e2e/smart-playlists-and-podcasts.spec.ts @@ -583,14 +583,17 @@ test("subscribes to a podcast, downloads, plays and keeps the place", async ({ ); await page.getByLabel("Feed address").fill("https://feeds.example.org/night.xml"); await page.getByRole("button", { name: "Subscribe" }).click(); - expect(podcastWrites).toEqual(["subscribe https://feeds.example.org/night.xml"]); + // The route sees the write after the click returns, not before it: polled. + await expect + .poll(() => podcastWrites) + .toEqual(["subscribe https://feeds.example.org/night.xml"]); const episodes = page.locator(".podcast-episodes"); await expect(episodes.getByRole("heading", { name: "Night Shift" })).toBeVisible(); const first = episodes.locator("li").filter({ hasText: "First night" }); await expect(first).toContainText("not downloaded"); await first.getByRole("button", { name: "Download" }).click(); - expect(podcastWrites).toContain("download episode-1"); + await expect.poll(() => podcastWrites).toContain("download episode-1"); await expect(first).toContainText("ready"); // The saved place is shown, and playing resumes from it. @@ -604,7 +607,7 @@ test("subscribes to a podcast, downloads, plays and keeps the place", async ({ await expect.poll(() => bookmarkWrites.map((write) => write.id)).toContain("episode-1"); await page.getByRole("button", { name: "Refresh every feed" }).click(); - expect(podcastWrites).toContain("refresh"); + await expect.poll(() => podcastWrites).toContain("refresh"); }); /** diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index d366a779..05045f8f 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -7436,6 +7436,12 @@ test("lists what needs attention, splits a credit on the second click, and undoe await expect(confirm).toBeVisible(); expect(state.writes).toEqual([]); await expectNoWcagViolations(page); + // The confirmation stands for four seconds, and on a loaded machine the scan + // alone can outlast them: armed again if it lapsed, so the click below meets + // the confirmation rather than racing its timer. + if (!(await confirm.isVisible())) { + await page.getByRole("button", { name: "Split", exact: true }).click(); + } await confirm.click(); await expect( page.getByText("Ashreveal, Ashish Swargiary is split."), diff --git a/webapp/src/components/action-menu.tsx b/webapp/src/components/action-menu.tsx index 5a4ccc5e..40922c57 100644 --- a/webapp/src/components/action-menu.tsx +++ b/webapp/src/components/action-menu.tsx @@ -1,34 +1,27 @@ -import { - type KeyboardEvent, - useEffect, - useId, - useLayoutEffect, - useRef, - useState, -} from "react"; +import { useId, useRef, useState } from "react"; import { createPortal } from "react-dom"; import { Icon } from "./icons"; +import { PopupMenu, type PopupMenuItem } from "./popup-menu"; -export type ActionMenuItem = { - key: string; - label: string; - run: () => void; - /** Drawn in the danger colour: the entry that destroys something. */ - danger?: boolean; -}; +export type ActionMenuItem = Pick< + PopupMenuItem, + "key" | "label" | "run" | "danger" +>; /** * A "⋯" button and the short menu it opens: the actions on one card that do * not deserve a button of their own. * - * Drawn as the track menu is — the same panel and entries, the same keys of - * the ARIA menu pattern — but anchored to its button rather than to a pointer. - * Portalled to the body and fixed to the viewport for the track menu's reason: - * a page block rising into place carries a `translate`, and a fixed child of - * it would be placed inside the block instead of beside the button. + * The menu is the track menu's own [`PopupMenu`], hung under the button by its + * right edge, and above it when there is no room below. Portalled to the body + * for the track menu's reason: a page block rising into place carries a + * `translate`, and a fixed child of it would be placed inside the block + * instead of beside the button. * - * Closing hands focus back to the button, before the chosen entry runs. + * Closing by a key or an entry hands focus back to the button, before the + * chosen entry runs: an entry that opens a dialog then finds it there to hand + * back when the dialog closes. */ export function ActionMenu({ label, @@ -40,12 +33,24 @@ export function ActionMenu({ items: ActionMenuItem[]; disabled?: boolean; }) { - const [open, setOpen] = useState(false); + const [at, setAt] = useState<{ x: number; y: number; above: number } | null>( + null, + ); const button = useRef(null); const menuId = useId(); + function toggle() { + if (at) { + setAt(null); + return; + } + const box = button.current?.getBoundingClientRect(); + if (!box) return; + setAt({ x: box.right, y: box.bottom + 4, above: box.top - 4 }); + } + function close(refocus: boolean) { - setOpen(false); + setAt(null); if (refocus) button.current?.focus(); } @@ -57,21 +62,35 @@ export function ActionMenu({ className="action-menu-button" aria-label={label} aria-haspopup="menu" - aria-expanded={open} - aria-controls={open ? menuId : undefined} + aria-expanded={at !== null} + aria-controls={at ? menuId : undefined} disabled={disabled} - onClick={() => setOpen((current) => !current)} + onClick={toggle} > - {open && button.current + {at ? createPortal( - ({ + ...item, + opensDialog: true, + run: () => { + close(true); + item.run(); + }, + }))} onClose={close} + anchor={button} />, document.body, ) @@ -79,153 +98,3 @@ export function ActionMenu({ ); } - -function ActionMenuPanel({ - id, - label, - anchor, - items, - onClose, -}: { - id: string; - label: string; - anchor: HTMLElement; - items: ActionMenuItem[]; - onClose: (refocus: boolean) => void; -}) { - const list = useRef(null); - const [place, setPlace] = useState<{ left: number; top: number } | null>( - null, - ); - - // Under the button, its right edge on the button's, and moved back inside - // the window when that would hang off an edge — above the button when there - // is no room below it. - useLayoutEffect(() => { - const node = list.current; - if (!node) return; - const from = anchor.getBoundingClientRect(); - const rect = node.getBoundingClientRect(); - const margin = 8; - const below = from.bottom + 4; - const top = - below + rect.height + margin > window.innerHeight - ? from.top - rect.height - 4 - : below; - setPlace({ - left: Math.max( - margin, - Math.min( - from.right - rect.width, - window.innerWidth - rect.width - margin, - ), - ), - top: Math.max(margin, top), - }); - }, [anchor]); - - // Focus moves in once the menu is placed: hidden for its measurement, it - // could not take focus before. - const placed = place !== null; - useEffect(() => { - if (placed) { - list.current?.querySelector('[role="menuitem"]')?.focus(); - } - }, [placed]); - - // Anything outside closes it, as it closes the track menu; a press on the - // button is left to the button, whose click toggles it shut. - const closeRef = useRef(onClose); - closeRef.current = onClose; - useEffect(() => { - const away = (event: Event) => { - const target = event.target as Node; - if (list.current?.contains(target) || anchor.contains(target)) return; - closeRef.current(false); - }; - const leave = () => closeRef.current(false); - document.addEventListener("pointerdown", away, true); - document.addEventListener("wheel", leave, { capture: true, passive: true }); - window.addEventListener("blur", leave); - window.addEventListener("resize", leave); - return () => { - document.removeEventListener("pointerdown", away, true); - document.removeEventListener("wheel", leave, true); - window.removeEventListener("blur", leave); - window.removeEventListener("resize", leave); - }; - }, [anchor]); - - function onKeyDown(event: KeyboardEvent) { - const entries = Array.from( - list.current?.querySelectorAll('[role="menuitem"]') ?? [], - ); - const at = entries.indexOf(document.activeElement as HTMLElement); - const go = (index: number) => { - event.preventDefault(); - entries[(index + entries.length) % entries.length]?.focus(); - }; - switch (event.key) { - case "ArrowDown": - go(at + 1); - break; - case "ArrowUp": - go(at - 1); - break; - case "Home": - go(0); - break; - case "End": - go(entries.length - 1); - break; - case "Escape": - event.preventDefault(); - onClose(true); - break; - // Focus goes back to the button first and the Tab is left to run from - // there, so it lands on what follows the button (or, with Shift, what - // precedes it). Left in the menu, which sits at the end of the body, - // the Tab ran off the page and the menu's unmounting dropped focus on - // the body. - case "Tab": - onClose(true); - break; - } - } - - return ( - - ); -} diff --git a/webapp/src/components/canvas.tsx b/webapp/src/components/canvas.tsx index dfffbf45..edeb5694 100644 --- a/webapp/src/components/canvas.tsx +++ b/webapp/src/components/canvas.tsx @@ -1,6 +1,7 @@ import { type ReactNode, useEffect, useState } from "react"; import { canvasUrl } from "../api"; +import { onMediaChange } from "../lib/media-query"; import type { TranslationKey } from "../state/i18n"; /** @@ -41,8 +42,7 @@ export function usePrefersReducedMotion(): boolean { if (!query) return; const onChange = () => setReduced(query.matches); onChange(); - query.addEventListener("change", onChange); - return () => query.removeEventListener("change", onChange); + return onMediaChange(query, onChange); }, []); return reduced; } diff --git a/webapp/src/components/popup-menu.tsx b/webapp/src/components/popup-menu.tsx index f227dbd9..a6e35a46 100644 --- a/webapp/src/components/popup-menu.tsx +++ b/webapp/src/components/popup-menu.tsx @@ -7,14 +7,17 @@ import { useState, } from "react"; +import { placeMenu } from "../lib/menu-place"; import { Icon, type IconName } from "./icons"; /** One entry of a menu. */ export type PopupMenuItem = { key: string; - icon: IconName; + icon?: IconName; label: string; run: () => void; + /** Drawn in the danger colour: the entry that destroys something. */ + danger?: boolean; /** * The entry replaces the menu with a dialog of its own, which takes focus. * Such an entry closes the menu itself: closing it here would hand focus back @@ -33,25 +36,41 @@ export type PopupMenuItem = { * and moved back inside the window when it would hang off an edge. It answers * the keys of the ARIA menu pattern, and anything outside it closes it. * + * `onClose` is told whether focus should go back to what opened the menu: yes + * for a key or a chosen entry, no for a press or a gesture elsewhere, which + * takes focus where it lands. + * * Fixed rather than absolute, so the caller portals it to the body: an * ancestor that clips its overflow, or that carries a transform while it * rises into place, would otherwise cut it off or place it inside itself. */ export function PopupMenu({ + id, + className, label, x, y, + above, align = "start", items, onClose, anchor, }: { + id?: string; + /** Added to the panel's own class. */ + className?: string; label: string; x: number; y: number; + /** + * Where to hang the menu upward from when it does not fit below `y`: the + * top of the button that opened it, for a menu opened near the bottom of + * the window. Without it the menu is only pushed back up inside the window. + */ + above?: number; align?: "start" | "end"; items: PopupMenuItem[]; - onClose: () => void; + onClose: (refocus: boolean) => void; /** * The button that opened the menu, if one did. A press on it is not a press * "outside": it toggles the menu, and closing here first would have the @@ -68,20 +87,19 @@ export function PopupMenu({ const node = list.current; if (!node) return; const rect = node.getBoundingClientRect(); - const margin = 8; - const left = align === "end" ? x - rect.width : x; - setPlace({ - left: Math.max( - margin, - Math.min(left, window.innerWidth - rect.width - margin), - ), - top: Math.max( - margin, - Math.min(y, window.innerHeight - rect.height - margin), - ), - }); + setPlace( + placeMenu({ + x, + y, + above, + align, + width: rect.width, + height: rect.height, + viewport: { width: window.innerWidth, height: window.innerHeight }, + }), + ); node.querySelector('[role="menuitem"]')?.focus(); - }, [align, x, y]); + }, [above, align, x, y]); // Anything outside the menu closes it: a click, the wheel or a finger // moving the page, the window losing focus. @@ -108,7 +126,7 @@ export function PopupMenu({ const target = event.target as Node; if (list.current?.contains(target)) return; if (anchorRef.current?.current?.contains(target)) return; - closeRef.current(); + closeRef.current(false); }; // The wheel closes it, over the menu too, unless the menu has rows to // scroll: a track credited to many artists makes it taller than a short @@ -119,9 +137,9 @@ export function PopupMenu({ const menu = list.current; const inside = menu?.contains(event.target as Node) ?? false; if (inside && menu && menu.scrollHeight > menu.clientHeight) return; - closeRef.current(); + closeRef.current(false); }; - const leave = () => closeRef.current(); + const leave = () => closeRef.current(false); // A long press opens the menu with the finger still down, and that finger // drifts as it lifts: its moves are the gesture that opened the menu, not // one leaving it. Only a touch that starts after the menu is open counts. @@ -180,10 +198,15 @@ export function PopupMenu({ break; case "Escape": event.preventDefault(); - onClose(); + onClose(true); break; + // Focus goes back to what opened the menu first, and the Tab is left to + // run from there, so it lands on what follows it (or, with Shift, what + // precedes it). Left in the menu, which sits at the end of the body, the + // Tab ran off the page and the menu's unmounting dropped focus on the + // body. case "Tab": - onClose(); + onClose(true); break; } } @@ -191,7 +214,8 @@ export function PopupMenu({ return (