From c1b3b5a0be5759b707e22c4e1b1236aaa82773a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claud=C3=A9ric=20Demers?= Date: Mon, 14 Mar 2022 22:12:28 -0400 Subject: [PATCH] Fixed an issue with collision detection using stale rects The `rect.current` ref stored on DroppableContainers can be stale if measuring is scheduled but has not completed yet. Collision detection algorithms should use the `droppableRects` map instead to get the latest, most up-to-date measurement of a droppable container in order to avoid computing collisions against stale rects. --- .changeset/stale-collision-rects.md | 16 ++++++++++++++++ .../src/components/DndContext/DndContext.tsx | 1 + .../src/utilities/algorithms/closestCenter.ts | 7 +++---- .../src/utilities/algorithms/closestCorners.ts | 7 +++---- .../src/utilities/algorithms/pointerWithin.ts | 7 +++---- .../src/utilities/algorithms/rectIntersection.ts | 7 +++---- packages/core/src/utilities/algorithms/types.ts | 3 ++- .../keyboard/sortableKeyboardCoordinates.ts | 11 ++++++++++- 8 files changed, 41 insertions(+), 18 deletions(-) create mode 100644 .changeset/stale-collision-rects.md diff --git a/.changeset/stale-collision-rects.md b/.changeset/stale-collision-rects.md new file mode 100644 index 00000000..9b1f6f32 --- /dev/null +++ b/.changeset/stale-collision-rects.md @@ -0,0 +1,16 @@ +--- +'@dnd-kit/core': minor +--- + +Fixed an issue with collision detection using stale rects. The `droppableRects` property has been added to the `CollisionDetection` interface. + +All built-in collision detection algorithms have been updated to get the rect for a given droppable container from `droppableRects` rather than from the `rect.current` ref: + +```diff +- const rect = droppableContainers.get(id).rect.current; ++ const rect = droppableRects.get(id); +``` + +The `rect.current` ref stored on DroppableContainers can be stale if measuring is scheduled but has not completed yet. Collision detection algorithms should use the `droppableRects` map instead to get the latest, most up-to-date measurement of a droppable container in order to avoid computing collisions against stale rects. + +This is not a breaking change. Hoever, if you've forked any of the built-in collision detection algorithms or you've authored custom ones, we highly recommend you update your use-cases to avoid possibly computing collisions against stale rects. diff --git a/packages/core/src/components/DndContext/DndContext.tsx b/packages/core/src/components/DndContext/DndContext.tsx index e902e273..2e267da0 100644 --- a/packages/core/src/components/DndContext/DndContext.tsx +++ b/packages/core/src/components/DndContext/DndContext.tsx @@ -292,6 +292,7 @@ export const DndContext = memo(function DndContext({ ? collisionDetection({ active, collisionRect, + droppableRects, droppableContainers: enabledDroppableContainers, pointerCoordinates, }) diff --git a/packages/core/src/utilities/algorithms/closestCenter.ts b/packages/core/src/utilities/algorithms/closestCenter.ts index 7469c29e..e381b39d 100644 --- a/packages/core/src/utilities/algorithms/closestCenter.ts +++ b/packages/core/src/utilities/algorithms/closestCenter.ts @@ -24,6 +24,7 @@ function centerOfRectangle( */ export const closestCenter: CollisionDetection = ({ collisionRect, + droppableRects, droppableContainers, }) => { const centerRect = centerOfRectangle( @@ -34,10 +35,8 @@ export const closestCenter: CollisionDetection = ({ const collisions: CollisionDescriptor[] = []; for (const droppableContainer of droppableContainers) { - const { - id, - rect: {current: rect}, - } = droppableContainer; + const {id} = droppableContainer; + const rect = droppableRects.get(id); if (rect) { const distBetween = distanceBetween(centerOfRectangle(rect), centerRect); diff --git a/packages/core/src/utilities/algorithms/closestCorners.ts b/packages/core/src/utilities/algorithms/closestCorners.ts index f4e28a2a..4e1bb488 100644 --- a/packages/core/src/utilities/algorithms/closestCorners.ts +++ b/packages/core/src/utilities/algorithms/closestCorners.ts @@ -9,16 +9,15 @@ import {cornersOfRectangle, sortCollisionsAsc} from './helpers'; */ export const closestCorners: CollisionDetection = ({ collisionRect, + droppableRects, droppableContainers, }) => { const corners = cornersOfRectangle(collisionRect); const collisions: CollisionDescriptor[] = []; for (const droppableContainer of droppableContainers) { - const { - id, - rect: {current: rect}, - } = droppableContainer; + const {id} = droppableContainer; + const rect = droppableRects.get(id); if (rect) { const rectCorners = cornersOfRectangle(rect); diff --git a/packages/core/src/utilities/algorithms/pointerWithin.ts b/packages/core/src/utilities/algorithms/pointerWithin.ts index 83e0758e..ca51460c 100644 --- a/packages/core/src/utilities/algorithms/pointerWithin.ts +++ b/packages/core/src/utilities/algorithms/pointerWithin.ts @@ -20,6 +20,7 @@ function isPointWithinRect(point: Coordinates, rect: ClientRect): boolean { */ export const pointerWithin: CollisionDetection = ({ droppableContainers, + droppableRects, pointerCoordinates, }) => { if (!pointerCoordinates) { @@ -29,10 +30,8 @@ export const pointerWithin: CollisionDetection = ({ const collisions: CollisionDescriptor[] = []; for (const droppableContainer of droppableContainers) { - const { - id, - rect: {current: rect}, - } = droppableContainer; + const {id} = droppableContainer; + const rect = droppableRects.get(id); if (rect && isPointWithinRect(pointerCoordinates, rect)) { /* There may be more than a single rectangle intersecting diff --git a/packages/core/src/utilities/algorithms/rectIntersection.ts b/packages/core/src/utilities/algorithms/rectIntersection.ts index c9739708..efaab91f 100644 --- a/packages/core/src/utilities/algorithms/rectIntersection.ts +++ b/packages/core/src/utilities/algorithms/rectIntersection.ts @@ -37,15 +37,14 @@ export function getIntersectionRatio( */ export const rectIntersection: CollisionDetection = ({ collisionRect, + droppableRects, droppableContainers, }) => { const collisions: CollisionDescriptor[] = []; for (const droppableContainer of droppableContainers) { - const { - id, - rect: {current: rect}, - } = droppableContainer; + const {id} = droppableContainer; + const rect = droppableRects.get(id); if (rect) { const intersectionRatio = getIntersectionRatio(rect, collisionRect); diff --git a/packages/core/src/utilities/algorithms/types.ts b/packages/core/src/utilities/algorithms/types.ts index 432bcb7e..a0e0c36d 100644 --- a/packages/core/src/utilities/algorithms/types.ts +++ b/packages/core/src/utilities/algorithms/types.ts @@ -1,4 +1,4 @@ -import type {Active, Data, DroppableContainer} from '../../store'; +import type {Active, Data, DroppableContainer, RectMap} from '../../store'; import type {Coordinates, ClientRect, UniqueIdentifier} from '../../types'; export interface Collision { @@ -17,6 +17,7 @@ export interface CollisionDescriptor extends Collision { export type CollisionDetection = (args: { active: Active; collisionRect: ClientRect; + droppableRects: RectMap; droppableContainers: DroppableContainer[]; pointerCoordinates: Coordinates | null; }) => Collision[]; diff --git a/packages/sortable/src/sensors/keyboard/sortableKeyboardCoordinates.ts b/packages/sortable/src/sensors/keyboard/sortableKeyboardCoordinates.ts index 022b0df4..6450af12 100644 --- a/packages/sortable/src/sensors/keyboard/sortableKeyboardCoordinates.ts +++ b/packages/sortable/src/sensors/keyboard/sortableKeyboardCoordinates.ts @@ -16,7 +16,15 @@ const directions: string[] = [ export const sortableKeyboardCoordinates: KeyboardCoordinateGetter = ( event, - {context: {active, droppableContainers, collisionRect, scrollableAncestors}} + { + context: { + active, + droppableRects, + droppableContainers, + collisionRect, + scrollableAncestors, + }, + } ) => { if (directions.includes(event.code)) { event.preventDefault(); @@ -65,6 +73,7 @@ export const sortableKeyboardCoordinates: KeyboardCoordinateGetter = ( const collisions = closestCorners({ active, collisionRect: collisionRect, + droppableRects, droppableContainers: filteredContainers, pointerCoordinates: null, });