peter
6a7ff330f6
Ten more suppressions, and React 19 turns out to supply the missing primitive.
useEffectEvent (stable since React 19.2, which S1 brought in) is the canonical answer to the
"bind the listener once, but read the latest values" pattern these hooks were disabling the
checker for. Three of playerModalHooks' four escape hatches become honest dep lists: the
wheel-volume handler, the native-controls handover, and the fullscreen-change sync. The
contract is no longer prose — the compiler enforces it.
The fourth stays, deliberately, and now says why at the site: the YouTube player's lifecycle
effect must run once per video (re-running destroys and rebuilds the iframe), and YT.Player
takes its callbacks as an object literal, which an effect event may not be passed into.
Fixed, not suppressed:
- useDismiss captured onClose when the popover opened, so a caller that re-rendered with a
different handler while it stayed open kept getting the OLD one called. The disabled dep list
asserted the opposite ("read fresh from this run's closure"). Both the handler and the ref
list are read through refs now; three tests, the two that matter mutation-checked.
- usePref wrapped its seed in a useCallback whose dep list had to be excused. useState calls a
lazy initializer exactly once, so the memo was saving nothing; inlined.
- useCardPager clamped an out-of-range page from an effect. Adjusted during render instead —
React discards that pass rather than committing it, so no out-of-range page is ever painted.
- plexPlayerHooks' auto-skip countdown reads its marker from the ref that already existed, so
the dep list names its real inputs. That also made the set-state-in-effect finding go away:
the progress is genuinely timer-driven, and the checker can now see that.
- useUrlMirror and useChipOverflowFit only needed their stable refs listed.
Two test-harness suppressions (a render-phase probe ref, a module-level setState handle) moved
inline with the reason they exist: both exist to observe a hook from outside without changing
what is being measured.
Suppressions 99 -> 89; exhaustive-deps 35 -> 27.
55 lines
3.1 KiB
TypeScript
55 lines
3.1 KiB
TypeScript
import { useEffect, useRef, 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).
|
|
*
|
|
* 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<HTMLElement | null> | RefObject<HTMLElement | null>[],
|
|
): void {
|
|
// Registration + topmost-only Escape. `trapsTab: false`, because this hook installs NO Tab trap:
|
|
// it owns Escape and the outside-press, nothing more. Claiming the trap made a dialog underneath
|
|
// stand its own down (`anyBlockingLayerAbove`) for a layer that traps nothing, so Tab walked out
|
|
// of the dialog while one of its own menus was open — measured in a real browser on the Plex
|
|
// player, whose settings and track menus are both dismissables (deep-review round 4). Every
|
|
// consumer renders its panel INSIDE the surface below it, so that surface's trap already spans
|
|
// the panel; a portaled one would need its own trap and must not use this default.
|
|
useLayer(active, onClose, { trapsTab: false });
|
|
|
|
// Outside-press stays per-instance (the refs/positioning are the caller's): close on a mousedown
|
|
// that lands outside every registered element.
|
|
// Both arguments are read through refs so the listener can stay subscribed once per open/close —
|
|
// which is the point of the `[active]` dep list — without going STALE. The disabled dep list this
|
|
// replaces asserted that onClose was "read fresh from this run's closure"; it was not. The closure
|
|
// is built when `active` last flipped, so a caller that re-rendered with a different handler while
|
|
// the popover stayed open kept getting the old one called (regression test in useDismiss.dom.test).
|
|
// `refs` is the weaker case — the ref OBJECTS are stable, so a late-mounting node is seen either
|
|
// way — but a caller that changes WHICH refs it passes while open was equally ignored.
|
|
const onCloseRef = useRef(onClose);
|
|
onCloseRef.current = onClose;
|
|
const refsRef = useRef(refs);
|
|
refsRef.current = refs;
|
|
|
|
useEffect(() => {
|
|
if (!active) return;
|
|
const onDown = (e: MouseEvent) => {
|
|
const current = refsRef.current;
|
|
const list = Array.isArray(current) ? current : [current];
|
|
const target = e.target as Node;
|
|
if (!list.some((r) => r.current?.contains(target))) onCloseRef.current();
|
|
};
|
|
document.addEventListener("mousedown", onDown);
|
|
return () => {
|
|
document.removeEventListener("mousedown", onDown);
|
|
};
|
|
}, [active]);
|
|
}
|