Skip to content

Commit 7ce486e

Browse files
author
Johannes Weber
committed
chore: Add pointer move tolerance that counts as a press rather than move
1 parent 65634e8 commit 7ce486e

File tree

6 files changed

+109
-8
lines changed

6 files changed

+109
-8
lines changed

src/internal/drag-handle/index.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
InternalDragHandleProps,
99
} from "@cloudscape-design/components/internal/do-not-use/drag-handle";
1010

11-
import { HandleActiveState } from "../item-container";
11+
import { CLICK_DRAG_THRESHOLD, HandleActiveState } from "../item-container";
1212

1313
import styles from "./styles.css.js";
1414
import testUtilsStyles from "./test-classes/styles.css.js";
@@ -60,6 +60,8 @@ function DragHandle(
6060
triggerMode="keyboard-activate"
6161
onDirectionClick={onDirectionClick}
6262
initialShowButtons={initialShowButtons}
63+
hideButtonsOnDrag={true}
64+
clickDragThreshold={CLICK_DRAG_THRESHOLD}
6365
/>
6466
);
6567
}

src/internal/item-container/__tests__/item-container.test.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,20 @@ describe("pointer interaction", () => {
8686
clickHandle(getByTestId("resize-handle"));
8787
expect(mockDraggable.start).toBeCalledWith("resize", "pointer", expect.any(Coordinates));
8888
});
89+
90+
test("does not call updateTransition on pointer down and a mouse movement within the CLICK_DRAG_THRESHOLD", () => {
91+
const { getByTestId } = render(<ItemContainer {...defaultProps} />);
92+
clickHandle(getByTestId("drag-handle"));
93+
expect(mockDraggable.updateTransition).not.toBeCalledWith();
94+
});
95+
96+
test("call updateTransition on pointer down and a mouse movement outside the CLICK_DRAG_THRESHOLD", () => {
97+
const { getByTestId } = render(<ItemContainer {...defaultProps} />);
98+
const dragHandleEl = getByTestId("drag-handle");
99+
fireEvent(dragHandleEl, new MouseEvent("pointerdown", { clientX: 10, clientY: 20, bubbles: true, button: 0 }));
100+
fireEvent(dragHandleEl, new MouseEvent("pointermove", { clientX: 15, clientY: 20, bubbles: true }));
101+
expect(mockDraggable.updateTransition).toBeCalledWith(expect.any(Coordinates));
102+
});
89103
});
90104

91105
describe("keyboard interaction", () => {

src/internal/item-container/__tests__/utils.test.ts

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,16 @@ import { beforeEach, describe, expect, Mock, test, vi } from "vitest";
44

55
import { getLogicalClientX as originalGetLogicalClientX } from "@cloudscape-design/component-toolkit/internal";
66

7+
import type { Operation } from "../../../../lib/components/internal/dnd-controller/controller";
78
import type { Transition } from "../../../../lib/components/internal/item-container";
89
import { determineHandleActiveState } from "../../../../lib/components/internal/item-container/utils";
9-
import type { Operation } from "../../dnd-controller/controller";
10-
import { Coordinates } from "../../utils/coordinates";
11-
import { calculateInitialPointerData, DetermineHandleActiveStateArgs, getDndOperationType } from "../utils";
10+
import {
11+
calculateInitialPointerData,
12+
DetermineHandleActiveStateArgs,
13+
getDndOperationType,
14+
hasPointerMovedBeyondThreshold,
15+
} from "../../../../lib/components/internal/item-container/utils";
16+
import { Coordinates } from "../../../../lib/components/internal/utils/coordinates";
1217

1318
const mockRect = {
1419
insetInlineStart: 10,
@@ -173,6 +178,51 @@ describe("calculateInitialPointerData", () => {
173178
});
174179
});
175180

181+
describe("hasPointerMovedBeyondThreshold", () => {
182+
const threshold = 3;
183+
184+
test("should return false when initialPosition is undefined", () => {
185+
const event = mockPointerEvent(50, 60) as PointerEvent;
186+
expect(hasPointerMovedBeyondThreshold(event, undefined)).toBe(false);
187+
});
188+
189+
test("should return false when pointer hasn't moved beyond threshold", () => {
190+
const initialPosition = { x: 50, y: 60 };
191+
const event = mockPointerEvent(51, 61) as PointerEvent;
192+
expect(hasPointerMovedBeyondThreshold(event, initialPosition, threshold)).toBe(false);
193+
});
194+
195+
test("should return true when pointer moved beyond threshold in positive X direction", () => {
196+
const initialPosition = { x: 50, y: 60 };
197+
const event = mockPointerEvent(54, 60) as PointerEvent;
198+
expect(hasPointerMovedBeyondThreshold(event, initialPosition, threshold)).toBe(true);
199+
});
200+
201+
test("should return true when pointer moved beyond threshold in negative X direction", () => {
202+
const initialPosition = { x: 50, y: 60 };
203+
const event = mockPointerEvent(46, 60) as PointerEvent;
204+
expect(hasPointerMovedBeyondThreshold(event, initialPosition, threshold)).toBe(true);
205+
});
206+
207+
test("should return true when pointer moved beyond threshold in positive Y direction", () => {
208+
const initialPosition = { x: 50, y: 60 };
209+
const event = mockPointerEvent(50, 64) as PointerEvent;
210+
expect(hasPointerMovedBeyondThreshold(event, initialPosition, threshold)).toBe(true);
211+
});
212+
213+
test("should return true when pointer moved beyond threshold in negative Y direction", () => {
214+
const initialPosition = { x: 50, y: 60 };
215+
const event = mockPointerEvent(50, 56) as PointerEvent;
216+
expect(hasPointerMovedBeyondThreshold(event, initialPosition, threshold)).toBe(true);
217+
});
218+
219+
test("should use default threshold when not provided", () => {
220+
const initialPosition = { x: 50, y: 60 };
221+
const event = mockPointerEvent(54, 60) as PointerEvent;
222+
expect(hasPointerMovedBeyondThreshold(event, initialPosition)).toBe(true);
223+
});
224+
});
225+
176226
describe("determineHandleActiveState", () => {
177227
const mockTransition = (operation: Operation): Transition => ({
178228
itemId: "test-item",

src/internal/item-container/index.tsx

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,12 @@ import { getNormalizedElementRect } from "../utils/screen";
3838
import { throttle } from "../utils/throttle";
3939
import { getCollisionRect } from "./get-collision-rect";
4040
import { getNextDroppable } from "./get-next-droppable";
41-
import { calculateInitialPointerData, determineHandleActiveState, getDndOperationType } from "./utils";
41+
import {
42+
calculateInitialPointerData,
43+
determineHandleActiveState,
44+
getDndOperationType,
45+
hasPointerMovedBeyondThreshold,
46+
} from "./utils";
4247

4348
import styles from "./styles.css.js";
4449

@@ -163,6 +168,12 @@ export interface ItemContainerProps {
163168

164169
export const ItemContainer = forwardRef(ItemContainerComponent);
165170

171+
// The amount of distance after pointer down that the cursor is allowed to
172+
// jitter for a subsequent mouseup to still register as a "press" instead of
173+
// a drag. A little allowance is needed for usability reasons, but this number
174+
// isn't set in stone.
175+
export const CLICK_DRAG_THRESHOLD = 3;
176+
166177
function ItemContainerComponent(
167178
{ item, placed, acquired, inTransition, transform, getItemSize, onKeyMove, children, isRtl }: ItemContainerProps,
168179
ref: Ref<ItemContainerRef>,
@@ -174,6 +185,7 @@ function ItemContainerComponent(
174185
const [isHidden, setIsHidden] = useState(false);
175186
const muteEventsRef = useRef(false);
176187
const itemRef = useRef<HTMLDivElement>(null);
188+
const initialPointerDownPosition = useRef<{ x: number; y: number } | undefined>();
177189
const draggableApi = useDraggable({
178190
draggableItem: item,
179191
getCollisionRect: (operation, coordinates, dropTarget) => {
@@ -322,17 +334,21 @@ function ItemContainerComponent(
322334
// 1. If the last interaction is not "keyboard" (the user clicked on another handle issuing a new transition);
323335
// 2. If the item is acquired by the board (in that case the focus moves to the board item which is expected, palette item is hidden and all events handlers must be muted).
324336
selectedHook.current.processBlur();
337+
initialPointerDownPosition.current = undefined;
325338
if (acquired || (transition && transition.interactionType === "keyboard" && !muteEventsRef.current)) {
326339
draggableApi.submitTransition();
327340
}
328341
}
329342

330343
function handleGlobalPointerMove(event: PointerEvent) {
331-
selectedHook.current.processPointerMove(event);
344+
if (hasPointerMovedBeyondThreshold(event, initialPointerDownPosition.current)) {
345+
selectedHook.current.processPointerMove(event);
346+
}
332347
}
333348

334349
function handleGlobalPointerUp(event: PointerEvent) {
335350
selectedHook.current.processPointerUp(event);
351+
initialPointerDownPosition.current = undefined;
336352
// Clean up global listeners after interaction ends
337353
window.removeEventListener("pointermove", handleGlobalPointerMove);
338354
window.removeEventListener("pointerup", handleGlobalPointerUp);
@@ -345,6 +361,7 @@ function ItemContainerComponent(
345361
if (event.button !== 0) {
346362
return;
347363
}
364+
initialPointerDownPosition.current = { x: event.clientX, y: event.clientY };
348365

349366
if (operation === "drag") {
350367
selectedHook.current = dragInteractionHook;

src/internal/item-container/utils.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { InteractionState } from "@cloudscape-design/components/internal/compone
55

66
import { Operation } from "../dnd-controller/controller"; // Adjust this path
77
import { Coordinates } from "../utils/coordinates"; // Adjust this path based on your project structure
8-
import { Transition } from ".";
8+
import { CLICK_DRAG_THRESHOLD, Transition } from ".";
99

1010
export type HandleActiveState = null | "pointer" | "uap";
1111

@@ -75,6 +75,22 @@ export function calculateInitialPointerData({
7575
return { pointerOffset, pointerBoundaries };
7676
}
7777

78+
export function hasPointerMovedBeyondThreshold(
79+
event: PointerEvent,
80+
initialPosition: { x: number; y: number } | undefined,
81+
threshold: number = CLICK_DRAG_THRESHOLD,
82+
): boolean {
83+
if (!initialPosition) {
84+
return false;
85+
}
86+
return (
87+
event.clientX > initialPosition.x + threshold ||
88+
event.clientX < initialPosition.x - threshold ||
89+
event.clientY > initialPosition.y + threshold ||
90+
event.clientY < initialPosition.y - threshold
91+
);
92+
}
93+
7894
export function determineHandleActiveState({
7995
isHandleActive,
8096
currentTransition,

src/internal/resize-handle/index.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
InternalDragHandleProps,
99
} from "@cloudscape-design/components/internal/do-not-use/drag-handle";
1010

11-
import { HandleActiveState } from "../item-container";
11+
import { CLICK_DRAG_THRESHOLD, HandleActiveState } from "../item-container";
1212

1313
import styles from "./styles.css.js";
1414
import testUtilsStyles from "./test-classes/styles.css.js";
@@ -53,6 +53,8 @@ export default function ResizeHandle({
5353
}}
5454
triggerMode="keyboard-activate"
5555
onDirectionClick={onDirectionClick}
56+
hideButtonsOnDrag={true}
57+
clickDragThreshold={CLICK_DRAG_THRESHOLD}
5658
/>
5759
);
5860
}

0 commit comments

Comments
 (0)