Skip to content

Commit df7a3c4

Browse files
authored
Group handles newly registered Panels + Separators during mount (#577)
Previously the Group ref object did not work properly in this scenario: ```tsx function Repro() { const groupRef = useGroupRef(); useEffect(() => { const group = groupRef.current; if (group) { group.setLayout({ left: 50, right: 50 }); } }, []); return ( <Group groupRef={groupRef}> <Panel id="left">Left</Panel> <Panel id="right">Right</Panel> </Group> ); } ``` The reason being that Group's internal initialization was like so: 1. Panels register themselves with their parent group on mount (in an effect) 2. Group adds pending panels into (React) state which schedules a re-render once all panels have finished registering - External effects run now, meaning they can't safely read/write layout 3. After the re-render, Group processes the newly registered panels in an effect After this commit, the internal timing of when a Group initialized itself is like so: 1. Panels register themselves with their parent group on mount (in an effect) 2. Group processes pending Panels (and Separators) in the next effect 3. External code can safely read/write layout during the next effect as well --- Resolves #576
1 parent 0a89acf commit df7a3c4

File tree

7 files changed

+208
-83
lines changed

7 files changed

+208
-83
lines changed

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
## Unreleased
44

5-
- Add `displayName` property to `Group`, `Panel`, and `Separator` components for better debugging experience.
5+
- [2a6b03f](https://github.com/bvaughn/react-resizable-panels/commit/2a6b03f67d7d8fea8483a6a69bcdaebbe1b18a7a): Add `displayName` property to `Group`, `Panel`, and `Separator` components for better debugging experience.
6+
- [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
67

78
## 4.2.0
89

integrations/vite/tests/imperative-api-hooks.spec.tsx

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,24 @@ test.describe("imperative API hooks", () => {
1010
{ useGroupRef: true },
1111
{ useGroupCallbackRef: true }
1212
]) {
13-
test(
13+
test.describe(
1414
useGroupCallbackRef ? "useGroupCallbackRef" : "useGroupRef",
15-
async ({ page: mainPage }) => {
16-
await goToUrl(
17-
mainPage,
18-
<Group>
19-
<Panel id="left" defaultSize="30" />
20-
<Separator />
21-
<Panel id="right" />
22-
</Group>,
23-
{ usePopUpWindow, useGroupCallbackRef, useGroupRef }
24-
);
15+
() => {
16+
test("should work", async ({ page: mainPage }) => {
17+
await goToUrl(
18+
mainPage,
19+
<Group>
20+
<Panel id="left" defaultSize="30" />
21+
<Separator />
22+
<Panel id="right" />
23+
</Group>,
24+
{ usePopUpWindow, useGroupCallbackRef, useGroupRef }
25+
);
2526

26-
await expect(
27-
mainPage.getByText("imperativeGroupApiLayout")
28-
).toContainText('"left": 30');
27+
await expect(
28+
mainPage.getByText("imperativeGroupApiLayout")
29+
).toContainText('"left": 30');
30+
});
2931
}
3032
);
3133
}
@@ -34,22 +36,24 @@ test.describe("imperative API hooks", () => {
3436
{ usePanelRef: true },
3537
{ usePanelCallbackRef: true }
3638
]) {
37-
test(
39+
test.describe(
3840
usePanelCallbackRef ? "usePanelCallbackRef" : "usePanelRef",
39-
async ({ page: mainPage }) => {
40-
await goToUrl(
41-
mainPage,
42-
<Group>
43-
<Panel id="left" defaultSize="30" />
44-
<Separator />
45-
<Panel id="right" />
46-
</Group>,
47-
{ usePopUpWindow, usePanelCallbackRef, usePanelRef }
48-
);
41+
() => {
42+
test("should work", async ({ page: mainPage }) => {
43+
await goToUrl(
44+
mainPage,
45+
<Group>
46+
<Panel id="left" defaultSize="30" />
47+
<Separator />
48+
<Panel id="right" />
49+
</Group>,
50+
{ usePopUpWindow, usePanelCallbackRef, usePanelRef }
51+
);
4952

50-
await expect(
51-
mainPage.getByText("imperativePanelApiSize")
52-
).toContainText('"asPercentage": 70');
53+
await expect(
54+
mainPage.getByText("imperativePanelApiSize")
55+
).toContainText('"asPercentage": 70');
56+
});
5357
}
5458
);
5559
}

lib/components/group/Group.test.tsx

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { render } from "@testing-library/react";
2+
import { createRef, useEffect } from "react";
23
import { beforeEach, describe, expect, test, vi } from "vitest";
34
import { eventEmitter } from "../../global/mutableState";
45
import { moveSeparator } from "../../global/test/moveSeparator";
@@ -10,8 +11,8 @@ import {
1011
import { Panel } from "../panel/Panel";
1112
import { Separator } from "../separator/Separator";
1213
import { Group } from "./Group";
13-
import { createRef } from "react";
1414
import type { GroupImperativeHandle } from "./types";
15+
import { useGroupRef } from "./useGroupRef";
1516

1617
describe("Group", () => {
1718
test("changes to defaultProps or disableCursor should not cause Group to remount", () => {
@@ -211,6 +212,45 @@ describe("Group", () => {
211212
right: 65
212213
});
213214
});
215+
216+
// See github.com/bvaughn/react-resizable-panels/issues/576
217+
test("should allow layout to be read or written on mount", () => {
218+
setElementBoundsFunction((element) => {
219+
if (element.hasAttribute("data-panel")) {
220+
return new DOMRect(0, 0, 50, 50);
221+
} else {
222+
return new DOMRect(0, 0, 100, 50);
223+
}
224+
});
225+
226+
function Repro() {
227+
const groupRef = useGroupRef();
228+
229+
useEffect(() => {
230+
const group = groupRef.current;
231+
assert(group);
232+
233+
expect(group.getLayout()).toEqual({
234+
left: 25,
235+
right: 75
236+
});
237+
238+
// Should not throw
239+
group.setLayout({ left: 50, right: 50 });
240+
}, [groupRef]);
241+
242+
return (
243+
<Group groupRef={groupRef}>
244+
<Panel defaultSize="25" id="left">
245+
Left
246+
</Panel>
247+
<Panel id="right">Right</Panel>
248+
</Group>
249+
);
250+
}
251+
252+
render(<Repro />);
253+
});
214254
});
215255

216256
describe("onLayoutChange", () => {

lib/components/group/Group.tsx

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { useEffect, useMemo, useRef, useState } from "react";
33
import { mountGroup } from "../../global/mountGroup";
44
import { eventEmitter, read } from "../../global/mutableState";
55
import { layoutsEqual } from "../../global/utils/layoutsEqual";
6+
import { useForceUpdate } from "../../hooks/useForceUpdate";
67
import { useId } from "../../hooks/useId";
78
import { useIsomorphicLayoutEffect } from "../../hooks/useIsomorphicLayoutEffect";
89
import { useMergedRefs } from "../../hooks/useMergedRefs";
@@ -57,18 +58,22 @@ export function Group({
5758

5859
const id = useId(idProp);
5960

60-
const [dragActive, setDragActive] = useState(false);
6161
const elementRef = useRef<HTMLDivElement | null>(null);
62-
const [layout, setLayout] = useState<Layout>(defaultLayout ?? {});
63-
const [panels, setPanels] = useState<RegisteredPanel[]>([]);
64-
const [separators, setSeparators] = useState<RegisteredSeparator[]>([]);
62+
63+
const [dragActive, setDragActive] = useState(false);
64+
const [layout, setLayout] = useState(defaultLayout ?? {});
65+
const [panelOrSeparatorChangeSigil, forceUpdate] = useForceUpdate();
6566

6667
const inMemoryValuesRef = useRef<{
6768
lastExpandedPanelSizes: { [panelIds: string]: number };
6869
layouts: { [panelIds: string]: Layout };
70+
panels: RegisteredPanel[];
71+
separators: RegisteredSeparator[];
6972
}>({
7073
lastExpandedPanelSizes: {},
71-
layouts: {}
74+
layouts: {},
75+
panels: [],
76+
separators: []
7277
});
7378

7479
const mergedRef = useMergedRefs(elementRef, elementRefProp);
@@ -80,23 +85,41 @@ export function Group({
8085
id,
8186
orientation,
8287
registerPanel: (panel: RegisteredPanel) => {
83-
setPanels((prev) => sortByElementOffset(orientation, [...prev, panel]));
88+
const inMemoryValues = inMemoryValuesRef.current;
89+
inMemoryValues.panels = sortByElementOffset(orientation, [
90+
...inMemoryValues.panels,
91+
panel
92+
]);
93+
94+
forceUpdate();
95+
8496
return () => {
85-
setPanels((prev) => prev.filter((current) => current !== panel));
97+
inMemoryValues.panels = inMemoryValues.panels.filter(
98+
(current) => current !== panel
99+
);
100+
101+
forceUpdate();
86102
};
87103
},
88104
registerSeparator: (separator: RegisteredSeparator) => {
89-
setSeparators((prev) =>
90-
sortByElementOffset(orientation, [...prev, separator])
91-
);
105+
const inMemoryValues = inMemoryValuesRef.current;
106+
inMemoryValues.separators = sortByElementOffset(orientation, [
107+
...inMemoryValues.separators,
108+
separator
109+
]);
110+
111+
forceUpdate();
112+
92113
return () => {
93-
setSeparators((prev) =>
94-
prev.filter((current) => current !== separator)
114+
inMemoryValues.separators = inMemoryValues.separators.filter(
115+
(current) => current !== separator
95116
);
117+
118+
forceUpdate();
96119
};
97120
}
98121
}),
99-
[id, orientation]
122+
[id, forceUpdate, orientation]
100123
);
101124

102125
const stableProps = useStableObject({
@@ -114,6 +137,8 @@ export function Group({
114137
return;
115138
}
116139

140+
const inMemoryValues = inMemoryValuesRef.current;
141+
117142
const group: RegisteredGroup = {
118143
defaultLayout: stableProps.defaultLayout,
119144
disableCursor: !!stableProps.disableCursor,
@@ -124,8 +149,8 @@ export function Group({
124149
inMemoryValuesRef.current.lastExpandedPanelSizes,
125150
inMemoryLayouts: inMemoryValuesRef.current.layouts,
126151
orientation,
127-
panels,
128-
separators
152+
panels: inMemoryValues.panels,
153+
separators: inMemoryValues.separators
129154
};
130155

131156
registeredGroupRef.current = group;
@@ -138,10 +163,8 @@ export function Group({
138163
const { defaultLayoutDeferred, derivedPanelConstraints, layout } = match;
139164

140165
if (!defaultLayoutDeferred && derivedPanelConstraints.length > 0) {
141-
// This indicates that the Group has not finished mounting yet
142-
// Likely because it has been rendered inside of a hidden DOM subtree
143-
// Ignore layouts in this case because they will not have been validated
144166
setLayout(layout);
167+
145168
onLayoutChangeStable?.(layout);
146169
}
147170
}
@@ -182,6 +205,7 @@ export function Group({
182205
}
183206

184207
setLayout(layout);
208+
185209
onLayoutChangeStable?.(layout);
186210
}
187211
}
@@ -199,8 +223,7 @@ export function Group({
199223
id,
200224
onLayoutChangeStable,
201225
orientation,
202-
panels,
203-
separators,
226+
panelOrSeparatorChangeSigil,
204227
stableProps
205228
]);
206229

0 commit comments

Comments
 (0)