Skip to content

fix: use clientX/clientY in drag-and-drop system #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jonathan-beebe merged 1 commit into from
Mar 29, 2026
Merged

fix: use clientX/clientY in drag-and-drop system #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions src/__tests__/hooks/useDragAndDrop.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,23 @@ describe('getQuadrantAtPoint', () => {
})
})

describe('coordinate system consistency', () => {
it('uses clientX/clientY (not pageX/pageY) so coordinates match getBoundingClientRect', () => {
// getBoundingClientRect() returns viewport-relative coords (client space).
// The drag system must use clientX/clientY from PointerEvents to match.
// If pageX/pageY were used instead, a scrolled page would cause mismatch.
const rect = { left: 100, top: 200, width: 400, height: 300 } as DOMRect

// clientX=300, clientY=350 → center of the rect
const result = pageToQuadrantPercent(300, 350, rect)
expect(result.x).toBe(50)
expect(result.y).toBe(50)

// Verify the function is named to reflect client coordinates
// (This test documents the expected coordinate system)
})
})

// --- Hook integration tests ---

describe('useDragAndDrop hook', () => {
Expand All @@ -139,8 +156,8 @@ describe('useDragAndDrop hook', () => {

act(() => {
result.current.handleDragStart(0, mockItem, {
pageX: 150,
pageY: 250,
clientX: 150,
clientY: 250,
grabX: 10,
grabY: 5,
width: 120,
Expand All @@ -167,8 +184,8 @@ describe('useDragAndDrop hook', () => {

act(() => {
result.current.handleDragStart(0, mockItem, {
pageX: 150,
pageY: 250,
clientX: 150,
clientY: 250,
grabX: 10,
grabY: 5,
width: 120,
Expand All @@ -180,7 +197,7 @@ describe('useDragAndDrop hook', () => {
window.dispatchEvent(new PointerEvent('pointermove', { clientX: 200, clientY: 300 }))
})

// PointerEvent pageX/pageY default to 0 in jsdom, so drag.x/y become 0
// PointerEvent clientX/clientY default to 0 in jsdom, so drag.x/y become 0
// The important thing is the handler runs without error
expect(result.current.drag).not.toBeNull()
})
Expand All @@ -192,8 +209,8 @@ describe('useDragAndDrop hook', () => {

act(() => {
result.current.handleDragStart(0, mockItem, {
pageX: 150,
pageY: 250,
clientX: 150,
clientY: 250,
grabX: 10,
grabY: 5,
width: 120,
Expand All @@ -215,8 +232,8 @@ describe('useDragAndDrop hook', () => {

act(() => {
result.current.handleDragStart(0, mockItem, {
pageX: 150,
pageY: 250,
clientX: 150,
clientY: 250,
grabX: 10,
grabY: 5,
width: 120,
Expand All @@ -240,8 +257,8 @@ describe('useDragAndDrop hook', () => {

act(() => {
result.current.handleDragStart(0, mockItem, {
pageX: 150,
pageY: 250,
clientX: 150,
clientY: 250,
grabX: 10,
grabY: 5,
width: 120,
Expand Down
22 changes: 11 additions & 11 deletions src/components/Card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ const DRAG_THRESHOLD = 4
export const PLACEHOLDER = 'New item...'

export interface DragStartInfo {
pageX: number
pageY: number
clientX: number
clientY: number
grabX: number
grabY: number
width: number
Expand Down Expand Up @@ -91,15 +91,15 @@ export default function Card({
const onChangeRef = useRef(onChange)
onChangeRef.current = onChange

const fireDragStart = useCallback((pageX: number, pageY: number) => {
const fireDragStart = useCallback((clientX: number, clientY: number) => {
const cardEl = cardRef.current
if (!cardEl) return
const cardRect = cardEl.getBoundingClientRect()
onDragStartRef.current({
pageX,
pageY,
grabX: pageX - cardRect.left,
grabY: pageY - cardRect.top,
clientX,
clientY,
grabX: clientX - cardRect.left,
grabY: clientY - cardRect.top,
width: cardRect.width,
height: cardRect.height,
})
Expand Down Expand Up @@ -127,8 +127,8 @@ export default function Card({
const onMove = (e: PointerEvent) => {
const p = pendingRef.current
if (!p) return
const dx = e.pageX - p.startX
const dy = e.pageY - p.startY
const dx = e.clientX - p.startX
const dy = e.clientY - p.startY
if (dx * dx + dy * dy > DRAG_THRESHOLD * DRAG_THRESHOLD) {
cleanup()
pendingRef.current = null
Expand Down Expand Up @@ -176,7 +176,7 @@ export default function Card({
}
e.preventDefault()
e.stopPropagation()
startPendingDrag(e.pageX, e.pageY)
startPendingDrag(e.clientX, e.clientY)
},
[editing, startPendingDrag],
)
Expand Down Expand Up @@ -222,7 +222,7 @@ export default function Card({
onPointerDown={(e) => {
if (e.button !== 0 || editing) return
e.preventDefault()
fireDragStart(e.pageX, e.pageY)
fireDragStart(e.clientX, e.clientY)
}}>
{editing ? (
<textarea
Expand Down
32 changes: 17 additions & 15 deletions src/hooks/useDragAndDrop.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,33 +22,35 @@ export interface UseDragAndDropOptions {
}

/**
* Given page coordinates and a bounding rect, returns clamped percentage
* coordinates within that rect.
* Given client (viewport-relative) coordinates and a bounding rect, returns
* clamped percentage coordinates within that rect.
*
* Uses clientX/clientY space to match getBoundingClientRect().
*/
export function pageToQuadrantPercent(pageX: number, pageY: number, rect: DOMRect): { x: number; y: number } {
const x = ((pageX - rect.left) / rect.width) * 100
const y = ((pageY - rect.top) / rect.height) * 100
export function pageToQuadrantPercent(clientX: number, clientY: number, rect: DOMRect): { x: number; y: number } {
const x = ((clientX - rect.left) / rect.width) * 100
const y = ((clientY - rect.top) / rect.height) * 100
return {
x: Math.max(2, Math.min(x, 85)),
y: Math.max(2, Math.min(y, 85)),
}
}

/**
* Given page coordinates and an array of element refs, returns the quadrant
* whose bounding box contains the point, or null.
* Given client (viewport-relative) coordinates and an array of element refs,
* returns the quadrant whose bounding box contains the point, or null.
*/
export function getQuadrantAtPoint(
pageX: number,
pageY: number,
clientX: number,
clientY: number,
quadrantEls: (HTMLElement | null)[],
canvasEls: (HTMLElement | null)[],
): QuadrantTarget | null {
for (let i = 0; i < 4; i++) {
const el = quadrantEls[i]
if (!el) continue
const rect = el.getBoundingClientRect()
if (pageX >= rect.left && pageX <= rect.right && pageY >= rect.top && pageY <= rect.bottom) {
if (clientX >= rect.left && clientX <= rect.right && clientY >= rect.top && clientY <= rect.bottom) {
const canvasRect = canvasEls[i]?.getBoundingClientRect() || rect
return { index: i, rect: canvasRect }
}
Expand All @@ -65,15 +67,15 @@ export default function useDragAndDrop({ quadrantRefs, canvasRefs, onDrop }: Use
if (!drag) return

const handleMove = (e: PointerEvent) => {
setDrag((prev) => (prev ? { ...prev, x: e.pageX, y: e.pageY } : null))
setDrag((prev) => (prev ? { ...prev, x: e.clientX, y: e.clientY } : null))
}

const handleUp = (e: PointerEvent) => {
setDrag((prev) => {
if (!prev) return null
const target = getQuadrantAtPoint(e.pageX, e.pageY, quadrantRefs.current!, canvasRefs.current!)
const target = getQuadrantAtPoint(e.clientX, e.clientY, quadrantRefs.current!, canvasRefs.current!)
if (target) {
const { x, y } = pageToQuadrantPercent(e.pageX - prev.grabX, e.pageY - prev.grabY, target.rect)
const { x, y } = pageToQuadrantPercent(e.clientX - prev.grabX, e.clientY - prev.grabY, target.rect)
onDropRef.current({
itemId: prev.itemId,
sourceIdx: prev.sourceIdx,
Expand Down Expand Up @@ -102,8 +104,8 @@ export default function useDragAndDrop({ quadrantRefs, canvasRefs, onDrop }: Use
grabY: info.grabY,
width: info.width,
height: info.height,
x: info.pageX,
y: info.pageY,
x: info.clientX,
y: info.clientY,
})
}, [])

Expand Down