Skip to content

Commit 6cb6ff8

Browse files
authored
style: adds left handle & style fixes for resizable component (#4745)
## 📝 Summary <!-- Provide a concise summary of what this pull request is addressing. If this PR fixes any issues, list them here by number (e.g., Fixes #123). --> - Allows a left handle (which will be used in the future for sidebar) - Makes the column drag handle slightly thinner - Prevents element selection underneath (importantt) ## 🔍 Description of Changes <!-- Detail the specific changes made in this pull request. Explain the problem addressed and how it was resolved. If applicable, provide before and after comparisons, screenshots, or any relevant details to help reviewers understand the changes easily. --> ## 📋 Checklist - [X] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [ ] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [X] I have added tests for the changes made. - [X] I have run the code and verified that it works as expected. ## 📜 Reviewers <!-- Tag potential reviewers from the community or maintainers who might be interested in reviewing this pull request. Your PR will be reviewed more quickly if you can figure out the right person to tag with @ --> @mscolnick
1 parent 66d9c46 commit 6cb6ff8

File tree

5 files changed

+113
-44
lines changed

5 files changed

+113
-44
lines changed

frontend/src/components/editor/columns/__tests__/storage.test.ts

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,20 @@
22
import { describe, it, expect, afterEach } from "vitest";
33
import { reorderColumnSizes, storageFn } from "../storage";
44

5-
describe("setColumnWidth", () => {
6-
const { clearStorage, getColumnWidth, setColumnWidth } = storageFn;
5+
describe("Column width storage", () => {
6+
const { clearStorage, getColumnWidth, saveColumnWidth } = storageFn;
77

88
afterEach(() => {
99
clearStorage();
1010
});
1111

1212
it("should set width for an existing index", () => {
1313
// Setup initial state
14-
setColumnWidth(0, 100);
15-
setColumnWidth(1, 200);
14+
saveColumnWidth(0, 100);
15+
saveColumnWidth(1, 200);
1616

1717
// Test
18-
setColumnWidth(0, 150);
18+
saveColumnWidth(0, 150);
1919

2020
// Verify
2121
expect(getColumnWidth(0)).toBe(150);
@@ -24,10 +24,10 @@ describe("setColumnWidth", () => {
2424

2525
it("should set width for a new index", () => {
2626
// Setup initial state
27-
setColumnWidth(0, 100);
27+
saveColumnWidth(0, 100);
2828

2929
// Test
30-
setColumnWidth(1, 200);
30+
saveColumnWidth(1, 200);
3131

3232
// Verify
3333
expect(getColumnWidth(0)).toBe(100);
@@ -36,10 +36,10 @@ describe("setColumnWidth", () => {
3636

3737
it("should pad with contentWidth when setting width for out of bounds index", () => {
3838
// Setup initial state
39-
setColumnWidth(0, 100);
39+
saveColumnWidth(0, 100);
4040

4141
// Test
42-
setColumnWidth(3, 300);
42+
saveColumnWidth(3, 300);
4343

4444
// Verify
4545
expect(getColumnWidth(0)).toBe(100);
@@ -50,7 +50,7 @@ describe("setColumnWidth", () => {
5050

5151
it("should handle empty initial state", () => {
5252
// Test
53-
setColumnWidth(2, 200);
53+
saveColumnWidth(2, 200);
5454

5555
// Verify
5656
expect(getColumnWidth(0)).toBe("contentWidth");
@@ -60,13 +60,13 @@ describe("setColumnWidth", () => {
6060

6161
it("should update multiple columns", () => {
6262
// Setup initial state
63-
setColumnWidth(0, 100);
64-
setColumnWidth(1, 200);
65-
setColumnWidth(2, 300);
63+
saveColumnWidth(0, 100);
64+
saveColumnWidth(1, 200);
65+
saveColumnWidth(2, 300);
6666

6767
// Test
68-
setColumnWidth(0, 150);
69-
setColumnWidth(2, 350);
68+
saveColumnWidth(0, 150);
69+
saveColumnWidth(2, 350);
7070

7171
// Verify
7272
expect(getColumnWidth(0)).toBe(150);
@@ -76,11 +76,11 @@ describe("setColumnWidth", () => {
7676

7777
it("should set contentWidth directly", () => {
7878
// Setup initial state
79-
setColumnWidth(0, 100);
80-
setColumnWidth(1, 200);
79+
saveColumnWidth(0, 100);
80+
saveColumnWidth(1, 200);
8181

8282
// Test
83-
setColumnWidth(0, "contentWidth");
83+
saveColumnWidth(0, "contentWidth");
8484

8585
// Verify
8686
expect(getColumnWidth(0)).toBe("contentWidth");
@@ -89,9 +89,9 @@ describe("setColumnWidth", () => {
8989

9090
it("should maintain correct widths after reordering", () => {
9191
// Setup initial state with 3 columns
92-
setColumnWidth(0, 100);
93-
setColumnWidth(1, 200);
94-
setColumnWidth(2, 300);
92+
saveColumnWidth(0, 100);
93+
saveColumnWidth(1, 200);
94+
saveColumnWidth(2, 300);
9595

9696
// Reorder column 0 to position 2
9797
reorderColumnSizes(0, 2);

frontend/src/components/editor/columns/cell-column.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ interface Props {
1818
canMoveRight: boolean;
1919
}
2020

21-
const { getColumnWidth, setColumnWidth } = storageFn;
21+
const { getColumnWidth, saveColumnWidth } = storageFn;
2222

2323
export const Column = memo((props: Props) => {
2424
const columnRef = useRef<HTMLDivElement>(null);
@@ -28,7 +28,7 @@ export const Column = memo((props: Props) => {
2828
<ResizableComponent
2929
startingWidth={getColumnWidth(props.index)}
3030
onResize={(width: number) => {
31-
setColumnWidth(props.index, width);
31+
saveColumnWidth(props.index, width);
3232
}}
3333
>
3434
{props.children}
@@ -78,6 +78,7 @@ const ResizableComponent = ({
7878
const { resizableDivRef, handleRef, style } = useResizeHandle({
7979
startingWidth,
8080
onResize,
81+
direction: "right",
8182
});
8283

8384
return (
@@ -91,7 +92,7 @@ const ResizableComponent = ({
9192
</div>
9293
<div
9394
ref={handleRef}
94-
className="w-1 cursor-col-resize transition-colors duration-200 z-10
95+
className="w-[3px] cursor-col-resize transition-colors duration-200 z-10
9596
group-hover/column:bg-[var(--slate-3)] dark:group-hover/column:bg-[var(--slate-5)]
9697
group-hover/column:hover:bg-primary/60 dark:group-hover/column:hover:bg-primary/60"
9798
/>

frontend/src/components/editor/columns/storage.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export const storageFn = {
2727
const widths = storage.get().widths;
2828
return widths[index] ?? "contentWidth";
2929
},
30-
setColumnWidth: (index: number, width: number | "contentWidth") => {
30+
saveColumnWidth: (index: number, width: number | "contentWidth") => {
3131
const widths = storage.get().widths;
3232
if (widths[index]) {
3333
widths[index] = width;
Lines changed: 81 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
/* Copyright 2024 Marimo. All rights reserved. */
2-
import { renderHook, act } from "@testing-library/react";
2+
import { renderHook, act, render } from "@testing-library/react";
33
import { useResizeHandle } from "../useResizeHandle";
44
import { describe, it, expect, vi } from "vitest";
55

66
describe("useResizeHandle", () => {
77
it("should initialize with correct refs and style", () => {
88
const { result } = renderHook(() =>
9-
useResizeHandle({ startingWidth: 500, onResize: vi.fn() }),
9+
useResizeHandle({
10+
startingWidth: 500,
11+
onResize: vi.fn(),
12+
direction: "right",
13+
}),
1014
);
1115

1216
expect(result.current.resizableDivRef.current).toBeNull();
@@ -16,32 +20,47 @@ describe("useResizeHandle", () => {
1620

1721
it("should handle contentWidth starting width", () => {
1822
const { result } = renderHook(() =>
19-
useResizeHandle({ startingWidth: "contentWidth", onResize: vi.fn() }),
23+
useResizeHandle({
24+
startingWidth: "contentWidth",
25+
onResize: vi.fn(),
26+
direction: "right",
27+
}),
2028
);
2129

2230
expect(result.current.style).toEqual({ width: "contentWidth" });
2331
});
2432

25-
it.skip("should call onResize when resizing ends", () => {
33+
it("should call onResize when resizing ends", () => {
2634
const onResize = vi.fn();
27-
const { result } = renderHook(() =>
28-
useResizeHandle({ startingWidth: 500, onResize }),
29-
);
3035

31-
// Mock DOM elements
32-
const mockDiv = document.createElement("div");
33-
mockDiv.style.width = "500px";
34-
// @ts-expect-error - we're testing the ref
35-
result.current.resizableDivRef.current = mockDiv;
36+
// Create a test component that uses the hook
37+
const TestComponent = () => {
38+
const { resizableDivRef, handleRef } = useResizeHandle({
39+
startingWidth: 500,
40+
onResize,
41+
direction: "right",
42+
});
43+
44+
return (
45+
<div>
46+
<div
47+
ref={resizableDivRef}
48+
style={{ width: "500px" }}
49+
data-testid="resizable-div"
50+
/>
51+
<div ref={handleRef} data-testid="handle" />
52+
</div>
53+
);
54+
};
3655

37-
const mockHandle = document.createElement("div");
38-
// @ts-expect-error - we're testing the ref
39-
result.current.handleRef.current = mockHandle;
56+
const { getByTestId } = render(<TestComponent />);
57+
const resizableDiv = getByTestId("resizable-div") as HTMLDivElement;
58+
const handle = getByTestId("handle") as HTMLDivElement;
4059

4160
// Simulate resize
4261
act(() => {
4362
const mousedownEvent = new MouseEvent("mousedown", { clientX: 0 });
44-
mockHandle.dispatchEvent(mousedownEvent);
63+
handle.dispatchEvent(mousedownEvent);
4564

4665
const mousemoveEvent = new MouseEvent("mousemove", { clientX: 100 });
4766
document.dispatchEvent(mousemoveEvent);
@@ -50,6 +69,51 @@ describe("useResizeHandle", () => {
5069
document.dispatchEvent(mouseupEvent);
5170
});
5271

53-
expect(onResize).toHaveBeenCalledWith(600); // 500px + 100px movement
72+
expect(resizableDiv.style.width).toBe("600px"); // 500px + 100px movement
73+
expect(onResize).toHaveBeenCalledWith(600);
74+
});
75+
76+
it("should handle left direction resizing", () => {
77+
const onResize = vi.fn();
78+
79+
// Create a test component that uses the hook
80+
const TestComponent = () => {
81+
const { resizableDivRef, handleRef } = useResizeHandle({
82+
startingWidth: 500,
83+
onResize,
84+
direction: "left",
85+
});
86+
87+
return (
88+
<div>
89+
<div
90+
ref={resizableDivRef}
91+
style={{ width: "500px" }}
92+
data-testid="resizable-div"
93+
/>
94+
<div ref={handleRef} data-testid="handle" />
95+
</div>
96+
);
97+
};
98+
99+
const { getByTestId } = render(<TestComponent />);
100+
101+
const resizableDiv = getByTestId("resizable-div") as HTMLDivElement;
102+
const handle = getByTestId("handle") as HTMLDivElement;
103+
104+
// Simulate resize
105+
act(() => {
106+
const mousedownEvent = new MouseEvent("mousedown", { clientX: 0 });
107+
handle.dispatchEvent(mousedownEvent);
108+
109+
const mousemoveEvent = new MouseEvent("mousemove", { clientX: -100 });
110+
document.dispatchEvent(mousemoveEvent);
111+
112+
const mouseupEvent = new MouseEvent("mouseup");
113+
document.dispatchEvent(mouseupEvent);
114+
});
115+
116+
expect(resizableDiv.style.width).toBe("600px"); // 500px - (-100px) movement
117+
expect(onResize).toHaveBeenCalledWith(600);
54118
});
55119
});

frontend/src/hooks/useResizeHandle.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@ import { useEffect, useRef } from "react";
44
interface UseResizeHandleProps {
55
onResize?: (width: number) => void;
66
startingWidth: number | "contentWidth";
7+
direction: "left" | "right";
78
}
89

910
export const useResizeHandle = ({
1011
onResize,
1112
startingWidth,
13+
direction,
1214
}: UseResizeHandleProps) => {
1315
const resizableDivRef = useRef<HTMLDivElement>(null);
1416
const handleRef = useRef<HTMLDivElement>(null);
@@ -31,7 +33,8 @@ export const useResizeHandle = ({
3133
}
3234
const dx = e.clientX - lastX;
3335
lastX = e.clientX;
34-
width += dx;
36+
// dx is negative when moving left
37+
width = direction === "left" ? width - dx : width + dx;
3538
resizableDiv.style.width = `${width}px`;
3639
};
3740

@@ -45,6 +48,7 @@ export const useResizeHandle = ({
4548
};
4649

4750
const onMouseDown = (e: MouseEvent) => {
51+
e.preventDefault(); // Prevent selection of elements underneath
4852
isResizing = true;
4953
lastX = e.clientX;
5054
document.addEventListener("mousemove", onMouseMove);
@@ -58,7 +62,7 @@ export const useResizeHandle = ({
5862
document.removeEventListener("mousemove", onMouseMove);
5963
document.removeEventListener("mouseup", onMouseUp);
6064
};
61-
}, [onResize]);
65+
}, [direction, onResize]);
6266

6367
return {
6468
resizableDivRef,

0 commit comments

Comments
 (0)