From 447bac328072531978e223f215a16429db07e483 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 24 Jul 2024 16:57:20 -0400 Subject: [PATCH] Make layout reactivity less brittle Follow-up to ea2d98179cae73b46a5eeb4dfc5a1dc089027238 This took a couple of iterations to find something that works without creating update loops, but I think that by automatically informing Grid whenever a layout component is re-rendered, we'll have a much easier time ensuring that our layouts are fully reactive. --- src/grid/Grid.tsx | 78 ++++++++++++++++++--------- src/grid/GridLayout.tsx | 18 ++----- src/grid/OneOnOneLayout.tsx | 14 ++--- src/grid/SpotlightExpandedLayout.tsx | 24 +++------ src/grid/SpotlightLandscapeLayout.tsx | 20 +++---- src/grid/SpotlightPortraitLayout.tsx | 18 ++----- 6 files changed, 80 insertions(+), 92 deletions(-) diff --git a/src/grid/Grid.tsx b/src/grid/Grid.tsx index ea33a32d..4b084db5 100644 --- a/src/grid/Grid.tsx +++ b/src/grid/Grid.tsx @@ -25,10 +25,15 @@ import { CSSProperties, ComponentProps, ComponentType, + Dispatch, FC, LegacyRef, ReactNode, - useCallback, + SetStateAction, + createContext, + forwardRef, + memo, + useContext, useEffect, useMemo, useRef, @@ -113,6 +118,27 @@ function offset(element: HTMLElement, relativeTo: Element): Offset { } } +interface LayoutContext { + setGeneration: Dispatch>; +} + +const LayoutContext = createContext(null); + +/** + * Enables Grid to react to layout changes. You must call this in your Layout + * component or else Grid will not be reactive. + */ +export function useLayout(): void { + const context = useContext(LayoutContext); + if (context === null) + throw new Error("useLayout called outside of a Grid layout component"); + + // On every render, tell Grid that the layout may have changed + useEffect(() => + context.setGeneration((prev) => (prev === null ? 0 : prev + 1)), + ); +} + export interface LayoutProps { ref: LegacyRef; model: LayoutModel; @@ -158,6 +184,11 @@ interface Drag { export type DragCallback = (drag: Drag) => void; +interface LayoutMemoProps + extends LayoutProps { + Layout: ComponentType>; +} + interface Props< LayoutModel, TileModel, @@ -209,7 +240,7 @@ export function Grid< const [gridRoot, gridRef2] = useState(null); const gridRef = useMergedRefs(gridRef1, gridRef2); - const [layoutRoot, setLayoutRoot] = useState(null); + const [layoutRoot, layoutRef] = useState(null); const [generation, setGeneration] = useState(null); const tiles = useInitial(() => new Map>()); const prefersReducedMotion = usePrefersReducedMotion(); @@ -236,27 +267,22 @@ export function Grid< [tiles], ); - const layoutRef = useCallback( - (e: HTMLElement | null) => { - setLayoutRoot(e); - if (e !== null) - setGeneration(parseInt(e.getAttribute("data-generation")!)); - }, - [setLayoutRoot, setGeneration], + // We must memoize the Layout component to break the update loop where a + // render of Grid causes a re-render of Layout, which in turn re-renders Grid + const LayoutMemo = useMemo( + () => + memo( + forwardRef< + LayoutRef, + LayoutMemoProps + >(function LayoutMemo({ Layout, ...props }, ref): ReactNode { + return ; + }), + ), + [], ); - useEffect(() => { - if (layoutRoot !== null) { - const observer = new MutationObserver((mutations) => { - if (mutations.some((m) => m.type === "attributes")) { - setGeneration(parseInt(layoutRoot.getAttribute("data-generation")!)); - } - }); - - observer.observe(layoutRoot, { attributes: true }); - return (): void => observer.disconnect(); - } - }, [layoutRoot, setGeneration]); + const context: LayoutContext = useMemo(() => ({ setGeneration }), []); // Combine the tile definitions and slots together to create placed tiles const placedTiles = useMemo(() => { @@ -279,10 +305,10 @@ export function Grid< } return result; - // The rects may change due to the grid updating to a new generation, but - // eslint can't statically verify this + // The rects may change due to the grid resizing or updating to a new + // generation, but eslint can't statically verify this // eslint-disable-next-line react-hooks/exhaustive-deps - }, [gridRoot, layoutRoot, tiles, generation]); + }, [gridRoot, layoutRoot, tiles, gridBounds, generation]); // Drag state is stored in a ref rather than component state, because we use // react-spring's imperative API during gestures to improve responsiveness @@ -463,7 +489,9 @@ export function Grid< className={classNames(className, styles.grid)} style={style} > - + + + {tileTransitions((spring, { id, model, onDrag, width, height }) => ( = ({ // The "fixed" (non-scrolling) part of the layout is where the spotlight tile // lives fixed: forwardRef(function GridLayoutFixed({ model, Slot }, ref) { - const { width, height } = useObservableEagerState(minBounds); + useLayout(); const alignment = useObservableEagerState( useInitial(() => spotlightAlignment.pipe( @@ -68,10 +67,6 @@ export const makeGridLayout: CallLayout = ({ }, [model.spotlight], ); - const [generation] = useReactiveState( - (prev) => (prev === undefined ? 0 : prev + 1), - [model.spotlight === undefined, width, height, alignment], - ); const onDragSpotlight: DragCallback = useCallback( ({ xRatio, yRatio }) => @@ -83,7 +78,7 @@ export const makeGridLayout: CallLayout = ({ ); return ( -
+
{tileModel && ( = ({ // The scrolling part of the layout is where all the grid tiles live scrolling: forwardRef(function GridLayout({ model, Slot }, ref) { + useLayout(); const { width, height: minHeight } = useObservableEagerState(minBounds); const { gap, tileWidth, tileHeight } = useMemo( () => arrangeTiles(width, minHeight, model.grid.length), [width, minHeight, model.grid.length], ); - const [generation] = useReactiveState( - (prev) => (prev === undefined ? 0 : prev + 1), - [model.grid, width, minHeight], - ); - const tileModels: GridTileModel[] = useMemo( () => model.grid.map((vm) => ({ type: "grid", vm })), [model.grid], @@ -119,7 +110,6 @@ export const makeGridLayout: CallLayout = ({ return (
= ({ scrollingOnTop: false, fixed: forwardRef(function OneOnOneLayoutFixed(_props, ref) { - return
; + useLayout(); + return
; }), scrolling: forwardRef(function OneOnOneLayoutScrolling({ model, Slot }, ref) { + useLayout(); const { width, height } = useObservableEagerState(minBounds); const pipAlignmentValue = useObservableEagerState(pipAlignment); const { tileWidth, tileHeight } = useMemo( @@ -46,11 +47,6 @@ export const makeOneOnOneLayout: CallLayout = ({ [width, height], ); - const [generation] = useReactiveState( - (prev) => (prev === undefined ? 0 : prev + 1), - [width, height, pipAlignmentValue], - ); - const remoteTileModel: GridTileModel = useMemo( () => ({ type: "grid", vm: model.remote }), [model.remote], @@ -70,7 +66,7 @@ export const makeOneOnOneLayout: CallLayout = ({ ); return ( -
+
= ({ minBounds, pipAlignment }) => ({ +> = ({ pipAlignment }) => ({ scrollingOnTop: true, fixed: forwardRef(function SpotlightExpandedLayoutFixed( { model, Slot }, ref, ) { - const { width, height } = useObservableEagerState(minBounds); - - const [generation] = useReactiveState( - (prev) => (prev === undefined ? 0 : prev + 1), - [width, height, model.spotlight], - ); - + useLayout(); const spotlightTileModel: SpotlightTileModel = useMemo( () => ({ type: "spotlight", vms: model.spotlight, maximised: true }), [model.spotlight], ); return ( -
+
( - (prev) => (prev === undefined ? 0 : prev + 1), - [width, height, model.pip === undefined, pipAlignmentValue], - ); - const pipTileModel: GridTileModel | undefined = useMemo( () => model.pip && { type: "grid", vm: model.pip }, [model.pip], @@ -86,7 +74,7 @@ export const makeSpotlightExpandedLayout: CallLayout< ); return ( -
+
{pipTileModel && ( ({ type: "spotlight", @@ -46,13 +47,9 @@ export const makeSpotlightLandscapeLayout: CallLayout< }), [model.spotlight], ); - const [generation] = useReactiveState( - (prev) => (prev === undefined ? 0 : prev + 1), - [model.grid.length, width, height, model.spotlight], - ); return ( -
+
@@ -65,18 +62,15 @@ export const makeSpotlightLandscapeLayout: CallLayout< { model, Slot }, ref, ) { - const { width, height } = useObservableEagerState(minBounds); + useLayout(); + useObservableEagerState(minBounds); const tileModels: GridTileModel[] = useMemo( () => model.grid.map((vm) => ({ type: "grid", vm })), [model.grid], ); - const [generation] = useReactiveState( - (prev) => (prev === undefined ? 0 : prev + 1), - [model.spotlight.length, model.grid, width, height], - ); return ( -
+
1, diff --git a/src/grid/SpotlightPortraitLayout.tsx b/src/grid/SpotlightPortraitLayout.tsx index 6bd44524..0c8dee83 100644 --- a/src/grid/SpotlightPortraitLayout.tsx +++ b/src/grid/SpotlightPortraitLayout.tsx @@ -26,7 +26,7 @@ import { } from "./CallLayout"; import { SpotlightPortraitLayout as SpotlightPortraitLayoutModel } from "../state/CallViewModel"; import styles from "./SpotlightPortraitLayout.module.css"; -import { useReactiveState } from "../useReactiveState"; +import { useLayout } from "./Grid"; interface GridCSSProperties extends CSSProperties { "--grid-gap": string; @@ -48,7 +48,7 @@ export const makeSpotlightPortraitLayout: CallLayout< { model, Slot }, ref, ) { - const { width, height } = useObservableEagerState(minBounds); + useLayout(); const tileModel: TileModel = useMemo( () => ({ type: "spotlight", @@ -57,13 +57,9 @@ export const makeSpotlightPortraitLayout: CallLayout< }), [model.spotlight], ); - const [generation] = useReactiveState( - (prev) => (prev === undefined ? 0 : prev + 1), - [model.grid.length, width, height, model.spotlight], - ); return ( -
+
@@ -75,7 +71,8 @@ export const makeSpotlightPortraitLayout: CallLayout< { model, Slot }, ref, ) { - const { width, height } = useObservableEagerState(minBounds); + useLayout(); + const { width } = useObservableEagerState(minBounds); const { gap, tileWidth, tileHeight } = arrangeTiles( width, 0, @@ -85,15 +82,10 @@ export const makeSpotlightPortraitLayout: CallLayout< () => model.grid.map((vm) => ({ type: "grid", vm })), [model.grid], ); - const [generation] = useReactiveState( - (prev) => (prev === undefined ? 0 : prev + 1), - [model.spotlight.length, model.grid, width, height], - ); return (