Skip to content

Commit 2063027

Browse files
bvaughnbvaughn
authored
More gracefully handle hidden or empty group elements (#541)
Whenever a group's layout is being calculated, either on mount or after a resize, guard against a hidden (or 0-width) scenario, as this results in NaN layout values. Resolves #536
1 parent fc71d8f commit 2063027

File tree

10 files changed

+263
-74
lines changed

10 files changed

+263
-74
lines changed

‎integrations/vite/src/main.tsx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { createRoot } from "react-dom/client";
33
import { BrowserRouter, Route, Routes } from "react-router";
44
import "./index.css";
55
import { Decoder } from "./routes/Decoder";
6+
import { Visibility } from "./routes/Visibility";
67
import { Edges } from "./routes/Edges";
78
import { Encoder } from "./routes/Encoder";
89
import { Home } from "./routes/Home";
@@ -15,6 +16,10 @@ createRoot(document.getElementById("root")!).render(
1516
<Route path="/e2e/decoder/:encoded" element={<Decoder />} />
1617
<Route path="/e2e/edges" element={<Edges />} />
1718
<Route path="/e2e/encoder" element={<Encoder />} />
19+
<Route
20+
path="/e2e/visibility/:mode/:default/:encoded"
21+
element={<Visibility />}
22+
/>
1823
</Routes>
1924
</BrowserRouter>
2025
</StrictMode>

‎integrations/vite/src/routes/Home.tsx‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,3 @@
1-
import { Link } from "react-router";
2-
31
export function Home() {
4-
return (
5-
<ul>
6-
<li>
7-
<Link to="/e2e/encoder">e2e: encoder</Link>
8-
</li>
9-
<li>
10-
<Link to="/e2e/dynamic">e2e: dynamic</Link>
11-
</li>
12-
<li>
13-
<Link to="/e2e/edges">e2e: edges</Link>
14-
</li>
15-
</ul>
16-
);
2+
return <div>This app exists for e2e tests</div>;
173
}

‎integrations/vite/src/routes/Visibility.tsx‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { Activity as ReactActivity, useMemo, useState } from "react";
2+
import type { Layout } from "react-resizable-panels";
3+
import { useParams } from "react-router";
4+
import { Box } from "../../../../src/components/Box";
5+
import { decode } from "../../tests/utils/serializer/decode";
6+
import { DebugData } from "../components/DebugData";
7+
8+
export function Visibility() {
9+
const { default: defaultValue, encoded, mode } = useParams();
10+
11+
const [hidden, setHidden] = useState(defaultValue === "hidden");
12+
13+
const [state, setState] = useState<{
14+
onLayoutCount: number;
15+
layout: Layout;
16+
}>({
17+
layout: {},
18+
onLayoutCount: 0
19+
});
20+
21+
const children = useMemo(() => {
22+
if (!encoded) {
23+
return null;
24+
}
25+
26+
const group = decode(encoded, {
27+
groupProps: {
28+
onLayoutChange: (layout) => {
29+
setState((prev) => ({
30+
onLayoutCount: prev.onLayoutCount + 1,
31+
layout
32+
}));
33+
}
34+
}
35+
});
36+
37+
return group;
38+
}, [encoded]);
39+
40+
return (
41+
<Box className="p-2" direction="column" gap={2}>
42+
<button
43+
onClick={() => {
44+
setHidden(!hidden);
45+
}}
46+
>
47+
toggle {mode} {hidden ? "hidden" : "visible"}
48+
{" → "}
49+
{hidden ? "visible" : "hidden"}
50+
</button>
51+
{mode === "activity" ? (
52+
<ReactActivity mode={hidden ? "hidden" : "visible"}>
53+
{children}
54+
</ReactActivity>
55+
) : (
56+
<div className={hidden ? "hidden" : "block"}>{children}</div>
57+
)}
58+
<Box className={"p-2"} direction="row" gap={2} wrap>
59+
<DebugData
60+
data={{
61+
layout: state.layout,
62+
onLayoutCount: state.onLayoutCount
63+
}}
64+
/>
65+
</Box>
66+
</Box>
67+
);
68+
}

‎integrations/vite/tests/utils/goToUrl.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,20 @@ import { encode } from "./serializer/encode";
55

66
export async function goToUrl(
77
page: Page,
8-
element: ReactElement<GroupProps> | null
8+
element: ReactElement<GroupProps> | null,
9+
routeProp: string = "/e2e/decoder/"
910
) {
11+
let route = routeProp;
12+
if (route.startsWith("/")) {
13+
route = route.substring(1);
14+
}
15+
if (route.endsWith("/")) {
16+
route = route.substring(0, route.length - 1);
17+
}
18+
1019
const encodedString = element ? encode(element) : "";
1120

12-
const url = new URL(`http://localhost:3012/e2e/decoder/${encodedString}`);
21+
const url = new URL(`http://localhost:3012/${route}/${encodedString}`);
1322

1423
// Uncomment when testing for easier repro
1524
console.log(url.toString());

‎integrations/vite/tests/visibility.spec.tsx‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { expect, test } from "@playwright/test";
2+
import { Group, Panel, Separator } from "react-resizable-panels";
3+
import { goToUrl } from "./utils/goToUrl";
4+
5+
test.describe("visibility", () => {
6+
test("Activity API should defer default layout calculation until visible", async ({
7+
page
8+
}) => {
9+
await goToUrl(
10+
page,
11+
<Group>
12+
<Panel defaultSize="25%" id="left" minSize={150} />
13+
<Separator />
14+
<Panel id="right" />
15+
</Group>,
16+
"/e2e/visibility/activity/hidden/"
17+
);
18+
19+
await expect(page.getByText('"onLayoutCount": 0')).toBeVisible();
20+
21+
await page.getByText("toggle activity hidden → visible").click();
22+
23+
await expect(page.getByText('"onLayoutCount": 1')).toBeVisible();
24+
await expect(page.getByText("id: left")).toContainText("25%");
25+
await expect(page.getByRole("separator")).toBeVisible();
26+
await expect(page.getByText("id: right")).toContainText("75%");
27+
28+
await page.getByText("toggle activity visible → hidden").click();
29+
30+
await expect(page.getByText('"onLayoutCount": 1')).toBeVisible();
31+
32+
await page.setViewportSize({
33+
width: 500,
34+
height: 500
35+
});
36+
await page.getByText("toggle activity hidden → visible").click();
37+
38+
await expect(page.getByText('"onLayoutCount": 2')).toBeVisible();
39+
await expect(page.getByText("id: left")).toContainText("33%");
40+
await expect(page.getByRole("separator")).toBeVisible();
41+
await expect(page.getByText("id: right")).toContainText("67%");
42+
});
43+
44+
test("display:hidden should defer default layout calculation until visible", async ({
45+
page
46+
}) => {
47+
await goToUrl(
48+
page,
49+
<Group>
50+
<Panel defaultSize="25%" id="left" minSize={150} />
51+
<Separator />
52+
<Panel id="right" />
53+
</Group>,
54+
"/e2e/visibility/display/hidden/"
55+
);
56+
57+
await expect(page.getByText('"onLayoutCount": 1')).toBeVisible();
58+
// Actual layout values are not meaningful since the Group is not visible
59+
60+
await page.getByText("toggle display hidden → visible").click();
61+
62+
await expect(page.getByText('"onLayoutCount": 2')).toBeVisible();
63+
await expect(page.getByText("id: left")).toContainText("25%");
64+
await expect(page.getByRole("separator")).toBeVisible();
65+
await expect(page.getByText("id: right")).toContainText("75%");
66+
67+
await page.getByText("toggle display visible → hidden").click();
68+
69+
await expect(page.getByText('"onLayoutCount": 2')).toBeVisible();
70+
// Actual layout values are not meaningful since the Group is not visible
71+
72+
await page.setViewportSize({
73+
width: 500,
74+
height: 500
75+
});
76+
await page.getByText("toggle display hidden → visible").click();
77+
78+
await expect(page.getByText('"onLayoutCount": 3')).toBeVisible();
79+
await expect(page.getByText("id: left")).toContainText("33%");
80+
await expect(page.getByRole("separator")).toBeVisible();
81+
await expect(page.getByText("id: right")).toContainText("67%");
82+
});
83+
});

‎lib/components/group/Group.tsx‎

Lines changed: 57 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -100,67 +100,68 @@ export function Group({
100100
// Register Group and child Panels/Separators with global state
101101
// Listen to global state for drag state related to this Group
102102
useIsomorphicLayoutEffect(() => {
103-
if (element !== null && panels.length > 0) {
104-
const group: RegisteredGroup = {
105-
defaultLayout,
106-
disableCursor: !!disableCursor,
107-
disabled: !!disabled,
108-
element,
109-
id,
110-
inMemoryLastExpandedPanelSizes:
111-
inMemoryLastExpandedPanelSizesRef.current,
112-
inMemoryLayouts: inMemoryLayoutsRef.current,
113-
orientation,
114-
panels,
115-
separators
116-
};
117-
118-
const unmountGroup = mountGroup(group);
119-
120-
const globalState = read();
121-
const match = globalState.mountedGroups.get(group);
122-
if (match) {
123-
setLayout(match.layout);
124-
onLayoutChangeStable?.(match.layout);
125-
}
103+
if (element === null || panels.length === 0) {
104+
return;
105+
}
106+
107+
const group: RegisteredGroup = {
108+
defaultLayout,
109+
disableCursor: !!disableCursor,
110+
disabled: !!disabled,
111+
element,
112+
id,
113+
inMemoryLastExpandedPanelSizes: inMemoryLastExpandedPanelSizesRef.current,
114+
inMemoryLayouts: inMemoryLayoutsRef.current,
115+
orientation,
116+
panels,
117+
separators
118+
};
119+
120+
const unmountGroup = mountGroup(group);
126121

127-
const removeInteractionStateChangeListener = eventEmitter.addListener(
128-
"interactionStateChange",
129-
(interactionState) => {
130-
switch (interactionState.state) {
131-
case "active": {
132-
setDragActive(
133-
interactionState.hitRegions.some(
134-
(current) => current.group === group
135-
)
136-
);
137-
break;
138-
}
139-
default: {
140-
setDragActive(false);
141-
break;
142-
}
122+
const globalState = read();
123+
const match = globalState.mountedGroups.get(group);
124+
if (match) {
125+
setLayout(match.layout);
126+
onLayoutChangeStable?.(match.layout);
127+
}
128+
129+
const removeInteractionStateChangeListener = eventEmitter.addListener(
130+
"interactionStateChange",
131+
(interactionState) => {
132+
switch (interactionState.state) {
133+
case "active": {
134+
setDragActive(
135+
interactionState.hitRegions.some(
136+
(current) => current.group === group
137+
)
138+
);
139+
break;
143140
}
144-
}
145-
);
146-
147-
const removeMountedGroupsChangeEventListener = eventEmitter.addListener(
148-
"mountedGroupsChange",
149-
(mountedGroups) => {
150-
const match = mountedGroups.get(group);
151-
if (match && match.derivedPanelConstraints.length > 0) {
152-
setLayout(match.layout);
153-
onLayoutChangeStable?.(match.layout);
141+
default: {
142+
setDragActive(false);
143+
break;
154144
}
155145
}
156-
);
146+
}
147+
);
148+
149+
const removeMountedGroupsChangeEventListener = eventEmitter.addListener(
150+
"mountedGroupsChange",
151+
(mountedGroups) => {
152+
const match = mountedGroups.get(group);
153+
if (match && match.derivedPanelConstraints.length > 0) {
154+
setLayout(match.layout);
155+
onLayoutChangeStable?.(match.layout);
156+
}
157+
}
158+
);
157159

158-
return () => {
159-
unmountGroup();
160-
removeInteractionStateChangeListener();
161-
removeMountedGroupsChangeEventListener();
162-
};
163-
}
160+
return () => {
161+
unmountGroup();
162+
removeInteractionStateChangeListener();
163+
removeMountedGroupsChangeEventListener();
164+
};
164165
}, [
165166
defaultLayout,
166167
disableCursor,

‎lib/components/group/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@ export type RegisteredGroup = {
2828
disabled: boolean;
2929
element: HTMLElement;
3030
id: string;
31+
// TODO Move to mutable state
3132
inMemoryLastExpandedPanelSizes: {
3233
[panelId: string]: number;
3334
};
35+
// TODO Move to mutable state
3436
inMemoryLayouts: {
3537
[panelIds: string]: Layout;
3638
};

‎lib/global/dom/calculatePanelConstraints.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,18 @@ export function calculatePanelConstraints(group: RegisteredGroup) {
88
const { panels } = group;
99

1010
const groupSize = calculateAvailableGroupSize({ group });
11+
if (groupSize === 0) {
12+
// Can't calculate anything meaningful if the group has a width/height of 0
13+
// (This could indicate that it's within a hidden subtree)
14+
return panels.map((current) => ({
15+
collapsedSize: 0,
16+
collapsible: current.panelConstraints.collapsible === true,
17+
defaultSize: undefined,
18+
minSize: 0,
19+
maxSize: 100,
20+
panelId: current.id
21+
}));
22+
}
1123

1224
return panels.map<PanelConstraints>((panel) => {
1325
const { element, panelConstraints } = panel;

0 commit comments

Comments
 (0)