diff --git a/frontend/eslint-suppressions.json b/frontend/eslint-suppressions.json index c714654c..594be87a 100644 --- a/frontend/eslint-suppressions.json +++ b/frontend/eslint-suppressions.json @@ -183,25 +183,5 @@ "react-hooks/set-state-in-effect": { "count": 1 } - }, - "src/lib/plexPlayerHooks.subtitle.dom.test.tsx": { - "react-hooks/immutability": { - "count": 1 - } - }, - "src/lib/plexPlayerHooks.ts": { - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, - "src/lib/useCardPager.ts": { - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, - "src/lib/useFocusKeeper.restore.dom.test.tsx": { - "react-hooks/globals": { - "count": 1 - } } } diff --git a/frontend/src/lib/playerModalHooks.ts b/frontend/src/lib/playerModalHooks.ts index b81e656c..ec2f52e9 100644 --- a/frontend/src/lib/playerModalHooks.ts +++ b/frontend/src/lib/playerModalHooks.ts @@ -1,5 +1,6 @@ import { useEffect, + useEffectEvent, useRef, useState, type Dispatch, @@ -133,20 +134,20 @@ export function useYtVolume({ // Mouse-wheel volume over the WHOLE modal. Non-passive so preventDefault stops the modal/page // scrolling. The cross-origin iframe swallows wheel over its native strips, but the central // interaction overlay is our element (wheel there bubbles here), and everything outside works. + const onWheelSpin = useEffectEvent((e: WheelEvent) => { + // Ctrl/⌘+wheel is the browser's zoom gesture — never swallow it. + if (e.ctrlKey || e.metaKey) return; + e.preventDefault(); + focusModal(); // re-grab focus if it had slipped into the native controls + nudgeVolume(e.deltaY < 0 ? 5 : -5); + }); useEffect(() => { const el = dialogRef.current; if (!el) return; - const onWheel = (e: WheelEvent) => { - // Ctrl/⌘+wheel is the browser's zoom gesture — never swallow it. - if (e.ctrlKey || e.metaKey) return; - e.preventDefault(); - focusModal(); // re-grab focus if it had slipped into the native controls - nudgeVolume(e.deltaY < 0 ? 5 : -5); - }; + const onWheel = (e: WheelEvent) => onWheelSpin(e); el.addEventListener("wheel", onWheel, { passive: false }); return () => el.removeEventListener("wheel", onWheel); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [dialogRef]); // On unmount, clear the flash timer and flush any pending debounced write — closing the player right // after a wheel spin (the normal way to leave) must not drop the level the user just set. @@ -183,18 +184,19 @@ export function useNativeControlsYield({ setNativeControls: (on: boolean) => void; } { const [nativeControls, setNativeControls] = useState(false); + const handOverToIframe = useEffectEvent(() => { + const p = playerRef.current; + const f = p && typeof p.getIframe === "function" ? p.getIframe() : null; + if (f && document.activeElement === f) setNativeControls(true); + }); + const rearmOurControls = useEffectEvent(() => { + setNativeControls(false); + focusModal(); + }); useEffect(() => { - const iframeFocused = () => { - const p = playerRef.current; - const f = p && typeof p.getIframe === "function" ? p.getIframe() : null; - return !!f && document.activeElement === f; - }; // activeElement settles on the tick after blur, so defer the check. - const onBlur = () => window.setTimeout(() => iframeFocused() && setNativeControls(true), 0); - const rearm = () => { - setNativeControls(false); - focusModal(); - }; + const onBlur = () => window.setTimeout(() => handOverToIframe(), 0); + const rearm = () => rearmOurControls(); const stage = stageRef.current; window.addEventListener("blur", onBlur); window.addEventListener("focus", rearm); @@ -204,8 +206,7 @@ export function useNativeControlsYield({ window.removeEventListener("focus", rearm); stage?.removeEventListener("mouseleave", rearm); }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [stageRef]); return { nativeControls, setNativeControls }; } @@ -264,19 +265,17 @@ export function useYtBigView({ // YouTube's own fullscreen — entered from its control bar, left by its button or Esc. On LEAVING, // focus is stuck in the iframe (the user had to click into the player to reach YT's button), so our // window-level shortcuts never fire — take focus back so F/Space/arrows work the moment fullscreen ends. + const syncFullscreen = useEffectEvent(() => { + const fsEl = fullscreenElement(); + const p = playerRef.current; + const frame = p && typeof p.getIframe === "function" ? p.getIframe() : null; + const ours = !!fsEl && (fsEl === stageRef.current || fsEl === frame); + isFullscreenRef.current = ours; // read by the once-bound keydown handler + setIsFullscreen(ours); // drives the overlay's pointer-events + if (!fsEl) focusModal(); + }); useEffect(() => { - const onChange = () => { - const fsEl = fullscreenElement(); - // Refs only — this listener is bound once, so it must not close over render values. - const p = playerRef.current; - const frame = p && typeof p.getIframe === "function" ? p.getIframe() : null; - const ours = !!fsEl && (fsEl === stageRef.current || fsEl === frame); - isFullscreenRef.current = ours; // read by the once-bound keydown handler - setIsFullscreen(ours); // drives the overlay's pointer-events - if (!fsEl) focusModal(); - }; - return onFullscreenChange(onChange); // both spellings — see lib/fullscreen - // eslint-disable-next-line react-hooks/exhaustive-deps + return onFullscreenChange(() => syncFullscreen()); // both spellings — see lib/fullscreen }, []); return { @@ -485,6 +484,13 @@ export function useYtProgress({ } playerRef.current = null; }; + // ONE PLAYER PER VIDEO is the invariant, and it is why this dep list is deliberately just the + // id: re-running would destroy and rebuild the YouTube iframe, losing the playing position and + // every listener attached to it. Nothing in the body reads a render value — the mutable state it + // needs mid-session (currentIdRef, modeRef, navRef, sessionPosRef, autoMarkedRef, + // errorStreakRef, playerRef) is all behind refs, which is the enforcement, not this comment. + // useEffectEvent cannot help here the way it does above: YT.Player takes its callbacks as an + // object literal, and an effect event may not be passed around as a value. // eslint-disable-next-line react-hooks/exhaustive-deps }, [activeId]); } diff --git a/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx b/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx index a56c0cdf..eb8b6017 100644 --- a/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx +++ b/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx @@ -330,6 +330,7 @@ function TrackHarness() { absRef: { current: 3600 }, loadStartRef: { current: 0 }, }); + // eslint-disable-next-line react-hooks/immutability -- a TEST probe: the hook's return has to reach the assertions, and a render-phase ref write is the only way to read it from outside without a wrapper that would change what is being measured. harness.current = tracks; return (