Skip to content

Commit

Permalink
fix: compute collision against DragOverlay child bounding rect (#408)
Browse files Browse the repository at this point in the history
* fix: compute collision against DragOverlay child bounding rect
* Fix assumptions around usage of drag source rect instead of drag overlay

When dragging without a drag overlay, we need to compute a delta between the draggable node's initial position and new position if it is reparented.

This logic isn't relevant when using the DragOverlay. It was working before because we were only using the width and height of the drag overlay in the collision rect

Co-authored-by: Billy Main <wmain@netflix.com>
Co-authored-by: Claudéric Demers <clauderic.d@gmail.com>
  • Loading branch information
3 people committed Aug 18, 2021
1 parent ad4a26f commit dea715c
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 24 deletions.
5 changes: 5 additions & 0 deletions .changeset/dragoverlay-collision-rect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@dnd-kit/core": minor
---

The collision rect is now completely based on the position of the `DragOverlay` when it is used. Previously, only the `width` and `height` properties of the `DragOverlay` were used for the collision rect, while the `top`, `left`, `bottom` and `right` properties were derived from the active node rect. This new approach is more aligned with developers would expect, but could cause issues for consumers that were relying on the previous (incorrect) behavior.
30 changes: 13 additions & 17 deletions packages/core/src/components/DndContext/DndContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ export const DndContext = memo(function DndContext({
const activeNodeClientRect = useClientRect(activeNode);
const initialActiveNodeRectRef = useRef<ViewRect | null>(null);
const initialActiveNodeRect = initialActiveNodeRectRef.current;
const nodeRectDelta = getRectDelta(activeNodeRect, initialActiveNodeRect);
const sensorContext = useRef<SensorContext>({
active: null,
activeNode,
Expand Down Expand Up @@ -229,11 +228,19 @@ export const DndContext = memo(function DndContext({

const [overlayNodeRef, setOverlayNodeRef] = useNodeRef();
const overlayNodeRect = useClientRect(
activeId ? getMeasurableNode(overlayNodeRef.current) : null,
willRecomputeLayouts
activeId ? getMeasurableNode(overlayNodeRef.current) : null
);

// Use the rect of the drag overlay if it is mounted
const draggingNodeRect = overlayNodeRect ?? activeNodeRect;

// The delta between the previous and new position of the draggable node
// is only relevant when there is no drag overlay
const nodeRectDelta =
draggingNodeRect === activeNodeRect
? getRectDelta(activeNodeRect, initialActiveNodeRect)
: defaultCoordinates;

const modifiedTranslate = applyModifiers(modifiers, {
transform: {
x: translate.x - nodeRectDelta.x,
Expand Down Expand Up @@ -261,20 +268,9 @@ export const DndContext = memo(function DndContext({

const scrollAdjustedTranslate = add(modifiedTranslate, scrollAdjustment);

// 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 translatedRect = draggingNodeRect
? getAdjustedRect(draggingNodeRect, modifiedTranslate)
: null;

const collisionRect = translatedRect
? getAdjustedRect(translatedRect, scrollAdjustment)
Expand Down
25 changes: 18 additions & 7 deletions packages/core/src/components/DragOverlay/DragOverlay.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React, {useContext, useEffect, useRef} from 'react';
import {CSS} from '@dnd-kit/utilities';
import {CSS, useLazyMemo} from '@dnd-kit/utilities';

import {getRelativeTransformOrigin} from '../../utilities';
import {applyModifiers, Modifiers} from '../../modifiers';
import {ActiveDraggableContext} from '../DndContext';
import {useDndContext} from '../../hooks';
import type {ViewRect} from '../../types';
import {useDerivedTransform, useDropAnimation, DropAnimation} from './hooks';

type TransitionGetter = (
Expand Down Expand Up @@ -88,21 +89,31 @@ export const DragOverlay = React.memo(
scaleX: 1,
scaleY: 1,
};
const style: React.CSSProperties | undefined = activeNodeRect
const initialNodeRect = useLazyMemo<ViewRect | null>(
(previousValue) => {
if (isDragging) {
return previousValue ?? activeNodeRect;
}

return null;
},
[isDragging, activeNodeRect]
);
const style: React.CSSProperties | undefined = initialNodeRect
? {
position: 'fixed',
width: activeNodeRect.width,
height: activeNodeRect.height,
top: activeNodeRect.top,
left: activeNodeRect.left,
width: initialNodeRect.width,
height: initialNodeRect.height,
top: initialNodeRect.top,
left: initialNodeRect.left,
zIndex,
transform: CSS.Transform.toString(finalTransform),
touchAction: 'none',
transformOrigin:
adjustScale && activatorEvent
? getRelativeTransformOrigin(
activatorEvent as MouseEvent | KeyboardEvent | TouchEvent,
activeNodeRect
initialNodeRect
)
: undefined,
transition: derivedTransform
Expand Down

0 comments on commit dea715c

Please sign in to comment.