From 8f6714e678ed15ca831ff271669f6d8eda5a7dc8 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 21 Aug 2026 06:16:15 +0200 Subject: [PATCH] =?UTF-8?q?refactor(e5):=20deferred=20review-tail=20(front?= =?UTF-8?q?end)=20=E2=80=94=20Carousel=20reuse=20+=20shared=20scroll=20cor?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last of the deep-review deferred reuse items: - extract the scroll-affordance machinery (watch the scroller AND its swappable/ lazily-growing children, diff two edge booleans) that useScrollFade (vertical) and Carousel's useHScroll (horizontal) each hand-duplicated into one shared lib/useScrollEdges(axis); both hooks now just map start/end to their own shape. - migrate PlexInfo's two remaining hand-rolled `flex overflow-x-auto` strips — the cast row and the collection strips — to the S1 primitive, so they get the edge fade + hover/focus scroll buttons (only on the clipped side) for free, matching the Discover rails / CastStrip / RelatedStrip. Pure refactor, no behaviour change. Gate green (tsc, vitest 351, eslint, prettier, knip). Browser-verified on a movie info page: cast row + collection strip render as Carousels with the edge fade and the right-scroll button on the clipped cast row. --- frontend/src/components/Carousel.tsx | 47 +++--------------- frontend/src/components/PlexInfo.tsx | 9 ++-- frontend/src/lib/useScrollEdges.ts | 62 +++++++++++++++++++++++ frontend/src/lib/useScrollFade.ts | 73 ++++++---------------------- 4 files changed, 90 insertions(+), 101 deletions(-) create mode 100644 frontend/src/lib/useScrollEdges.ts diff --git a/frontend/src/components/Carousel.tsx b/frontend/src/components/Carousel.tsx index b30b48e6..31834026 100644 --- a/frontend/src/components/Carousel.tsx +++ b/frontend/src/components/Carousel.tsx @@ -1,7 +1,8 @@ -import { useCallback, useEffect, useState, type ReactNode } from "react"; +import { useCallback, type ReactNode } from "react"; import { ChevronLeft, ChevronRight } from "lucide-react"; import { useTranslation } from "react-i18next"; import clsx from "clsx"; +import { useScrollEdges } from "../lib/useScrollEdges"; // Shared horizontal rail — the one place the app's "row of cards you scroll sideways" lives, replacing // the hand-rolled `flex gap overflow-x-auto` strips (Cast / Related / collections) and the future @@ -19,50 +20,18 @@ function scrollBehavior(): ScrollBehavior { return reduce ? "auto" : "smooth"; } -// Horizontal scroll-affordance: tracks whether the scroller is clipped on the left / right so the rail -// can show a button + fade only where there's more to see. The X-axis twin of useScrollFade — same -// node-state callback ref (re-runs on attach) and same watch-the-children discipline (a rail whose -// cards load in later grows scrollWidth without ever firing a scroll event), and it also hands back the -// element so the buttons can scroll it. +// Horizontal scroll-affordance: whether the scroller is clipped on the left / right, so the rail shows +// a button + fade only where there's more to see. The X-axis face of the shared `useScrollEdges` core +// (the same watch-the-scroller-and-its-children discipline as the vertical `useScrollFade`), plus the +// element itself so the paging buttons can scroll it. function useHScroll(): { ref: (node: HTMLElement | null) => void; el: HTMLElement | null; left: boolean; right: boolean; } { - const [el, setEl] = useState(null); - const [edge, setEdge] = useState({ left: false, right: false }); - - useEffect(() => { - if (!el) return; - const update = () => { - const left = el.scrollLeft > 1; - const right = el.scrollLeft + el.clientWidth < el.scrollWidth - 1; - setEdge((prev) => (prev.left === left && prev.right === right ? prev : { left, right })); - }; - update(); - el.addEventListener("scroll", update, { passive: true }); - const ro = new ResizeObserver(update); - ro.observe(el); - for (const child of Array.from(el.children)) ro.observe(child); - const mo = new MutationObserver((records) => { - for (const rec of records) { - for (const node of Array.from(rec.addedNodes)) - if (node instanceof Element) ro.observe(node); - for (const node of Array.from(rec.removedNodes)) - if (node instanceof Element) ro.unobserve(node); - } - update(); - }); - mo.observe(el, { childList: true }); - return () => { - el.removeEventListener("scroll", update); - ro.disconnect(); - mo.disconnect(); - }; - }, [el]); - - return { ref: setEl, el, left: edge.left, right: edge.right }; + const { ref, el, start, end } = useScrollEdges("x"); + return { ref, el, left: start, right: end }; } const FADE_PX = 28; diff --git a/frontend/src/components/PlexInfo.tsx b/frontend/src/components/PlexInfo.tsx index 3788ef31..ca799d19 100644 --- a/frontend/src/components/PlexInfo.tsx +++ b/frontend/src/components/PlexInfo.tsx @@ -15,6 +15,7 @@ import { useDetailPrefs, } from "../lib/plexDetailUi"; import { usePref } from "../lib/usePref"; +import { Carousel } from "./Carousel"; import PlexCollectionEditor from "./PlexCollectionEditor"; import PlexPlaylistAdd from "./PlexPlaylistAdd"; @@ -614,7 +615,7 @@ export default function PlexInfo({

{t("plex.info.cast")}

-
+ {cast.map((c, i) => { const clickable = !!(onOpenPerson || onFilter); const inner = ( @@ -669,7 +670,7 @@ export default function PlexInfo({
); })} - +
)} @@ -694,7 +695,7 @@ export default function PlexInfo({ )} -
+ {col.items.map((m) => (
: null} ))} - + ))} diff --git a/frontend/src/lib/useScrollEdges.ts b/frontend/src/lib/useScrollEdges.ts new file mode 100644 index 00000000..0f86c0fa --- /dev/null +++ b/frontend/src/lib/useScrollEdges.ts @@ -0,0 +1,62 @@ +import { useEffect, useState } from "react"; + +// The shared scroll-affordance core behind `useScrollFade` (vertical edge fade) and the Carousel's +// `useHScroll` (horizontal scroll buttons). Both need the exact same machinery — track whether a +// scroller is clipped at the START and END of ONE axis, watching the scroller AND its children so the +// edges re-evaluate even when no `scroll` event fires (a rail whose cards load in later grows +// scrollWidth/scrollHeight silently; a Suspense fallback gets SWAPPED for the real content). This is +// that machinery, parameterised by axis; the two hooks just map `start`/`end` to their own shape. +// +// Uses a node-state callback ref (not useRef) so the effect (re)runs whenever the element attaches — +// e.g. a SidePanel that mounts collapsed and only renders its scroll body once expanded still wires up. +// `start`/`end` = there is more content before / after the visible window ("y": top/bottom, "x": +// left/right). The boolean-diff bail-out matters: `scroll` fires per frame and the page scroller's hook +// sits above the whole tree, so returning a new object each frame would re-render everything. +export function useScrollEdges(axis: "x" | "y"): { + ref: (node: HTMLElement | null) => void; + el: HTMLElement | null; + start: boolean; + end: boolean; +} { + const [el, setEl] = useState(null); + const [edge, setEdge] = useState({ start: false, end: false }); + + useEffect(() => { + if (!el) return; + const update = () => { + const pos = axis === "x" ? el.scrollLeft : el.scrollTop; + const client = axis === "x" ? el.clientWidth : el.clientHeight; + const scroll = axis === "x" ? el.scrollWidth : el.scrollHeight; + const start = pos > 1; + const end = pos + client < scroll - 1; + setEdge((prev) => (prev.start === start && prev.end === end ? prev : { start, end })); + }; + update(); + el.addEventListener("scroll", update, { passive: true }); + // Watch the scroller AND its content: content that grows/shrinks/decrypts/filters changes neither + // the scroller's own box nor its scroll offset, so without the children nothing re-evaluates. + const ro = new ResizeObserver(update); + ro.observe(el); + for (const child of Array.from(el.children)) ro.observe(child); + // Children get SWAPPED (a lazy page replacing its Suspense fallback), and a ResizeObserver holding a + // detached node simply never fires again — so follow the child list, observing/unobserving exactly + // the nodes named in each record (O(1) per change, not a re-observe of a long list on every append). + const mo = new MutationObserver((records) => { + for (const rec of records) { + for (const node of Array.from(rec.addedNodes)) + if (node instanceof Element) ro.observe(node); + for (const node of Array.from(rec.removedNodes)) + if (node instanceof Element) ro.unobserve(node); + } + update(); + }); + mo.observe(el, { childList: true }); + return () => { + el.removeEventListener("scroll", update); + ro.disconnect(); + mo.disconnect(); + }; + }, [el, axis]); + + return { ref: setEl, el, start: edge.start, end: edge.end }; +} diff --git a/frontend/src/lib/useScrollFade.ts b/frontend/src/lib/useScrollFade.ts index a20a0ed4..d64d879c 100644 --- a/frontend/src/lib/useScrollFade.ts +++ b/frontend/src/lib/useScrollFade.ts @@ -1,15 +1,15 @@ -import { useEffect, useState, type CSSProperties } from "react"; +import { type CSSProperties } from "react"; +import { useScrollEdges } from "./useScrollEdges"; -// Scroll-affordance for a scroll container (typically one whose native scrollbar is hidden with -// `.no-scrollbar`, but the page scroller keeps its bar and takes the fade anyway): -// fades the top/bottom edge of the content to hint there's more to scroll, but only on the edge -// that's actually clipped (no fade at a boundary, none when it all fits). Returns a callback ref -// to attach to the scroller and the mask `style` to spread onto it. Uses a node-state callback -// ref (not useRef) so the effect (re)runs whenever the element attaches — e.g. a SidePanel that -// mounts collapsed and only renders its scroll body once expanded still wires up correctly. -// `fadeTop` gates the top edge only. Pass false when the scroller's top is a sticky element (a -// table's sticky header): fading there would eat into the header instead of hinting at content, and -// the header would visibly change as the fade turns on with the first scroll. +// Vertical scroll-affordance for a scroll container (typically one whose native scrollbar is hidden +// with `.no-scrollbar`, but the page scroller keeps its bar and takes the fade anyway): fades the +// top/bottom edge of the content to hint there's more to scroll, but only on the edge that's actually +// clipped (no fade at a boundary, none when it all fits). Returns a callback ref to attach to the +// scroller and the mask `style` to spread onto it. The scroller-and-children watching machinery lives +// in the shared `useScrollEdges` (the horizontal twin, Carousel's `useHScroll`, uses the same core). +// `fadeTop` gates the top edge only. Pass false when the scroller's top is a sticky element (a table's +// sticky header): fading there would eat into the header instead of hinting at content, and the header +// would visibly change as the fade turns on with the first scroll. export function useScrollFade( fadePx = 20, fadeTop = true, @@ -17,52 +17,9 @@ export function useScrollFade( ref: (node: HTMLElement | null) => void; style: CSSProperties; } { - const [el, setEl] = useState(null); - const [fade, setFade] = useState({ up: false, down: false }); - - useEffect(() => { - if (!el) return; - // Bail out when neither edge changed: `scroll` fires per frame, and the page scroller's - // hook sits above the whole page tree, so a new object here would re-render on every frame - // of every scroll. The two booleans only flip at the ends of the range. - const update = () => { - const up = el.scrollTop > 1; - const down = el.scrollTop + el.clientHeight < el.scrollHeight - 1; - setFade((prev) => (prev.up === up && prev.down === down ? prev : { up, down })); - }; - update(); - el.addEventListener("scroll", update, { passive: true }); - // Watch the scroller AND its content. Content that grows or shrinks — a lazy page loading in, - // a thread's messages decrypting, a list being filtered — changes neither the scroller's own - // box nor its scrollTop, so without the children nothing would re-evaluate the edges. - const ro = new ResizeObserver(update); - ro.observe(el); - for (const child of Array.from(el.children)) ro.observe(child); - // Children get SWAPPED, and a ResizeObserver holding a detached node simply never fires - // again. Measured: the Plex page opened with no bottom fade over a 3119px list, because the - // node observed at mount was the Suspense fallback the real page then replaced. (Same - // lazy-page race that broke BackToTop.) So follow the child list: the records name exactly - // which nodes arrived and left, which keeps this O(1) per change rather than re-observing - // every child of a long list on each append. - const mo = new MutationObserver((records) => { - for (const rec of records) { - for (const node of Array.from(rec.addedNodes)) - if (node instanceof Element) ro.observe(node); - for (const node of Array.from(rec.removedNodes)) - if (node instanceof Element) ro.unobserve(node); - } - update(); - }); - mo.observe(el, { childList: true }); - return () => { - el.removeEventListener("scroll", update); - ro.disconnect(); - mo.disconnect(); - }; - }, [el]); - - const mask = `linear-gradient(to bottom, transparent, #000 ${fade.up && fadeTop ? fadePx : 0}px, #000 calc(100% - ${ - fade.down ? fadePx : 0 + const { ref, start, end } = useScrollEdges("y"); + const mask = `linear-gradient(to bottom, transparent, #000 ${start && fadeTop ? fadePx : 0}px, #000 calc(100% - ${ + end ? fadePx : 0 }px), transparent)`; - return { ref: setEl, style: { maskImage: mask, WebkitMaskImage: mask } }; + return { ref, style: { maskImage: mask, WebkitMaskImage: mask } }; }