diff --git a/CHANGELOG.md b/CHANGELOG.md index ad67d9913..a4945424f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ ## Unreleased -- Add `displayName` property to `Group`, `Panel`, and `Separator` components for better debugging experience. +- [2a6b03f](https://github.com/bvaughn/react-resizable-panels/commit/2a6b03f67d7d8fea8483a6a69bcdaebbe1b18a7a): Add `displayName` property to `Group`, `Panel`, and `Separator` components for better debugging experience. +- [577](https://github.com/bvaughn/react-resizable-panels/pull/577): `Group` handles newly registered `Panels` + `Separators` during mount so that user code can safely call imperative APIs earlier ## 4.2.0 diff --git a/integrations/vite/tests/imperative-api-hooks.spec.tsx b/integrations/vite/tests/imperative-api-hooks.spec.tsx index b2906646f..fef998c92 100644 --- a/integrations/vite/tests/imperative-api-hooks.spec.tsx +++ b/integrations/vite/tests/imperative-api-hooks.spec.tsx @@ -10,22 +10,24 @@ test.describe("imperative API hooks", () => { { useGroupRef: true }, { useGroupCallbackRef: true } ]) { - test( + test.describe( useGroupCallbackRef ? "useGroupCallbackRef" : "useGroupRef", - async ({ page: mainPage }) => { - await goToUrl( - mainPage, - - - - - , - { usePopUpWindow, useGroupCallbackRef, useGroupRef } - ); + () => { + test("should work", async ({ page: mainPage }) => { + await goToUrl( + mainPage, + + + + + , + { usePopUpWindow, useGroupCallbackRef, useGroupRef } + ); - await expect( - mainPage.getByText("imperativeGroupApiLayout") - ).toContainText('"left": 30'); + await expect( + mainPage.getByText("imperativeGroupApiLayout") + ).toContainText('"left": 30'); + }); } ); } @@ -34,22 +36,24 @@ test.describe("imperative API hooks", () => { { usePanelRef: true }, { usePanelCallbackRef: true } ]) { - test( + test.describe( usePanelCallbackRef ? "usePanelCallbackRef" : "usePanelRef", - async ({ page: mainPage }) => { - await goToUrl( - mainPage, - - - - - , - { usePopUpWindow, usePanelCallbackRef, usePanelRef } - ); + () => { + test("should work", async ({ page: mainPage }) => { + await goToUrl( + mainPage, + + + + + , + { usePopUpWindow, usePanelCallbackRef, usePanelRef } + ); - await expect( - mainPage.getByText("imperativePanelApiSize") - ).toContainText('"asPercentage": 70'); + await expect( + mainPage.getByText("imperativePanelApiSize") + ).toContainText('"asPercentage": 70'); + }); } ); } diff --git a/lib/components/group/Group.test.tsx b/lib/components/group/Group.test.tsx index c72d97a26..e193e19d5 100644 --- a/lib/components/group/Group.test.tsx +++ b/lib/components/group/Group.test.tsx @@ -1,4 +1,5 @@ import { render } from "@testing-library/react"; +import { createRef, useEffect } from "react"; import { beforeEach, describe, expect, test, vi } from "vitest"; import { eventEmitter } from "../../global/mutableState"; import { moveSeparator } from "../../global/test/moveSeparator"; @@ -10,8 +11,8 @@ import { import { Panel } from "../panel/Panel"; import { Separator } from "../separator/Separator"; import { Group } from "./Group"; -import { createRef } from "react"; import type { GroupImperativeHandle } from "./types"; +import { useGroupRef } from "./useGroupRef"; describe("Group", () => { test("changes to defaultProps or disableCursor should not cause Group to remount", () => { @@ -211,6 +212,45 @@ describe("Group", () => { right: 65 }); }); + + // See github.com/bvaughn/react-resizable-panels/issues/576 + test("should allow layout to be read or written on mount", () => { + setElementBoundsFunction((element) => { + if (element.hasAttribute("data-panel")) { + return new DOMRect(0, 0, 50, 50); + } else { + return new DOMRect(0, 0, 100, 50); + } + }); + + function Repro() { + const groupRef = useGroupRef(); + + useEffect(() => { + const group = groupRef.current; + assert(group); + + expect(group.getLayout()).toEqual({ + left: 25, + right: 75 + }); + + // Should not throw + group.setLayout({ left: 50, right: 50 }); + }, [groupRef]); + + return ( + + + Left + + Right + + ); + } + + render(); + }); }); describe("onLayoutChange", () => { diff --git a/lib/components/group/Group.tsx b/lib/components/group/Group.tsx index 6d282f3d2..9abd1d011 100644 --- a/lib/components/group/Group.tsx +++ b/lib/components/group/Group.tsx @@ -3,6 +3,7 @@ import { useEffect, useMemo, useRef, useState } from "react"; import { mountGroup } from "../../global/mountGroup"; import { eventEmitter, read } from "../../global/mutableState"; import { layoutsEqual } from "../../global/utils/layoutsEqual"; +import { useForceUpdate } from "../../hooks/useForceUpdate"; import { useId } from "../../hooks/useId"; import { useIsomorphicLayoutEffect } from "../../hooks/useIsomorphicLayoutEffect"; import { useMergedRefs } from "../../hooks/useMergedRefs"; @@ -57,18 +58,22 @@ export function Group({ const id = useId(idProp); - const [dragActive, setDragActive] = useState(false); const elementRef = useRef(null); - const [layout, setLayout] = useState(defaultLayout ?? {}); - const [panels, setPanels] = useState([]); - const [separators, setSeparators] = useState([]); + + const [dragActive, setDragActive] = useState(false); + const [layout, setLayout] = useState(defaultLayout ?? {}); + const [panelOrSeparatorChangeSigil, forceUpdate] = useForceUpdate(); const inMemoryValuesRef = useRef<{ lastExpandedPanelSizes: { [panelIds: string]: number }; layouts: { [panelIds: string]: Layout }; + panels: RegisteredPanel[]; + separators: RegisteredSeparator[]; }>({ lastExpandedPanelSizes: {}, - layouts: {} + layouts: {}, + panels: [], + separators: [] }); const mergedRef = useMergedRefs(elementRef, elementRefProp); @@ -80,23 +85,41 @@ export function Group({ id, orientation, registerPanel: (panel: RegisteredPanel) => { - setPanels((prev) => sortByElementOffset(orientation, [...prev, panel])); + const inMemoryValues = inMemoryValuesRef.current; + inMemoryValues.panels = sortByElementOffset(orientation, [ + ...inMemoryValues.panels, + panel + ]); + + forceUpdate(); + return () => { - setPanels((prev) => prev.filter((current) => current !== panel)); + inMemoryValues.panels = inMemoryValues.panels.filter( + (current) => current !== panel + ); + + forceUpdate(); }; }, registerSeparator: (separator: RegisteredSeparator) => { - setSeparators((prev) => - sortByElementOffset(orientation, [...prev, separator]) - ); + const inMemoryValues = inMemoryValuesRef.current; + inMemoryValues.separators = sortByElementOffset(orientation, [ + ...inMemoryValues.separators, + separator + ]); + + forceUpdate(); + return () => { - setSeparators((prev) => - prev.filter((current) => current !== separator) + inMemoryValues.separators = inMemoryValues.separators.filter( + (current) => current !== separator ); + + forceUpdate(); }; } }), - [id, orientation] + [id, forceUpdate, orientation] ); const stableProps = useStableObject({ @@ -114,6 +137,8 @@ export function Group({ return; } + const inMemoryValues = inMemoryValuesRef.current; + const group: RegisteredGroup = { defaultLayout: stableProps.defaultLayout, disableCursor: !!stableProps.disableCursor, @@ -124,8 +149,8 @@ export function Group({ inMemoryValuesRef.current.lastExpandedPanelSizes, inMemoryLayouts: inMemoryValuesRef.current.layouts, orientation, - panels, - separators + panels: inMemoryValues.panels, + separators: inMemoryValues.separators }; registeredGroupRef.current = group; @@ -138,10 +163,8 @@ export function Group({ const { defaultLayoutDeferred, derivedPanelConstraints, layout } = match; if (!defaultLayoutDeferred && derivedPanelConstraints.length > 0) { - // This indicates that the Group has not finished mounting yet - // Likely because it has been rendered inside of a hidden DOM subtree - // Ignore layouts in this case because they will not have been validated setLayout(layout); + onLayoutChangeStable?.(layout); } } @@ -182,6 +205,7 @@ export function Group({ } setLayout(layout); + onLayoutChangeStable?.(layout); } } @@ -199,8 +223,7 @@ export function Group({ id, onLayoutChangeStable, orientation, - panels, - separators, + panelOrSeparatorChangeSigil, stableProps ]); diff --git a/lib/components/group/useDefaultLayout.test.tsx b/lib/components/group/useDefaultLayout.test.tsx index a98249966..3a298addd 100644 --- a/lib/components/group/useDefaultLayout.test.tsx +++ b/lib/components/group/useDefaultLayout.test.tsx @@ -83,41 +83,81 @@ describe("useDefaultLayout", () => { }); // See github.com/bvaughn/react-resizable-panels/pull/540 - test("should ignore invalid layouts (num panels mismatch)", () => { - const groupRef = createRef(); + describe("should ignore temporarily invalid layouts", () => { + test("on mount (ids mismatch)", () => { + const groupRef = createRef(); - setDefaultElementBounds(new DOMRect(0, 0, 100, 50)); + setDefaultElementBounds(new DOMRect(0, 0, 100, 50)); - function Test({ hideMiddlePanel }: { hideMiddlePanel?: boolean }) { - const { defaultLayout, onLayoutChange } = useDefaultLayout({ - debounceSaveMs: 0, - id: "test-group-id", - storage: localStorage - }); + render( + + + + + ); - // Prime the local storage - onLayoutChange({ - left: 20, - middle: 30, - right: 50 - }); + expect(groupRef.current?.getLayout()).toMatchInlineSnapshot(` + { + "after": 50, + "before": 50, + } + `); + }); + + test("on mount (num panels mismatch)", () => { + const groupRef = createRef(); - return ( + setDefaultElementBounds(new DOMRect(0, 0, 100, 50)); + + render( - left - {!hideMiddlePanel && middle} - right + + ); - } - const { rerender } = render(); + expect(groupRef.current?.getLayout()).toMatchInlineSnapshot(` + { + "left": 50, + "right": 50, + } + `); + }); - expect(groupRef.current?.getLayout()).toMatchInlineSnapshot(` + test("after update (num panels mismatch)", () => { + const groupRef = createRef(); + + setDefaultElementBounds(new DOMRect(0, 0, 100, 50)); + + const { rerender } = render( + + + + + + ); + + expect(groupRef.current?.getLayout()).toMatchInlineSnapshot(` { "left": 20, "middle": 30, @@ -125,14 +165,27 @@ describe("useDefaultLayout", () => { } `); - rerender(); + rerender( + + left + right + + ); - expect(groupRef.current?.getLayout()).toMatchInlineSnapshot(` + expect(groupRef.current?.getLayout()).toMatchInlineSnapshot(` { "left": 50, "right": 50, } `); + }); }); test("should save separate layouts per panel group when panelIds param is present", () => { diff --git a/lib/components/separator/Separator.tsx b/lib/components/separator/Separator.tsx index 5c3dbdd63..feb1e5cfc 100644 --- a/lib/components/separator/Separator.tsx +++ b/lib/components/separator/Separator.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState } from "react"; +import { useRef, useState } from "react"; import { eventEmitter } from "../../global/mutableState"; import type { InteractionState } from "../../global/types"; import { calculateSeparatorAriaValues } from "../../global/utils/calculateSeparatorAriaValues"; @@ -28,7 +28,7 @@ import type { RegisteredSeparator, SeparatorProps } from "./types"; export function Separator({ children, className, - elementRef, + elementRef: elementRefProp, id: idProp, style, ...rest @@ -44,9 +44,10 @@ export function Separator({ const [dragState, setDragState] = useState("inactive"); - const [element, setElement] = useState(null); - const mergedRef = useMergedRefs(setElement, elementRef); + const elementRef = useRef(null); + + const mergedRef = useMergedRefs(elementRef, elementRefProp); const { id: groupId, @@ -60,6 +61,7 @@ export function Separator({ // Register Separator with parent Group // Listen to global state for drag state related to this Separator useIsomorphicLayoutEffect(() => { + const element = elementRef.current; if (element !== null) { const separator: RegisteredSeparator = { element, @@ -117,7 +119,7 @@ export function Separator({ unregisterSeparator(); }; } - }, [element, groupId, id, registerSeparator]); + }, [groupId, id, registerSeparator]); return (
setCount((prevCount) => prevCount + 1), []); + const forceUpdate = useCallback(() => setSigil({}), []); + + return [sigil as unknown, forceUpdate] as const; }