From 280de2e59229f0d5d1397d8911d6b467ba3aea70 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 31 Jul 2026 00:02:14 +0200 Subject: [PATCH] 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. --- frontend/src/components/AddToPlaylist.tsx | 181 ++++++++---------- frontend/src/components/AnchoredPopover.tsx | 18 +- frontend/src/components/LanguageSwitcher.tsx | 185 +++++++------------ frontend/src/components/ViewSwitcher.tsx | 151 +++++---------- frontend/src/lib/plexDetailUi.tsx | 65 +++---- 5 files changed, 240 insertions(+), 360 deletions(-) diff --git a/frontend/src/components/AddToPlaylist.tsx b/frontend/src/components/AddToPlaylist.tsx index 8d8fc581..9236233a 100644 --- a/frontend/src/components/AddToPlaylist.tsx +++ b/frontend/src/components/AddToPlaylist.tsx @@ -1,16 +1,17 @@ -import { useEffect, useLayoutEffect, useRef, useState } from "react"; -import { createPortal } from "react-dom"; +import { useState } from "react"; import { useTranslation } from "react-i18next"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { qk } from "../lib/queryKeys"; import { Check, ListPlus, Plus, Youtube } from "lucide-react"; import { api, type Playlist } from "../lib/api"; -import { useDismiss } from "../lib/useDismiss"; import { useScrollFade } from "../lib/useScrollFade"; import { playlistName } from "../lib/playlistName"; +import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; -// A small popover (portaled to body, so the feed grid can't clip it) for toggling a -// video's membership in the user's playlists, with inline "new playlist" creation. +// A small anchored menu for toggling a video's membership in the user's playlists, with inline +// "new playlist" creation. Same shape as the channel tag editor (R8 S1's pilot): radio-style +// toggles plus a free-text creator, so it follows that file's two conventions — typeahead OFF and +// the input swallowing non-Escape keys, since character and arrow keys belong to the text field. export default function AddToPlaylist({ videoId, className, @@ -21,60 +22,23 @@ export default function AddToPlaylist({ const { t } = useTranslation(); const qc = useQueryClient(); const fade = useScrollFade(); - const triggerRef = useRef(null); - const panelRef = useRef(null); const [open, setOpen] = useState(false); - const [coords, setCoords] = useState<{ top: number; left: number }>({ top: 0, left: 0 }); const [newName, setNewName] = useState(""); const [busy, setBusy] = useState(false); + const pop = useAnchoredPopover({ + open, + onOpenChange: setOpen, + offset: 6, + typeahead: false, + }); + const membership = useQuery({ queryKey: qk.playlistMembership(videoId), queryFn: () => api.playlists(videoId), enabled: open, }); - // Re-place once the panel is actually rendered (so we know its real height) and whenever - // its content — hence height — changes, so the flip-above decision uses the true size. - useLayoutEffect(() => { - if (open) place(); - }, [open, membership.data]); - - function place() { - const r = triggerRef.current?.getBoundingClientRect(); - if (!r) return; - const width = 240; - const margin = 8; - // Use the panel's real height once it's mounted; estimate on the first open. - const h = panelRef.current?.offsetHeight ?? 300; - let top = r.bottom + 6; - if (top + h > window.innerHeight - margin) { - // Not enough room below — open upward, clamped to the viewport. - top = Math.max(margin, r.top - h - 6); - } - const left = Math.min(Math.max(margin, r.left), window.innerWidth - width - margin); - setCoords({ top: Math.round(top), left: Math.round(left) }); - } - - function toggleOpen(e: React.MouseEvent) { - e.preventDefault(); - e.stopPropagation(); - if (!open) place(); - setOpen((o) => !o); - } - - useDismiss(open, () => setOpen(false), [panelRef, triggerRef]); - // Keep the panel anchored to the trigger as the page resizes/scrolls while open. - useEffect(() => { - if (!open) return; - window.addEventListener("resize", place); - window.addEventListener("scroll", place, true); - return () => { - window.removeEventListener("resize", place); - window.removeEventListener("scroll", place, true); - }; - }, [open]); - function refresh() { qc.invalidateQueries({ queryKey: qk.playlistMembership(videoId) }); qc.invalidateQueries({ queryKey: qk.playlists() }); @@ -95,8 +59,7 @@ export default function AddToPlaylist({ } } - async function createAndAdd(e: React.FormEvent) { - e.preventDefault(); + async function createAndAdd() { const name = newName.trim(); if (!name || busy) return; setBusy(true); @@ -117,8 +80,15 @@ export default function AddToPlaylist({ return ( <> - {open && - createPortal( -
e.stopPropagation()} - > -
{t("playlists.addToPlaylist")}
-
- {lists.length === 0 && !membership.isLoading && ( -
{t("playlists.noneYet")}
- )} - {lists.map((pl) => ( - - ))} -
-
- - setNewName(e.target.value)} - placeholder={t("playlists.newPlaylist")} - className="flex-1 min-w-0 bg-card border border-border rounded-md px-2 py-1 text-xs outline-none focus:border-accent" - /> - -
, - document.body, - )} + {pl.has_video && } + + {playlistName(pl, t)} + {(pl.source === "youtube" || pl.yt_playlist_id) && ( + + )} + + ))} + +
+ + setNewName(e.target.value)} + onKeyDown={(e) => { + // Escape bubbles to the popover's dismiss; every other key is text editing here and + // must not reach the surrounding menu's list navigation. + if (e.key === "Escape") return; + e.stopPropagation(); + if (e.key === "Enter") { + e.preventDefault(); + void createAndAdd(); + } + }} + placeholder={t("playlists.newPlaylist")} + className="flex-1 min-w-0 bg-card border border-border rounded-md px-2 py-1 text-xs outline-none focus:border-accent" + /> +
+ + ); } diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx index d0ca9d77..2eeefef7 100644 --- a/frontend/src/components/AnchoredPopover.tsx +++ b/frontend/src/components/AnchoredPopover.tsx @@ -82,6 +82,10 @@ export interface AnchoredPopoverOptions { /** Type-to-focus a matching item by label (menu/listbox only). Default on; turn OFF for menus that * contain a free-text input, where character keys belong to the text field. */ typeahead?: boolean; + /** Index of the item that is currently CHOSEN, for a single-choice menu/listbox. Opening then + * lands the roving focus on it instead of the first item — the behaviour a "pick one of these + * modes" menu needs. Leave unset for action menus and multi-select toggle lists. */ + selectedIndex?: number | null; } export type AnchoredPopover = ReturnType; @@ -149,6 +153,7 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { listRef: elementsRef, activeIndex, onNavigate: setActiveIndex, + selectedIndex: opts.selectedIndex ?? null, loop: true, // Focus the first item on open for BOTH click and keyboard (the default 'auto' skips pointer-opens, // which strands focus on the menu container — arrows then can't enter the roving list at all). @@ -273,11 +278,15 @@ export function PopoverSurface({ focusManaged, initialFocus, passthrough, + ariaLabel, }: { popover: AnchoredPopover; className?: string; style?: CSSProperties; children: ReactNode; + /** Accessible name for the panel itself — a menu whose trigger is icon-only has nothing else to + * take its name from. */ + ariaLabel?: string; focusManaged?: boolean; initialFocus?: number | React.MutableRefObject; /** The surface never receives pointer events — for a passive caption that must not swallow a click @@ -321,6 +330,7 @@ export function PopoverSurface({
{positioned} diff --git a/frontend/src/components/LanguageSwitcher.tsx b/frontend/src/components/LanguageSwitcher.tsx index 6ae3bc70..8f427c68 100644 --- a/frontend/src/components/LanguageSwitcher.tsx +++ b/frontend/src/components/LanguageSwitcher.tsx @@ -1,15 +1,16 @@ -import { useEffect, useRef, useState } from "react"; -import { createPortal } from "react-dom"; +import { useState } from "react"; import { Check, Globe } from "lucide-react"; import { LANGUAGES, type LangCode } from "../i18n"; +import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; // Compact language picker (globe + current code). Presentational: the parent decides what // changing the language does (set it; persist server-side when signed in). // -// Two variants: "header" (legacy inline dropdown, opens below) and "rail" (used in the left -// nav's bottom icon cluster — an icon-only button whose menu is portaled to and -// anchored to the right + above the button, escaping the nav's backdrop-filter which would -// otherwise trap an absolutely-positioned popover). +// Two variants, now one implementation on the R8 popover platform: "header" opens below the pill +// (used on the landing page and the setup wizard) and "rail" is an icon-only button in the left +// nav's bottom cluster whose menu opens to the RIGHT, bottom-aligned. Both used to hand-roll their +// own anchoring, and the header one also opened on hover — dropped on purpose: the platform focuses +// the first item when a menu opens, so a hover-open would yank focus off whatever the user was doing. export default function LanguageSwitcher({ value, onChange, @@ -22,128 +23,68 @@ export default function LanguageSwitcher({ variant?: "header" | "rail"; }) { const [open, setOpen] = useState(false); - const closeTimer = useRef | null>(null); - const btnRef = useRef(null); - const panelRef = useRef(null); - const [pos, setPos] = useState<{ left: number; bottom: number }>({ left: 0, bottom: 0 }); - const current = LANGUAGES.find((l) => l.code === value) ?? LANGUAGES[0]; - - function openNow() { - if (closeTimer.current) clearTimeout(closeTimer.current); - setOpen(true); - } - function closeSoon() { - closeTimer.current = setTimeout(() => setOpen(false), 200); - } - function toggleRail() { - if (!open) { - const r = btnRef.current?.getBoundingClientRect(); - if (r) setPos({ left: r.right + 8, bottom: window.innerHeight - r.bottom }); - } - setOpen((o) => !o); - } - - // Rail popover: dismiss on outside click / Escape (it's portaled, so a contains() check - // spans both the button and the floating panel). - useEffect(() => { - if (variant !== "rail" || !open) return; - function onDoc(e: MouseEvent) { - const target = e.target as Node; - if (panelRef.current?.contains(target) || btnRef.current?.contains(target)) return; - setOpen(false); - } - function onKey(e: KeyboardEvent) { - if (e.key === "Escape") setOpen(false); - } - document.addEventListener("mousedown", onDoc); - document.addEventListener("keydown", onKey); - return () => { - document.removeEventListener("mousedown", onDoc); - document.removeEventListener("keydown", onKey); - }; - }, [variant, open]); - - const menu = ( -
- {LANGUAGES.map((l) => ( - - ))} -
+ const currentIdx = Math.max( + 0, + LANGUAGES.findIndex((l) => l.code === value), ); + const current = LANGUAGES.find((l) => l.code === value) ?? LANGUAGES[0]; + const rail = variant === "rail"; - if (variant === "rail") { - return ( - <> - - {open && - createPortal( -
- {menu} -
, - document.body, - )} - - ); - } - - return ( -
- - {open && ( -
- {LANGUAGES.map((l) => ( - - ))} -
- )} -
+ + {LANGUAGES.map((l) => ( + { + onChange(l.code); + setOpen(false); + }} + label={l.label} + className="w-full flex items-center justify-between gap-2 text-sm px-2 py-2 rounded-lg text-muted outline-none transition hover:text-fg hover:bg-card data-[active]:bg-[color-mix(in_srgb,var(--accent)_24%,transparent)] data-[active]:text-fg" + > + {l.label} + {l.code === value && } + + ))} + + ); } diff --git a/frontend/src/components/ViewSwitcher.tsx b/frontend/src/components/ViewSwitcher.tsx index 80a808e5..61d38d98 100644 --- a/frontend/src/components/ViewSwitcher.tsx +++ b/frontend/src/components/ViewSwitcher.tsx @@ -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 { id: T; @@ -13,9 +13,11 @@ export interface ViewOption { // 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 ). +// 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({ value, options, @@ -34,19 +36,6 @@ export default function ViewSwitcher({ 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(null); - const menuRef = useRef(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({ 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 ( -
+ <> - {open && ( -
- {/* 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. */} -
- {options.map((o, i) => { - const Icon = o.icon; - const on = o.id === value; - return ( - - ); - })} -
+ + {/* 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. */} +
+ {options.map((o) => { + const Icon = o.icon; + const on = o.id === value; + return ( + { + 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" + }`} + > + + {o.label} + {on && } + + ); + })}
- )} -
+ + ); } diff --git a/frontend/src/lib/plexDetailUi.tsx b/frontend/src/lib/plexDetailUi.tsx index 08e7f4a9..6c78ad2d 100644 --- a/frontend/src/lib/plexDetailUi.tsx +++ b/frontend/src/lib/plexDetailUi.tsx @@ -1,10 +1,10 @@ -import { useLayoutEffect, useRef, useState, type ReactNode } from "react"; +import { useLayoutEffect, useState, type ReactNode } from "react"; import { qk } from "./queryKeys"; import { useTranslation } from "react-i18next"; import { useQueryClient } from "@tanstack/react-query"; import { SlidersHorizontal } from "lucide-react"; -import { useDismiss } from "./useDismiss"; import { api } from "./api"; +import { PopoverItem, PopoverSurface, useAnchoredPopover } from "../components/AnchoredPopover"; // Shared "detail page" UI reused by the movie info page (PlexInfo) and the series show/season pages, // so they look consistent (the standing glassy/HTPC brief): the faint fixed art backdrop, the two @@ -131,43 +131,41 @@ export function DetailCustomizeMenu({ }) { const { t } = useTranslation(); const [open, setOpen] = useState(false); - const btnRef = useRef(null); - const menuRef = useRef(null); - useDismiss(open, () => setOpen(false), [menuRef, btnRef]); + const pop = useAnchoredPopover({ open, onOpenChange: setOpen, placement: "bottom-end" }); return ( -
+
- {open && ( -
- - {hasCast && ( - - )} - {hasRelated && onToggleRelated && ( - - )} - {extra} -
- )} + + + {hasCast && ( + + )} + {hasRelated && onToggleRelated && ( + + )} + {extra} +
); } +/** One switch row inside `DetailCustomizeMenu` — a `PopoverItem`, so the rows callers pass through + * `extra` join the same roving-focus list as the built-in ones. */ export function PrefToggle({ label, on, @@ -178,9 +176,12 @@ export function PrefToggle({ onClick: () => void; }) { return ( - + ); }