feat(popover): migrate the click menus onto the platform (R8 S4b)
AddToPlaylist, LanguageSwitcher, the Plex DetailCustomizeMenu and ViewSwitcher each had their own anchoring, dismissal and (for ViewSwitcher) keyboard engine. All four now use the platform, so their items are PopoverItems with real menuitemradio/aria-checked, roving focus, typeahead and flip/shift anchoring. Two platform additions this needed: - `selectedIndex`: opening a single-choice menu must land on the choice already in effect, which is what ViewSwitcher's hand-rolled engine did by seeding its roving index to the current option. - `ariaLabel` on PopoverSurface: an icon-only trigger leaves the panel with no name. Fixed while wiring `selectedIndex` up: it stranded focus on the menu container. That path focuses its item synchronously in a layout effect, and FloatingFocusManager's own initial focus then ran afterwards and took focus back; the no-selectedIndex path only escaped this because it defers focus to a microtask while waiting for the item list to populate. On a roving list the list navigation owns initial focus, so the manager is now told to take none (initialFocus -1). Behaviour changes worth naming: - The header LanguageSwitcher no longer opens on hover. The platform focuses the first item on open, so a hover-open would yank focus off whatever the user was doing. Both variants are now click-only, like every other menu in the app. - PrefToggle became a PopoverItem, so the rows callers inject through the menu's `extra` prop join the same roving list as the built-in ones. - AddToPlaylist fetches its list on open, so on the first paint there are no items to focus and focus stays on the trigger (as it did before the migration); arrows move into the list once it lands. Verified in-browser: ViewSwitcher opens on the checked mode, arrows rove, Escape closes and refocuses the trigger; the ChannelTags pilot is unregressed; AddToPlaylist anchors bottom-start without the click reaching the feed card underneath; the rail language menu keeps its right-edge, bottom-aligned anchoring.
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import { useState } from "react";
|
||||
import { Check, ChevronDown, type LucideIcon } from "lucide-react";
|
||||
import clsx from "clsx";
|
||||
import { useDismiss } from "../lib/useDismiss";
|
||||
import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover";
|
||||
|
||||
export interface ViewOption<T extends string> {
|
||||
id: T;
|
||||
@@ -13,9 +13,11 @@ export interface ViewOption<T extends string> {
|
||||
// menu behind it. Generic over the mode id so any module's list can adopt it (the feed's
|
||||
// density modes; the managers' layouts) without re-deriving the menu behaviour.
|
||||
//
|
||||
// This is a real menu, so it carries the whole keyboard model the role promises: arrows/Home/End
|
||||
// move a roving focus, Escape closes, and closing always hands focus back to the trigger (the
|
||||
// menu unmounts under the focused item, which would otherwise strand focus on <body>).
|
||||
// This was the house's textbook menu, with its own hand-rolled roving-tabindex/arrow/Escape engine.
|
||||
// It now rides the R8 popover platform like every other menu, which supplies the same model
|
||||
// (arrows/Home/End rove, Escape closes and hands focus back to the trigger, Tab out closes) plus the
|
||||
// two things the hand-rolled version lacked: typeahead, and anchoring that flips and shifts instead
|
||||
// of being absolutely positioned inside whatever ancestor happens to clip it.
|
||||
export default function ViewSwitcher<T extends string>({
|
||||
value,
|
||||
options,
|
||||
@@ -34,19 +36,6 @@ export default function ViewSwitcher<T extends string>({
|
||||
labelClassName?: string;
|
||||
}) {
|
||||
const [open, setOpen] = useState(false);
|
||||
// Which item holds the roving tabindex; seeded to the active one when the menu opens.
|
||||
const [focusIdx, setFocusIdx] = useState(0);
|
||||
const btnRef = useRef<HTMLButtonElement | null>(null);
|
||||
const menuRef = useRef<HTMLDivElement | null>(null);
|
||||
const itemRefs = useRef<(HTMLButtonElement | null)[]>([]);
|
||||
|
||||
function close({ refocus = true }: { refocus?: boolean } = {}) {
|
||||
setOpen(false);
|
||||
if (refocus) btnRef.current?.focus();
|
||||
}
|
||||
// Outside-click must NOT refocus the trigger — that would steal focus from whatever the user
|
||||
// just clicked. Escape is handled on the menu itself (below), where returning focus is right.
|
||||
useDismiss(open, () => close({ refocus: false }), [btnRef, menuRef]);
|
||||
|
||||
// A `value` outside `options` is a caller bug; degrade to the first option rather than render
|
||||
// nothing. The menu then shows no checked item, which is honest — nothing IS selected.
|
||||
@@ -54,55 +43,24 @@ export default function ViewSwitcher<T extends string>({
|
||||
0,
|
||||
options.findIndex((o) => o.id === value),
|
||||
);
|
||||
// Seeding the roving index HERE — rather than in an effect keyed on `open` — matters: an effect
|
||||
// would only schedule the update, so the focus effect below would still see the previous index
|
||||
// and focus the wrong item for one render before correcting.
|
||||
function openMenu() {
|
||||
setFocusIdx(currentIdx);
|
||||
setOpen(true);
|
||||
}
|
||||
// Clamped at render, so a caller whose option list shrinks while the menu is open can't leave
|
||||
// the roving tabindex pointing past the end (which would make every item un-tabbable).
|
||||
const rovingIdx = Math.min(focusIdx, options.length - 1);
|
||||
useEffect(() => {
|
||||
if (open) itemRefs.current[rovingIdx]?.focus();
|
||||
}, [open, rovingIdx]);
|
||||
|
||||
const pop = useAnchoredPopover({
|
||||
open,
|
||||
onOpenChange: setOpen,
|
||||
placement: "bottom-end",
|
||||
// Opening a "pick one of these modes" menu should land on the mode that is already picked.
|
||||
selectedIndex: currentIdx,
|
||||
});
|
||||
|
||||
const current = options[currentIdx];
|
||||
if (!current) return null;
|
||||
const CurrentIcon = current.icon;
|
||||
|
||||
function onMenuKeyDown(e: React.KeyboardEvent) {
|
||||
const last = options.length - 1;
|
||||
if (e.key === "Escape") {
|
||||
// Stop it reaching useDismiss's document listener, which would close without refocusing.
|
||||
e.stopPropagation();
|
||||
close();
|
||||
} else if (e.key === "ArrowDown") {
|
||||
e.preventDefault();
|
||||
setFocusIdx((i) => (i >= last ? 0 : i + 1));
|
||||
} else if (e.key === "ArrowUp") {
|
||||
e.preventDefault();
|
||||
setFocusIdx((i) => (i <= 0 ? last : i - 1));
|
||||
} else if (e.key === "Home") {
|
||||
e.preventDefault();
|
||||
setFocusIdx(0);
|
||||
} else if (e.key === "End") {
|
||||
e.preventDefault();
|
||||
setFocusIdx(last);
|
||||
} else if (e.key === "Tab") {
|
||||
// Tabbing out of a menu closes it, but the focus is moving on by itself.
|
||||
close({ refocus: false });
|
||||
}
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="relative">
|
||||
<>
|
||||
<button
|
||||
ref={btnRef}
|
||||
onClick={() => (open ? close() : openMenu())}
|
||||
aria-haspopup="menu"
|
||||
aria-expanded={open}
|
||||
ref={pop.refs.setReference}
|
||||
{...pop.getReferenceProps()}
|
||||
// The aria-label carries the mode's name at every size — the visible text below is a
|
||||
// nicety for wide windows, not the accessible name.
|
||||
aria-label={`${label}: ${current.label}`}
|
||||
@@ -115,46 +73,39 @@ export default function ViewSwitcher<T extends string>({
|
||||
className={`w-3 h-3 shrink-0 transition-transform ${open ? "rotate-180" : ""}`}
|
||||
/>
|
||||
</button>
|
||||
{open && (
|
||||
<div
|
||||
ref={menuRef}
|
||||
role="menu"
|
||||
aria-label={label}
|
||||
onKeyDown={onMenuKeyDown}
|
||||
className="glass-menu absolute right-0 top-full mt-1 z-chrome min-w-44 p-1.5 rounded-xl animate-[popIn_0.16s_ease]"
|
||||
>
|
||||
{/* The modes are one exclusive choice, so the radio items need a group to scope their
|
||||
checked-ness to — otherwise they read as independent checkable items. */}
|
||||
<div role="group">
|
||||
{options.map((o, i) => {
|
||||
const Icon = o.icon;
|
||||
const on = o.id === value;
|
||||
return (
|
||||
<button
|
||||
key={o.id}
|
||||
ref={(el) => {
|
||||
itemRefs.current[i] = el;
|
||||
}}
|
||||
role="menuitemradio"
|
||||
aria-checked={on}
|
||||
tabIndex={i === rovingIdx ? 0 : -1}
|
||||
onClick={() => {
|
||||
onChange(o.id);
|
||||
close();
|
||||
}}
|
||||
className={`w-full flex items-center gap-2 px-2 py-1.5 rounded-lg text-sm text-left transition ${
|
||||
on ? "text-accent" : "text-fg hover:bg-card"
|
||||
}`}
|
||||
>
|
||||
<Icon className="w-4 h-4 shrink-0" />
|
||||
<span className="flex-1 truncate">{o.label}</span>
|
||||
{on && <Check className="w-3.5 h-3.5 shrink-0" />}
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
<PopoverSurface
|
||||
popover={pop}
|
||||
ariaLabel={label}
|
||||
className="glass-menu min-w-44 p-1.5 rounded-xl animate-[popIn_0.16s_ease]"
|
||||
>
|
||||
{/* The modes are one exclusive choice, so the radio items need a group to scope their
|
||||
checked-ness to — otherwise they read as independent checkable items. */}
|
||||
<div role="group">
|
||||
{options.map((o) => {
|
||||
const Icon = o.icon;
|
||||
const on = o.id === value;
|
||||
return (
|
||||
<PopoverItem
|
||||
key={o.id}
|
||||
role="menuitemradio"
|
||||
checked={on}
|
||||
onSelect={() => {
|
||||
onChange(o.id);
|
||||
setOpen(false);
|
||||
}}
|
||||
label={o.label}
|
||||
className={`w-full flex items-center gap-2 px-2 py-1.5 rounded-lg text-sm text-left outline-none transition data-[active]:bg-[color-mix(in_srgb,var(--accent)_24%,transparent)] ${
|
||||
on ? "text-accent" : "text-fg hover:bg-card"
|
||||
}`}
|
||||
>
|
||||
<Icon className="w-4 h-4 shrink-0" />
|
||||
<span className="flex-1 truncate">{o.label}</span>
|
||||
{on && <Check className="w-3.5 h-3.5 shrink-0" />}
|
||||
</PopoverItem>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</PopoverSurface>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user