Skip to content

Commit

Permalink
Fix bad dev-conditionally around data attributes; better handle negat…
Browse files Browse the repository at this point in the history
…ive group sizes
  • Loading branch information
bvaughn committed Nov 13, 2023
1 parent cb48f3d commit b55808e
Show file tree
Hide file tree
Showing 9 changed files with 177 additions and 19 deletions.
2 changes: 1 addition & 1 deletion packages/react-resizable-panels/src/Panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,12 @@ export function PanelWithForwardedRef({

// CSS selectors
"data-panel": "",
"data-panel-id": panelId,

// e2e test attributes
"data-panel-collapsible": isDevelopment
? collapsible || undefined
: undefined,
"data-panel-id": isDevelopment ? panelId : undefined,
"data-panel-size": isDevelopment
? parseFloat("" + style.flexGrow).toFixed(1)
: undefined,
Expand Down
10 changes: 6 additions & 4 deletions packages/react-resizable-panels/src/PanelGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ function PanelGroupWithForwardedRef({
}

const groupSizePixels = calculateAvailablePanelSizeInPixels(groupId);
if (groupSizePixels <= 0) {
// Wait until the group has rendered a non-zero size before computing layout.
return;
}

if (unsafeLayout == null) {
unsafeLayout = calculateUnsafeDefaultLayout({
Expand Down Expand Up @@ -925,10 +929,8 @@ function PanelGroupWithForwardedRef({

// CSS selectors
"data-panel-group": "",

// e2e test attributes
"data-panel-group-direction": isDevelopment ? direction : undefined,
"data-panel-group-id": isDevelopment ? groupId : undefined,
"data-panel-group-direction": direction,
"data-panel-group-id": groupId,
})
);
}
Expand Down
25 changes: 12 additions & 13 deletions packages/react-resizable-panels/src/PanelResizeHandle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,19 +151,6 @@ export function PanelResizeHandle({
return createElement(Type, {
children,
className: classNameFromProps,

// CSS selectors
"data-resize-handle": "",

"data-resize-handle-active": isDragging
? "pointer"
: isFocused
? "keyboard"
: undefined,
"data-panel-group-direction": direction,
"data-panel-group-id": groupId,
"data-panel-resize-handle-enabled": !disabled,
"data-panel-resize-handle-id": resizeHandleId,
onBlur: () => setIsFocused(false),
onFocus: () => setIsFocused(true),
onMouseDown: (event: ReactMouseEvent) => {
Expand Down Expand Up @@ -192,6 +179,18 @@ export function PanelResizeHandle({
...styleFromProps,
},
tabIndex: 0,

// CSS selectors
"data-panel-group-direction": direction,
"data-panel-group-id": groupId,
"data-resize-handle": "",
"data-resize-handle-active": isDragging
? "pointer"
: isFocused
? "keyboard"
: undefined,
"data-panel-resize-handle-enabled": !disabled,
"data-panel-resize-handle-id": resizeHandleId,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,31 @@ describe("computePercentagePanelConstraints", () => {
}
`);
});

it("should compute reasonable percentage based constraints from pixels if group size is negative", () => {
jest.spyOn(console, "warn").mockImplementation(() => {});

expect(
computePercentagePanelConstraints(
[
{
minSizePixels: 25,
maxSizePixels: 100,
},
],

0,
-100
)
).toMatchInlineSnapshot(`
{
"collapsedSizePercentage": 0,
"defaultSizePercentage": undefined,
"maxSizePercentage": 0,
"minSizePercentage": 0,
}
`);

expect(console.warn).toHaveBeenCalledTimes(1);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { convertPixelConstraintsToPercentages } from "./convertPixelConstraintsToPercentages";

describe("convertPixelConstraintsToPercentages", () => {
it("should respect percentage panel constraints if group size is negative", () => {
jest.spyOn(console, "warn").mockImplementation(() => {});

expect(
convertPixelConstraintsToPercentages(
{
minSizePercentage: 25,
defaultSizePercentage: 50,
maxSizePercentage: 75,
},
-100
)
).toEqual({
collapsedSizePercentage: 0,
defaultSizePercentage: 50,
maxSizePercentage: 75,
minSizePercentage: 25,
});

expect(console.warn).toHaveBeenCalledTimes(0);
});

// Edge case test (issues/206)
it("should ignore pixel panel constraints if group size is negative", () => {
jest.spyOn(console, "warn").mockImplementation(() => {});

expect(
convertPixelConstraintsToPercentages(
{
minSizePixels: 25,
maxSizePixels: 75,
},
-100
)
).toEqual({
collapsedSizePercentage: 0,
defaultSizePercentage: undefined,
maxSizePercentage: 0,
minSizePercentage: 0,
});

expect(console.warn).toHaveBeenCalledTimes(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,23 @@ export function convertPixelConstraintsToPercentages(
minSizePixels,
} = panelConstraints;

const hasPixelConstraints =
collapsedSizePixels != null ||
defaultSizePixels != null ||
minSizePixels != null ||
maxSizePixels != null;

if (hasPixelConstraints && groupSizePixels <= 0) {
console.warn(`WARNING: Invalid group size: ${groupSizePixels}px`);

return {
collapsedSizePercentage: 0,
defaultSizePercentage,
maxSizePercentage: 0,
minSizePercentage: 0,
};
}

if (collapsedSizePixels != null) {
collapsedSizePercentage = convertPixelsToPercentage(
collapsedSizePixels,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
export function getPanelGroupElement(id: string): HTMLDivElement | null {
const element = document.querySelector(`[data-panel-group-id="${id}"]`);
const element = document.querySelector(
`[data-panel-group][data-panel-group-id="${id}"]`
);
if (element) {
return element as HTMLDivElement;
}
Expand Down
45 changes: 45 additions & 0 deletions packages/react-resizable-panels/src/utils/resizePanel.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { resizePanel } from "./resizePanel";

describe("resizePanel", () => {
// Edge case test (issues/206)
fit("should respect percentage panel constraints if group size is negative", () => {
jest.spyOn(console, "warn").mockImplementation(() => {});

expect(
resizePanel({
groupSizePixels: -100,
panelConstraints: [
{
minSizePercentage: 25,
maxSizePercentage: 75,
},
],
panelIndex: 0,
size: 50,
})
).toBe(50);

expect(console.warn).toHaveBeenCalledTimes(0);
});

// Edge case test (issues/206)
it("should handle pixel panel constraints if group size is negative", () => {
jest.spyOn(console, "warn").mockImplementation(() => {});

expect(
resizePanel({
groupSizePixels: -100,
panelConstraints: [
{
minSizePixels: 25,
maxSizePixels: 75,
},
],
panelIndex: 0,
size: 50,
})
).toBe(0);

expect(console.warn).toHaveBeenCalledTimes(1);
});
});
19 changes: 19 additions & 0 deletions packages/react-resizable-panels/src/utils/resizePanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,25 @@ export function resizePanel({
panelIndex: number;
size: number;
}) {
const hasPixelConstraints = panelConstraints.some(
({
collapsedSizePixels,
defaultSizePixels,
minSizePixels,
maxSizePixels,
}) =>
collapsedSizePixels != null ||
defaultSizePixels != null ||
minSizePixels != null ||
maxSizePixels != null
);

if (hasPixelConstraints && groupSizePixels <= 0) {
console.warn(`WARNING: Invalid group size: ${groupSizePixels}px`);

return 0;
}

let { collapsible } = panelConstraints[panelIndex]!;

const { collapsedSizePercentage, maxSizePercentage, minSizePercentage } =
Expand Down

0 comments on commit b55808e

Please sign in to comment.