Skip to content

Commit

Permalink
Fix issues with DragOverlay measuring (#427)
Browse files Browse the repository at this point in the history
* Fix issues with DragOverlay measuring

* Simplified measuring strategy for DragOverlay
This approach is transform agnostic to keep things simple and reliable. It can be improved later down the road.
  • Loading branch information
clauderic authored Aug 18, 2021
1 parent 59ef705 commit f96cb5d
Show file tree
Hide file tree
Showing 12 changed files with 92 additions and 88 deletions.
7 changes: 7 additions & 0 deletions .changeset/drag-overlay-measuring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@dnd-kit/core": major
"@dnd-kit/sortable": major
---

- Using transform-agnostic measurements for the DragOverlay node.
- Renamed the `overlayNode` property to `dragOverlay` on the `DndContextDescriptor` interface.
25 changes: 9 additions & 16 deletions packages/core/src/components/DndContext/DndContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
getEventCoordinates,
Transform,
useIsomorphicLayoutEffect,
useNodeRef,
useUniqueId,
} from '@dnd-kit/utilities';

Expand All @@ -31,6 +30,7 @@ import {
useAutoScroller,
useCachedNode,
useCombineActivators,
useDragOverlayMeasuring,
useDroppableMeasuring,
useScrollableAncestors,
useClientRect,
Expand Down Expand Up @@ -61,7 +61,6 @@ import {
getViewRect,
rectIntersection,
} from '../../utilities';
import {getMeasurableNode} from '../../utilities/nodes';
import {applyModifiers, Modifiers} from '../../modifiers';
import type {Active, DataRef} from '../../store/types';
import type {
Expand Down Expand Up @@ -226,13 +225,13 @@ export const DndContext = memo(function DndContext({
);
const scrollableAncestorRects = useClientRects(scrollableAncestors);

const [overlayNodeRef, setOverlayNodeRef] = useNodeRef();
const overlayNodeRect = useClientRect(
activeId ? getMeasurableNode(overlayNodeRef.current) : null
);
const dragOverlay = useDragOverlayMeasuring({
disabled: activeId == null,
forceRecompute: willRecomputeLayouts,
});

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

// The delta between the previous and new position of the draggable node
// is only relevant when there is no drag overlay
Expand All @@ -254,7 +253,7 @@ export const DndContext = memo(function DndContext({
containerNodeRect,
draggingNodeRect,
over: sensorContext.current.over,
overlayNodeRect,
overlayNodeRect: dragOverlay.rect,
scrollableAncestors,
scrollableAncestorRects,
windowRect,
Expand Down Expand Up @@ -577,13 +576,9 @@ export const DndContext = memo(function DndContext({
ariaDescribedById: {
draggable: draggableDescribedById,
},
overlayNode: {
nodeRef: overlayNodeRef,
rect: overlayNodeRect,
setRef: setOverlayNodeRef,
},
containerNodeRect,
dispatch,
dragOverlay,
draggableNodes,
droppableContainers,
droppableRects,
Expand All @@ -604,8 +599,7 @@ export const DndContext = memo(function DndContext({
activatorEvent,
activators,
containerNodeRect,
overlayNodeRect,
overlayNodeRef,
dragOverlay,
dispatch,
draggableNodes,
draggableDescribedById,
Expand All @@ -615,7 +609,6 @@ export const DndContext = memo(function DndContext({
recomputeLayouts,
scrollableAncestors,
scrollableAncestorRects,
setOverlayNodeRef,
willRecomputeLayouts,
windowRect,
]);
Expand Down
32 changes: 13 additions & 19 deletions packages/core/src/components/DragOverlay/DragOverlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {applyModifiers, Modifiers} from '../../modifiers';
import {ActiveDraggableContext} from '../DndContext';
import {useDndContext} from '../../hooks';
import type {ViewRect} from '../../types';
import {useDerivedTransform, useDropAnimation, DropAnimation} from './hooks';
import {useDropAnimation, DropAnimation} from './hooks';

type TransitionGetter = (
activatorEvent: Event | null
Expand Down Expand Up @@ -56,7 +56,7 @@ export const DragOverlay = React.memo(
draggableNodes,
activatorEvent,
over,
overlayNode,
dragOverlay,
scrollableAncestors,
scrollableAncestorRects,
windowRect,
Expand All @@ -67,28 +67,23 @@ export const DragOverlay = React.memo(
active,
activeNodeRect: activeNodeClientRect,
containerNodeRect,
draggingNodeRect: overlayNode.rect,
draggingNodeRect: dragOverlay.rect,
over,
overlayNodeRect: overlayNode.rect,
overlayNodeRect: dragOverlay.rect,
scrollableAncestors,
scrollableAncestorRects,
transform,
windowRect,
});
const derivedTransform = useDerivedTransform(
modifiedTransform,
activeNodeRect,
overlayNode.nodeRef.current
);
const isDragging = active !== null;
const intermediateTransform = derivedTransform ?? modifiedTransform;
const finalTransform = adjustScale
? intermediateTransform
? modifiedTransform
: {
...intermediateTransform,
...modifiedTransform,
scaleX: 1,
scaleY: 1,
};

const initialNodeRect = useLazyMemo<ViewRect | null>(
(previousValue) => {
if (isDragging) {
Expand Down Expand Up @@ -116,11 +111,10 @@ export const DragOverlay = React.memo(
initialNodeRect
)
: undefined,
transition: derivedTransform
? undefined
: typeof transition === 'function'
? transition(activatorEvent)
: transition,
transition:
typeof transition === 'function'
? transition(activatorEvent)
: transition,
...styleProp,
}
: undefined;
Expand All @@ -145,7 +139,7 @@ export const DragOverlay = React.memo(
duration: dropAnimation?.duration,
easing: dropAnimation?.easing,
dragSourceOpacity: dropAnimation?.dragSourceOpacity,
node: overlayNode.nodeRef.current,
node: dragOverlay.nodeRef.current,
transform: attributesSnapshot.current?.transform,
});
const shouldRender = Boolean(
Expand Down Expand Up @@ -176,7 +170,7 @@ export const DragOverlay = React.memo(
wrapperElement,
{
...otherAttributes,
ref: overlayNode.setRef,
ref: dragOverlay.setRef,
},
finalChildren
);
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/components/DragOverlay/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export {useDerivedTransform} from './useDerivedTransform';
export {useDropAnimation, DropAnimation} from './useDropAnimation';
export {useDropAnimation} from './useDropAnimation';
export type {DropAnimation} from './useDropAnimation';

This file was deleted.

1 change: 1 addition & 0 deletions packages/core/src/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export {useDndContext} from './useDndContext';
export type {UseDndContextReturnValue} from './useDndContext';
export {useDroppable} from './useDroppable';
export type {UseDroppableArguments} from './useDroppable';

export {
AutoScrollActivator,
MeasuringStrategy,
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/hooks/utilities/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ export type {
SyntheticListeners,
SyntheticListenerMap,
} from './useSyntheticListeners';
export {useRect, useClientRect, useClientRects} from './useRect';
export {useRect, useClientRect, useClientRects, useViewRect} from './useRect';
export {useDragOverlayMeasuring} from './useDragOverlayMeasuring';
51 changes: 51 additions & 0 deletions packages/core/src/hooks/utilities/useDragOverlayMeasuring.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import {useMemo} from 'react';
import {useNodeRef} from '@dnd-kit/utilities';

import {getMeasurableNode} from '../../utilities/nodes';
import {getLayoutRect} from '../../utilities/rect';
import type {DndContextDescriptor} from '../../store';
import type {ViewRect} from '../../types';

import {createUseRectFn} from './useRect';

interface Arguments {
disabled: boolean;
forceRecompute: boolean;
}

// To-do: Delete and replace with `getViewRect` when https://github.com/clauderic/dnd-kit/pull/415 is merged
function getDragOverlayRect(element: HTMLElement): ViewRect {
const {width, height, offsetLeft, offsetTop} = getLayoutRect(element);

return {
top: offsetTop,
bottom: offsetTop + height,
left: offsetLeft,
right: offsetLeft + width,
width,
height,
offsetTop,
offsetLeft,
};
}
const useDragOverlayRect = createUseRectFn(getDragOverlayRect);

export function useDragOverlayMeasuring({
disabled,
forceRecompute,
}: Arguments): DndContextDescriptor['dragOverlay'] {
const [nodeRef, setRef] = useNodeRef();
const rect = useDragOverlayRect(
disabled ? null : getMeasurableNode(nodeRef.current),
forceRecompute
);

return useMemo(
() => ({
nodeRef,
rect,
setRef,
}),
[rect, nodeRef, setRef]
);
}
5 changes: 3 additions & 2 deletions packages/core/src/hooks/utilities/useRect.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {useRef} from 'react';
import {useLazyMemo} from '@dnd-kit/utilities';

import {getBoundingClientRect} from '../../utilities';
import {getBoundingClientRect, getViewRect} from '../../utilities';
import type {LayoutRect} from '../../types';

type RectFn<T, U> = (element: U) => T;

export const useViewRect = createUseRectFn(getViewRect);
export const useClientRect = createUseRectFn(getBoundingClientRect);
export const useClientRects = createUseRectsFn(getBoundingClientRect);

Expand Down Expand Up @@ -40,7 +41,7 @@ export function useRect<T = LayoutRect, U = HTMLElement>(
);
}

function createUseRectFn<T = LayoutRect, U = HTMLElement>(
export function createUseRectFn<T = LayoutRect, U = HTMLElement>(
getRect: RectFn<T, U>
) {
return (element: U | null, forceRecompute?: boolean) =>
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/store/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const Context = createContext<DndContextDescriptor>({
droppableRects: new Map(),
droppableContainers: new DroppableContainersMap(),
over: null,
overlayNode: {
dragOverlay: {
nodeRef: {
current: null,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/store/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export interface DndContextDescriptor {
droppableContainers: DroppableContainers;
droppableRects: LayoutRectMap;
over: Over | null;
overlayNode: {
dragOverlay: {
nodeRef: MutableRefObject<HTMLElement | null>;
rect: ViewRect | null;
setRef: (element: HTMLElement | null) => void;
Expand Down
4 changes: 2 additions & 2 deletions packages/sortable/src/components/SortableContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ export function SortableContext({
}: Props) {
const {
active,
overlayNode,
dragOverlay,
droppableRects,
over,
recomputeLayouts,
willRecomputeLayouts,
} = useDndContext();
const containerId = useUniqueId(ID_PREFIX, id);
const useDragOverlay = Boolean(overlayNode.rect !== null);
const useDragOverlay = Boolean(dragOverlay.rect !== null);
const items = useMemo(
() =>
userDefinedItems.map((item) =>
Expand Down

0 comments on commit f96cb5d

Please sign in to comment.