From 5ba8feaf310382fd1ae20e451667c5a27e8ab371 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 26 Oct 2026 08:51:42 +0100 Subject: [PATCH 1/3] refactor(webapp): build the card menu on the track menu's popup ActionMenu (#348) carried its own copy of the panel, its keys and its outside-close listeners beside PopupMenu (#347). It is now a button over PopupMenu, which gains an optional icon, a danger entry, an id, a class, and a placement above its button when there is no room below. The placement is a pure function with its own tests. onClose now says whether focus goes back to the opener: yes for a key or an entry, no for a press elsewhere. Signed-off-by: InstaZDLL --- webapp/src/components/action-menu.tsx | 229 ++++++-------------------- webapp/src/components/popup-menu.tsx | 72 +++++--- webapp/src/lib/menu-place.test.ts | 54 ++++++ webapp/src/lib/menu-place.ts | 38 +++++ 4 files changed, 189 insertions(+), 204 deletions(-) create mode 100644 webapp/src/lib/menu-place.test.ts create mode 100644 webapp/src/lib/menu-place.ts 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/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 (