diff --git a/frontend/src/components/AnchoredPopover.tsx b/frontend/src/components/AnchoredPopover.tsx index 5ea62784..bd4255f1 100644 --- a/frontend/src/components/AnchoredPopover.tsx +++ b/frontend/src/components/AnchoredPopover.tsx @@ -34,6 +34,7 @@ import { type Placement, type UseRoleProps, } from "@floating-ui/react"; +import { useLayer } from "../lib/layerStack"; // 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 @@ -136,7 +137,11 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { handleClose: safePolygon(), }); const focus = useFocus(context, { enabled: isHover }); - const dismiss = useDismiss(context); + // Escape for an interactive (non-hover) popover is dispatched through the shared `layerStack` + // (below) so it coordinates with Modals and players, not just other floating nodes — so turn + // floating-ui's own Escape off there. Outside-press stays with floating-ui (tree-aware). Hover + // tooltips keep floating-ui's Escape and are NOT layers (they mustn't make a player defer). + const dismiss = useDismiss(context, { escapeKey: isHover }); const role = useRole(context, { role: opts.role ?? (isHover ? "tooltip" : "menu") }); const listNav = useListNavigation(context, { enabled: isNav, @@ -157,6 +162,10 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { const interactions = useInteractions([click, hover, focus, dismiss, role, listNav, typeahead]); + // Register the open interactive popover as a dismissable layer (topmost-only Escape, and so a + // player/modal underneath defers to it). Hover tooltips are excluded on purpose. + useLayer(opts.open && !isHover, () => opts.onOpenChange(false)); + return { nodeId, mode, diff --git a/frontend/src/components/Modal.tsx b/frontend/src/components/Modal.tsx index 805743fc..170e3130 100644 --- a/frontend/src/components/Modal.tsx +++ b/frontend/src/components/Modal.tsx @@ -3,20 +3,12 @@ import { createPortal } from "react-dom"; import { useTranslation } from "react-i18next"; import { X } from "lucide-react"; import { useBackToClose } from "../lib/history"; +import { pushLayer, removeLayer, isTopmost } from "../lib/layerStack"; -// Stack of open modals so ESC only closes the topmost one — e.g. an error dialog over the -// tag editor: pressing ESC dismisses just the error and returns to the editor underneath. -let modalStack: number[] = []; -let nextModalId = 1; - -/** How many s are currently open. The players (which use their own window-level Escape - * listeners, not this stack) check it so their Escape defers to a Modal opened above them — - * otherwise one Escape closes the player UNDER the dialog and then the dialog too (U-C). */ -export function modalCount(): number { - return modalStack.length; -} - -// Small centered modal shell (portaled to ): backdrop + ESC + scroll-lock close. +// Small centered modal shell (portaled to ): backdrop + ESC + scroll-lock close. Each open +// modal is a layer in the shared `layerStack` so ESC only closes the topmost one — e.g. an error +// dialog over the tag editor dismisses just the error and returns to the editor underneath, and a +// player below defers to it (R8 S3). export default function Modal({ title, onClose, @@ -41,10 +33,9 @@ export default function Modal({ const onCloseRef = useRef(onClose); onCloseRef.current = onClose; useEffect(() => { - const id = nextModalId++; - modalStack.push(id); + const id = pushLayer(); const onKey = (e: KeyboardEvent) => { - if (e.key === "Escape" && modalStack[modalStack.length - 1] === id) { + if (e.key === "Escape" && isTopmost(id)) { e.stopPropagation(); onCloseRef.current(); } @@ -54,7 +45,7 @@ export default function Modal({ document.body.style.overflow = "hidden"; return () => { window.removeEventListener("keydown", onKey); - modalStack = modalStack.filter((x) => x !== id); + removeLayer(id); document.body.style.overflow = prev; }; }, []); diff --git a/frontend/src/components/PlayerModal.tsx b/frontend/src/components/PlayerModal.tsx index 23c5fd83..4aa5bd09 100644 --- a/frontend/src/components/PlayerModal.tsx +++ b/frontend/src/components/PlayerModal.tsx @@ -27,7 +27,7 @@ import { } from "lucide-react"; import Avatar from "./Avatar"; import AddToPlaylist from "./AddToPlaylist"; -import { modalCount } from "./Modal"; +import { pushLayer, removeLayer, anyLayerAbove } from "../lib/layerStack"; import DownloadButton from "./DownloadButton"; import { api, type Video, type VideoStatus } from "../lib/api"; import type { YTNamespace, YTPlayer, YTPlayerEvent } from "../lib/youtube"; @@ -490,12 +490,28 @@ export default function PlayerModal({ staleTime: 5 * 60_000, }); + // This player is a dismissable layer: register it so a Modal or popover opened ABOVE it (e.g. the + // download dialog, or — after S4 — an anchored menu) takes Escape first and this player defers. + const layerIdRef = useRef(null); + useEffect(() => { + const id = pushLayer(); + layerIdRef.current = id; + return () => { + removeLayer(id); + layerIdRef.current = null; + }; + }, []); + // Keyboard shortcuts (Esc close / F fullscreen / Space play-pause) + background scroll lock. // These fire only while focus is on our page, not inside the cross-origin player iframe — so // we focus the modal card on open and again on player-ready (autoplay can grab focus). useEffect(() => { const onKey = (e: KeyboardEvent) => { if (e.key === "Escape") { + // Defer to any layer opened above the player FIRST (a Modal like the download dialog, or an + // anchored popover): it takes the Escape and we do nothing — so we don't also step out of + // maximise or close the player underneath it. + if (layerIdRef.current != null && anyLayerAbove(layerIdRef.current)) return; // In fullscreen, let the browser's Esc exit fullscreen only — don't also close the modal. if (fullscreenElement()) return; // Same for our own maximise: Esc steps back out of it before it closes the player. @@ -504,9 +520,6 @@ export default function PlayerModal({ setMaximized(false); return; } - // Defer to any Modal opened above the player (e.g. the download dialog): its Escape closes - // it, ours would otherwise also fire and close the player underneath it (U-C). - if (modalCount() > 0) return; onClose(); return; } diff --git a/frontend/src/components/PlexPlayer.tsx b/frontend/src/components/PlexPlayer.tsx index 4dcb5509..225e089d 100644 --- a/frontend/src/components/PlexPlayer.tsx +++ b/frontend/src/components/PlexPlayer.tsx @@ -46,7 +46,7 @@ import { LS, useAccountPersistedObject } from "../lib/storage"; import { useDismiss } from "../lib/useDismiss"; import { useScrollFade } from "../lib/useScrollFade"; import { useBackToClose } from "../lib/history"; -import { modalCount } from "./Modal"; +import { pushLayer, removeLayer, anyLayerAbove } from "../lib/layerStack"; // The rich info overlay (poster/cast/ratings) reuses the same component as the card's info page. const PlexInfo = lazy(() => import("./PlexInfo")); @@ -808,6 +808,18 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) { handleBack(); }, [handleBack]); + // This player is a dismissable layer: register it so a Modal or popover opened ABOVE it (its own + // settings/tracks menus included) takes Escape first and this player defers (R8 S3). + const layerIdRef = useRef(null); + useEffect(() => { + const id = pushLayer(); + layerIdRef.current = id; + return () => { + removeLayer(id); + layerIdRef.current = null; + }; + }, []); + // --- keyboard -------------------------------------------------------------------------------- useEffect(() => { const onKey = (e: KeyboardEvent) => { @@ -874,8 +886,10 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) { else handleBack(); break; case "Escape": - // Defer to any Modal opened above the player — let its Escape close it, not ours (U-C). - if (modalCount() > 0) break; + // Defer to any layer opened above the player (Modal or anchored popover) — let it take the + // Escape, not us. A player MENU registers as a layer too, so it closes first and the next + // Escape reaches the auto-skip-cancel below (the old blanket-stopPropagation bug). + if (layerIdRef.current != null && anyLayerAbove(layerIdRef.current)) break; if (skipProgressRef.current != null) cancelAutoSkipRef.current(); else if (menuOpenRef.current) { setMenuOpen(false); diff --git a/frontend/src/lib/layerStack.ts b/frontend/src/lib/layerStack.ts new file mode 100644 index 00000000..d0380824 --- /dev/null +++ b/frontend/src/lib/layerStack.ts @@ -0,0 +1,76 @@ +import { useEffect, useRef } from "react"; + +// R8 S3 — one shared registry of open "dismissable layers" (modals, popovers, players), ordered by +// open time. It replaces the three mutually-unaware Escape systems that each guessed at ordering with +// its own `stopPropagation`: Modal's private `modalStack`, the popovers' document listeners, and the +// two players' `modalCount()` checks. Now EVERY layer keeps its own key handler but coordinates +// through this one stack — a layer acts on Escape only when it is topmost, and reads `anyAbove()` to +// know whether to defer. Because the decision is a stack lookup, not event-propagation order, the fix +// is robust to which listener happens to run first (the old stopPropagation was not). +// +// Notably this fixes the documented PlexPlayer bug: its menus register here, so the player sees a menu +// as a layer ABOVE it and defers, letting the menu's own Escape close it first — the player's +// auto-skip-cancel Escape then lands on the next press, instead of being swallowed by a blanket +// stopPropagation. (Popover-vs-popover ordering is still floating-ui's ; this stack adds +// the cross-TYPE ordering the tree can't see.) + +let stack: number[] = []; +let nextId = 1; + +export function pushLayer(): number { + const id = nextId++; + stack.push(id); + return id; +} + +export function removeLayer(id: number): void { + stack = stack.filter((x) => x !== id); +} + +export function isTopmost(id: number): boolean { + return stack.length > 0 && stack[stack.length - 1] === id; +} + +/** True when at least one layer sits above `id` — the "should I defer?" predicate for a layer with a + * richer key handler than a plain close (the players). */ +export function anyLayerAbove(id: number): boolean { + const i = stack.indexOf(id); + return i !== -1 && i < stack.length - 1; +} + +/** Register an open layer for as long as `active` is true, and get live predicates to read inside your + * own key handler. Pass `onEscape` to also install a topmost-only Escape listener (the common case — + * modals, popovers). Omit it for a layer that owns a bigger key handler and only needs `anyAbove()` + * to decide whether to defer (the players). */ +export function useLayer( + active: boolean, + onEscape?: () => void, +): { isTopmost: () => boolean; anyAbove: () => boolean } { + const idRef = useRef(null); + const escRef = useRef(onEscape); + escRef.current = onEscape; + + useEffect(() => { + if (!active) return; + const id = pushLayer(); + idRef.current = id; + let onKey: ((e: KeyboardEvent) => void) | undefined; + if (escRef.current) { + onKey = (e: KeyboardEvent) => { + if (e.key === "Escape" && isTopmost(id)) escRef.current?.(); + }; + document.addEventListener("keydown", onKey); + } + return () => { + if (onKey) document.removeEventListener("keydown", onKey); + removeLayer(id); + idRef.current = null; + }; + // Re-register only when open/closed toggles; onEscape is read fresh via escRef. + }, [active]); + + return { + isTopmost: () => idRef.current != null && isTopmost(idRef.current), + anyAbove: () => idRef.current != null && anyLayerAbove(idRef.current), + }; +} diff --git a/frontend/src/lib/useDismiss.ts b/frontend/src/lib/useDismiss.ts index 94b365bb..d5e7be2c 100644 --- a/frontend/src/lib/useDismiss.ts +++ b/frontend/src/lib/useDismiss.ts @@ -1,14 +1,25 @@ import { useEffect, type RefObject } from "react"; +import { useLayer } from "./layerStack"; /** While `active`, close (call `onClose`) on Escape or a mousedown outside ALL of the given * element(s). The reusable half of the popover/dropdown pattern that several components each * hand-rolled; positioning stays with the caller (it varies per popover). Pass the panel ref - * (and optionally a trigger ref so clicking the trigger doesn't immediately re-close). */ + * (and optionally a trigger ref so clicking the trigger doesn't immediately re-close). + * + * Escape is dispatched through the shared `layerStack` (R8 S3): the popover registers as a layer and + * closes only when it is topmost, so a Modal or another popover opened above it takes the Escape + * first, and a player below defers to it. This replaces the old blanket `stopPropagation` — which + * swallowed a player's own Escape (e.g. PlexPlayer's auto-skip-cancel) whenever any popover was open. */ export function useDismiss( active: boolean, onClose: () => void, refs: RefObject | RefObject[], ): void { + // Registration + topmost-only Escape. + useLayer(active, onClose); + + // Outside-press stays per-instance (the refs/positioning are the caller's): close on a mousedown + // that lands outside every registered element. useEffect(() => { if (!active) return; const list = Array.isArray(refs) ? refs : [refs]; @@ -16,29 +27,12 @@ export function useDismiss( const target = e.target as Node; if (!list.some((r) => r.current?.contains(target))) onClose(); }; - const onKey = (e: KeyboardEvent) => { - if (e.key === "Escape") { - // Stop the same Escape from also reaching a player's window-level listener underneath — - // this document listener bubbles first, so one Escape closes only this popover, not the - // popover AND the player behind it. (U-C quick-fix.) - // - // KNOWN LIMITATION (accepted, → R8 popover-platform): because this is a blanket - // stopPropagation, a PlexPlayer settings/tracks menu open DURING an auto-skip countdown - // swallows the Escape that would otherwise ALSO cancel the skip — Escape closes the menu, - // and the skip proceeds. Properly fixing this needs the shared layer-registry R8 builds - // (topmost-only dispatch), not a broad stopPropagation. - e.stopPropagation(); - onClose(); - } - }; document.addEventListener("mousedown", onDown); - document.addEventListener("keydown", onKey); return () => { document.removeEventListener("mousedown", onDown); - document.removeEventListener("keydown", onKey); }; - // Matches the original effects: (re)subscribe only when open/closed toggles. Refs are - // stable and onClose is read fresh from this run's closure. + // Matches the original effect: (re)subscribe only when open/closed toggles. Refs are stable and + // onClose is read fresh from this run's closure. // eslint-disable-next-line react-hooks/exhaustive-deps }, [active]); }