diff --git a/.changeset/collision-detection-issues.md b/.changeset/collision-detection-issues.md new file mode 100644 index 00000000..8dbf6085 --- /dev/null +++ b/.changeset/collision-detection-issues.md @@ -0,0 +1,9 @@ +--- +"@dnd-kit/core": major +--- + +React updates in non-synthetic event handlers are now batched to reduce re-renders and prepare for React 18. + +Also fixed issues with collision detection: +- Defer measurement of droppable node rects until second render after dragging. +- Use DragOverlay's width and height in collision rect (if it is used) diff --git a/cypress.json b/cypress.json index ca64cb3d..b60db64f 100644 --- a/cypress.json +++ b/cypress.json @@ -2,5 +2,6 @@ "$schema": "https://on.cypress.io/cypress.schema.json", "supportFile": "cypress/support/index.ts", "projectId": "zkn9qu", - "baseUrl": "http://localhost:6006/" + "baseUrl": "http://localhost:6006/", + "viewportHeight": 1000 } diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index 53be1c4d..6b005af7 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -135,10 +135,20 @@ Cypress.Commands.add( cy.wrap(subject, {log: false}) .focus({log: false}) - .type(Keys.Space, {force: true, log: false}) + .type(Keys.Space, { + delay: 150, + scrollBehavior: false, + force: true, + log: false, + }) .closest('body') - .type(arrowKey.repeat(times), {delay: 150, force: true}) - .type(Keys.Space, {log: false, force: true}); + .type(arrowKey.repeat(times), { + scrollBehavior: false, + delay: 150, + force: true, + }) + .wait(150) + .type(Keys.Space, {scrollBehavior: false, log: false, force: true}); } ); diff --git a/packages/core/src/components/DndContext/DndContext.tsx b/packages/core/src/components/DndContext/DndContext.tsx index 20246758..52b1ec8a 100644 --- a/packages/core/src/components/DndContext/DndContext.tsx +++ b/packages/core/src/components/DndContext/DndContext.tsx @@ -8,6 +8,7 @@ import React, { useRef, useState, } from 'react'; +import {unstable_batchedUpdates} from 'react-dom'; import { add, Transform, @@ -59,6 +60,7 @@ import { getEventCoordinates, rectIntersection, } from '../../utilities'; +import {getMeasurableNode} from '../../utilities/nodes'; import {applyModifiers, Modifiers} from '../../modifiers'; import type { Active, @@ -142,6 +144,7 @@ export const DndContext = memo(function DndContext({ type: null, event: null, })); + const [isDragging, setIsDragging] = useState(false); const { draggable: {active: activeId, nodes: draggableNodes, translate}, droppable: {containers: droppableContainers}, @@ -173,7 +176,7 @@ export const DndContext = memo(function DndContext({ recomputeLayouts, willRecomputeLayouts, } = useLayoutMeasuring(droppableContainers, { - dragging: activeId != null, + dragging: isDragging, dependencies: [translate.x, translate.y], config: layoutMeasuring, }); @@ -216,11 +219,11 @@ export const DndContext = memo(function DndContext({ const [overlayNodeRef, setOverlayNodeRef] = useNodeRef(); const overlayNodeRect = useClientRect( - activeId ? overlayNodeRef.current : null, + activeId ? getMeasurableNode(overlayNodeRef.current) : null, willRecomputeLayouts ); - const draggingNodeRect = overlayNodeRect ?? activeNodeClientRect; + const draggingNodeRect = overlayNodeRect ?? activeNodeRect; const modifiedTranslate = applyModifiers(modifiers, { transform: { x: translate.x - nodeRectDelta.x, @@ -247,9 +250,20 @@ export const DndContext = memo(function DndContext({ const scrollAdjustedTranslate = add(modifiedTranslate, scrollAdjustment); - const translatedRect = activeNodeRect - ? getAdjustedRect(activeNodeRect, modifiedTranslate) - : null; + // The overlay node's position is based on the active node's position. + // We assume that any difference in positioning is for visual purposes only + // and shouldn't affect collision detection. However, the computed height of + // the overlay node should affect the collision rect. + const rect = + overlayNodeRect && activeNodeRect + ? { + ...activeNodeRect, + height: overlayNodeRect.height, + width: overlayNodeRect.width, + } + : activeNodeRect; + + const translatedRect = rect ? getAdjustedRect(rect, modifiedTranslate) : null; const collisionRect = translatedRect ? getAdjustedRect(translatedRect, scrollAdjustment) @@ -320,13 +334,15 @@ export const DndContext = memo(function DndContext({ active: {id, data: node.data, rect: activeRects}, }; - dispatch({ - type: Action.DragStart, - initialCoordinates, - active: id, + unstable_batchedUpdates(() => { + dispatch({ + type: Action.DragStart, + initialCoordinates, + active: id, + }); + setMonitorState({type: Action.DragStart, event}); + onDragStart?.(event); }); - setMonitorState({type: Action.DragStart, event}); - onDragStart?.(event); }, onMove(coordinates) { dispatch({ @@ -338,8 +354,10 @@ export const DndContext = memo(function DndContext({ onCancel: createHandler(Action.DragCancel), }); - setActiveSensor(sensorInstance); - setActivatorEvent(event.nativeEvent); + unstable_batchedUpdates(() => { + setActiveSensor(sensorInstance); + setActivatorEvent(event.nativeEvent); + }); function createHandler(type: Action.DragEnd | Action.DragCancel) { return async function handler() { @@ -366,17 +384,21 @@ export const DndContext = memo(function DndContext({ activeRef.current = null; - dispatch({type}); - setActiveSensor(null); - setActivatorEvent(null); + unstable_batchedUpdates(() => { + dispatch({type}); + setIsDragging(false); + setActiveSensor(null); + setActivatorEvent(null); - if (event) { - const {onDragCancel, onDragEnd} = latestProps.current; - const handler = type === Action.DragEnd ? onDragEnd : onDragCancel; + if (event) { + const {onDragCancel, onDragEnd} = latestProps.current; + const handler = + type === Action.DragEnd ? onDragEnd : onDragCancel; - setMonitorState({type, event}); - handler?.(event); - } + setMonitorState({type, event}); + handler?.(event); + } + }); }; } }, @@ -427,6 +449,12 @@ export const DndContext = memo(function DndContext({ Object.values(props) ); + useEffect(() => { + if (activeId != null) { + setIsDragging(true); + } + }, [activeId]); + useEffect(() => { if (!active) { initialActiveNodeRectRef.current = null; diff --git a/packages/core/src/components/DragOverlay/hooks/useDropAnimation.ts b/packages/core/src/components/DragOverlay/hooks/useDropAnimation.ts index b0149d93..74c6f6b5 100644 --- a/packages/core/src/components/DragOverlay/hooks/useDropAnimation.ts +++ b/packages/core/src/components/DragOverlay/hooks/useDropAnimation.ts @@ -4,6 +4,7 @@ import {CSS, Transform, useIsomorphicLayoutEffect} from '@dnd-kit/utilities'; import type {UniqueIdentifier} from '../../../types'; import type {DraggableNodes} from '../../../store'; import {getViewRect} from '../../../utilities'; +import {getMeasurableNode} from '../../../utilities/nodes'; export interface DropAnimation { duration: number; @@ -49,7 +50,7 @@ export function useDropAnimation({ const finalNode = draggableNodes[activeId]?.node.current; if (transform && node && finalNode && finalNode.parentNode !== null) { - const fromNode = node.children.length > 1 ? node : node.children[0]; + const fromNode = getMeasurableNode(node); if (fromNode) { const from = fromNode.getBoundingClientRect(); diff --git a/packages/core/src/utilities/nodes/getMeasurableNode.ts b/packages/core/src/utilities/nodes/getMeasurableNode.ts new file mode 100644 index 00000000..f568d769 --- /dev/null +++ b/packages/core/src/utilities/nodes/getMeasurableNode.ts @@ -0,0 +1,14 @@ +export function getMeasurableNode( + node: HTMLElement | undefined | null +): HTMLElement | null { + if (!node) { + return null; + } + + if (node.children.length > 1) { + return node; + } + const firstChild = node.children[0]; + + return firstChild instanceof HTMLElement ? firstChild : node; +} diff --git a/packages/core/src/utilities/nodes/index.ts b/packages/core/src/utilities/nodes/index.ts new file mode 100644 index 00000000..54ee0703 --- /dev/null +++ b/packages/core/src/utilities/nodes/index.ts @@ -0,0 +1 @@ +export {getMeasurableNode} from './getMeasurableNode'; diff --git a/stories/2 - Presets/Sortable/1-Vertical.story.tsx b/stories/2 - Presets/Sortable/1-Vertical.story.tsx index 4aeceb2e..42cae91a 100644 --- a/stories/2 - Presets/Sortable/1-Vertical.story.tsx +++ b/stories/2 - Presets/Sortable/1-Vertical.story.tsx @@ -140,7 +140,7 @@ export const RerenderBeforeSorting = () => { {...props} wrapperStyle={({isDragging}) => { return { - height: isDragging ? 80 : undefined, + height: isDragging ? undefined : 200, }; }} /> diff --git a/stories/2 - Presets/Sortable/3-Grid.story.tsx b/stories/2 - Presets/Sortable/3-Grid.story.tsx index 9a196503..17258fc0 100644 --- a/stories/2 - Presets/Sortable/3-Grid.story.tsx +++ b/stories/2 - Presets/Sortable/3-Grid.story.tsx @@ -1,6 +1,5 @@ import React from 'react'; import {LayoutMeasuringStrategy} from '@dnd-kit/core'; -import {restrictToWindowEdges} from '@dnd-kit/modifiers'; import { AnimateLayoutChanges, defaultAnimateLayoutChanges, @@ -22,7 +21,6 @@ const props: Partial = { width: 140, height: 140, }), - modifiers: [restrictToWindowEdges], }; export const BasicSetup = () => ; diff --git a/stories/2 - Presets/Sortable/5-Virtualized.story.tsx b/stories/2 - Presets/Sortable/5-Virtualized.story.tsx index db1759a5..7f806ba4 100644 --- a/stories/2 - Presets/Sortable/5-Virtualized.story.tsx +++ b/stories/2 - Presets/Sortable/5-Virtualized.story.tsx @@ -112,6 +112,9 @@ function Sortable({ overIndex: -1, isDragOverlay: true, })} + wrapperStyle={{ + padding: 5, + }} dragOverlay /> ) : null}