Skip to content

Commit 4625da2

Browse files
committed
fix(panel): ignore handle clicks and flag pane value mistakes
A press on the resize handle that never moved still committed the rendered width on release. When the container had clamped a stored width, that click silently overwrote the user's choice. When the stored width equalled the default, the click reset it. The pointer now has to travel 4px before the press counts as a drag, and only a drag commits. In development, PanelContent now logs a console error when two panes share a value or when its value names no pane. Each pane registers its value from an effect, so this check also catches a missing value without keepMounted, where panes that are not shown never reach the DOM. Adds tests for both, and for the right-to-left mirroring of keys and drags, which had none.
1 parent dfb15c4 commit 4625da2

4 files changed

Lines changed: 167 additions & 6 deletions

File tree

‎src/hooks/use-resizable.test.ts‎

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ type Mounted = {
6161
readonly handle: HTMLElement;
6262
readonly changes: (number | null)[];
6363
press(key: string, init?: KeyboardEventInit): void;
64+
pointer(type: string, clientX: number): void;
6465
rerender(options?: Partial<UseResizableOptions>): void;
6566
unmount(): void;
6667
};
@@ -71,7 +72,10 @@ function find(container: HTMLElement, selector: string): HTMLElement {
7172
return element;
7273
}
7374

74-
function mount(options: Partial<UseResizableOptions> = {}): Mounted {
75+
function mount(
76+
options: Partial<UseResizableOptions> = {},
77+
dir?: "rtl",
78+
): Mounted {
7579
const changes: (number | null)[] = [];
7680
const container = document.createElement("div");
7781
document.body.appendChild(container);
@@ -89,7 +93,11 @@ function mount(options: Partial<UseResizableOptions> = {}): Mounted {
8993
{ "data-row": "" },
9094
createElement(
9195
"aside",
92-
{ ref: resizable.panelRef },
96+
{
97+
ref: resizable.panelRef,
98+
// happy-dom does not derive `direction` from a `dir` attribute.
99+
style: dir === undefined ? undefined : { direction: dir },
100+
},
93101
createElement("div", {
94102
"data-handle": "",
95103
"aria-label": "Resize",
@@ -106,6 +114,10 @@ function mount(options: Partial<UseResizableOptions> = {}): Mounted {
106114
});
107115
render(options);
108116
const handle = find(container, "[data-handle]");
117+
// happy-dom has no pointer capture; the hook only needs it not to throw.
118+
handle.setPointerCapture = () => {};
119+
handle.releasePointerCapture = () => {};
120+
handle.hasPointerCapture = () => false;
109121
return {
110122
panel: find(container, "aside"),
111123
handle,
@@ -117,6 +129,19 @@ function mount(options: Partial<UseResizableOptions> = {}): Mounted {
117129
);
118130
});
119131
},
132+
pointer(type, clientX) {
133+
act(() => {
134+
handle.dispatchEvent(
135+
new PointerEvent(type, {
136+
bubbles: true,
137+
button: 0,
138+
isPrimary: true,
139+
pointerId: 1,
140+
clientX,
141+
}),
142+
);
143+
});
144+
},
120145
rerender(next = options) {
121146
render(next);
122147
},
@@ -243,4 +268,48 @@ describe("useResizable", () => {
243268
expect(mounted.changes).toEqual([]);
244269
mounted.unmount();
245270
});
271+
272+
test("a drag commits once on release, from where the pointer stopped", () => {
273+
const mounted = mount();
274+
mounted.pointer("pointerdown", 500);
275+
mounted.pointer("pointermove", 450);
276+
mounted.pointer("pointermove", 400);
277+
expect(mounted.changes).toEqual([]);
278+
mounted.pointer("pointerup", 400);
279+
expect(mounted.changes).toEqual([744]);
280+
mounted.unmount();
281+
});
282+
283+
test("a press that never moves past the drag threshold commits nothing", () => {
284+
// The stored 900 renders clamped to 560: a commit would overwrite it.
285+
rowWidth = 1000;
286+
const clamped = mount({ defaultWidth: 900 });
287+
expect(shownWidth(clamped)).toBe(560);
288+
clamped.pointer("pointerdown", 500);
289+
clamped.pointer("pointermove", 498);
290+
clamped.pointer("pointerup", 498);
291+
expect(clamped.changes).toEqual([]);
292+
clamped.unmount();
293+
294+
// A stored width equal to the default: a commit would reset it.
295+
rowWidth = 1400;
296+
const atDefault = mount({ defaultWidth: 644 });
297+
atDefault.pointer("pointerdown", 500);
298+
atDefault.pointer("pointerup", 500);
299+
expect(atDefault.changes).toEqual([]);
300+
atDefault.unmount();
301+
});
302+
303+
test("mirrors keys and drags in right-to-left text", () => {
304+
const mounted = mount({}, "rtl");
305+
mounted.press("ArrowRight");
306+
expect(mounted.changes).toEqual([660]);
307+
mounted.press("ArrowLeft");
308+
expect(mounted.changes).toEqual([660, null]);
309+
mounted.pointer("pointerdown", 500);
310+
mounted.pointer("pointermove", 600);
311+
mounted.pointer("pointerup", 600);
312+
expect(mounted.changes).toEqual([660, null, 744]);
313+
mounted.unmount();
314+
});
246315
});

‎src/hooks/use-resizable.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ const KEY_STEP_PX = 16;
1717
const LARGE_STEP_FACTOR = 4;
1818
/** A drag ending this close to the default width lands on the default. */
1919
const SNAP_PX = 12;
20+
/** How far the pointer travels before a press on the handle becomes a drag. */
21+
const DRAG_THRESHOLD_PX = 4;
2022

2123
export type UseResizableOptions = {
2224
/**
@@ -114,8 +116,9 @@ function growDirection(element: HTMLElement): 1 | -1 {
114116
* A pointer drag writes the variable straight to the element each frame and
115117
* updates the handle's ARIA value by hand, so nothing re-renders until
116118
* release, when the width is committed once (snapping to the default when
117-
* close). Like a controlled input, a parent that ignores `onWidthChange`
118-
* keeps its width: the drag's override is dropped before the commit.
119+
* close). A press that never moves is a click and commits nothing. Like a
120+
* controlled input, a parent that ignores `onWidthChange` keeps its width:
121+
* the drag's override is dropped before the commit.
119122
*
120123
* Keys on the handle: Left/Right arrows step (mirrored in right-to-left
121124
* text), Shift for bigger steps, Home and End jump to the limits, Enter
@@ -239,13 +242,19 @@ export function useResizable(options: UseResizableOptions): UseResizableResult {
239242
const previousUserSelect = root.style.userSelect;
240243
let latest = startWidth;
241244
let frame = 0;
245+
// A press that never travels is a click (focus, or half a double-click),
246+
// not a resize: it commits nothing.
247+
let moved = false;
242248

243249
draggingRef.current = true;
244250
handle.dataset["dragging"] = "true";
245251
root.style.cursor = "col-resize";
246252
root.style.userSelect = "none";
247253

248254
const move = (moveEvent: PointerEvent) => {
255+
if (!moved && Math.abs(moveEvent.clientX - startX) < DRAG_THRESHOLD_PX)
256+
return;
257+
moved = true;
249258
let next = clamp(
250259
startWidth + direction * (startX - moveEvent.clientX),
251260
min,
@@ -280,7 +289,7 @@ export function useResizable(options: UseResizableOptions): UseResizableResult {
280289

281290
function end() {
282291
stop();
283-
commit(latest);
292+
if (moved) commit(latest);
284293
}
285294

286295
handle.addEventListener("pointermove", move);

‎src/ui/panel.test.tsx‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,4 +217,48 @@ describe("Panel", () => {
217217
console.error = original;
218218
}
219219
});
220+
221+
test("in development, pane values that cannot show one pane log an error", () => {
222+
const errors: unknown[][] = [];
223+
const original = console.error;
224+
console.error = (...args: unknown[]) => errors.push(args);
225+
try {
226+
const duplicate = mount(
227+
<PanelContent value="a">
228+
<PanelPane value="a">first</PanelPane>
229+
<PanelPane value="a">second</PanelPane>
230+
</PanelContent>,
231+
);
232+
duplicate.unmount();
233+
expect(errors).toEqual([
234+
["PanelContent: two PanelPanes share the value `a`."],
235+
]);
236+
237+
errors.length = 0;
238+
const missing = mount(
239+
<PanelContent value="missing">
240+
<PanelPane value="a">first</PanelPane>
241+
</PanelContent>,
242+
);
243+
missing.unmount();
244+
expect(errors).toEqual([
245+
[
246+
"PanelContent: `value` names none of its panes, so the panel body is empty.",
247+
],
248+
]);
249+
250+
errors.length = 0;
251+
const fine = mount(
252+
<PanelContent value="b" keepMounted>
253+
<PanelPane value="a">first</PanelPane>
254+
<PanelPane value="b">second</PanelPane>
255+
</PanelContent>,
256+
);
257+
fine.render(<PanelContent value="b" />);
258+
fine.unmount();
259+
expect(errors).toEqual([]);
260+
} finally {
261+
console.error = original;
262+
}
263+
});
220264
});

‎src/ui/panel.tsx‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
createContext,
66
useCallback,
77
useContext,
8+
useEffect,
89
useId,
910
useLayoutEffect,
1011
useRef,
@@ -19,6 +20,9 @@ import { inertSiblings } from "../lib/inert-siblings.js";
1920
import { cn } from "../lib/utils.js";
2021
import { Button } from "./button.js";
2122

23+
const isProduction =
24+
typeof process !== "undefined" && process.env?.NODE_ENV === "production";
25+
2226
const DEFAULT_MIN_WIDTH_PX = 320;
2327
const DEFAULT_MAIN_MIN_WIDTH_PX = 440;
2428

@@ -38,6 +42,8 @@ function usePanelContext(part: string): PanelContextValue {
3842
type PanelPanesContextValue = {
3943
readonly value: string;
4044
readonly keepMounted: boolean;
45+
/** Development only: a pane reports its value; returns its removal. */
46+
readonly register: (value: string) => () => void;
4147
};
4248

4349
const PanelPanesContext = createContext<PanelPanesContextValue | null>(null);
@@ -305,8 +311,36 @@ export function PanelContent({
305311
className,
306312
...props
307313
}: PanelContentProps) {
314+
// Every mounted pane's value, kept in development to catch host mistakes:
315+
// panes report from effects, which run before this component's own.
316+
const registeredRef = useRef<string[]>([]);
317+
const register = useCallback((paneValue: string) => {
318+
const registered = registeredRef.current;
319+
registered.push(paneValue);
320+
return () => {
321+
registered.splice(registered.indexOf(paneValue), 1);
322+
};
323+
}, []);
324+
325+
useEffect(() => {
326+
if (isProduction) return;
327+
const registered = registeredRef.current;
328+
if (registered.length === 0) return;
329+
const duplicate = registered.find(
330+
(paneValue, index) => registered.indexOf(paneValue) !== index,
331+
);
332+
if (duplicate !== undefined)
333+
console.error(
334+
`PanelContent: two PanelPanes share the value \`${duplicate}\`.`,
335+
);
336+
if (!registered.includes(value))
337+
console.error(
338+
"PanelContent: `value` names none of its panes, so the panel body is empty.",
339+
);
340+
});
341+
308342
return (
309-
<PanelPanesContext.Provider value={{ value, keepMounted }}>
343+
<PanelPanesContext.Provider value={{ value, keepMounted, register }}>
310344
<div
311345
data-slot="panel-content"
312346
className={cn("grid min-h-0 flex-1 grid-cols-1 grid-rows-1", className)}
@@ -330,6 +364,11 @@ export function PanelPane({ value, className, ...props }: PanelPaneProps) {
330364
const context = useContext(PanelPanesContext);
331365
if (context === null)
332366
throw new Error("PanelPane must be inside PanelContent.");
367+
const { register } = context;
368+
useEffect(
369+
() => (isProduction ? undefined : register(value)),
370+
[register, value],
371+
);
333372
const shown = context.value === value;
334373
if (!shown && !context.keepMounted) return null;
335374
return (

0 commit comments

Comments
 (0)