From a1c1a5e5288145fe4d453e35b2ed4bc2aacc15e3 Mon Sep 17 00:00:00 2001 From: Clauderic Demers Date: Wed, 18 Aug 2021 15:04:54 -0400 Subject: [PATCH 1/4] Fix issues with DragOverlay measuring --- .../src/components/DndContext/DndContext.tsx | 25 ++++----- .../components/DragOverlay/DragOverlay.tsx | 27 +++++----- .../src/components/DragOverlay/hooks/index.ts | 4 +- .../DragOverlay/hooks/useDerivedTransform.ts | 44 ---------------- packages/core/src/hooks/index.ts | 1 + packages/core/src/hooks/utilities/index.ts | 3 +- .../utilities/useDragOverlayMeasuring.ts | 51 +++++++++++++++++++ packages/core/src/hooks/utilities/useRect.ts | 3 +- packages/core/src/store/context.ts | 5 +- packages/core/src/store/types.ts | 4 +- .../src/components/SortableContext.tsx | 4 +- 11 files changed, 88 insertions(+), 83 deletions(-) delete mode 100644 packages/core/src/components/DragOverlay/hooks/useDerivedTransform.ts create mode 100644 packages/core/src/hooks/utilities/useDragOverlayMeasuring.ts diff --git a/packages/core/src/components/DndContext/DndContext.tsx b/packages/core/src/components/DndContext/DndContext.tsx index e8ff3322..5325669c 100644 --- a/packages/core/src/components/DndContext/DndContext.tsx +++ b/packages/core/src/components/DndContext/DndContext.tsx @@ -14,7 +14,6 @@ import { getEventCoordinates, Transform, useIsomorphicLayoutEffect, - useNodeRef, useUniqueId, } from '@dnd-kit/utilities'; @@ -31,6 +30,7 @@ import { useAutoScroller, useCachedNode, useCombineActivators, + useDragOverlayMeasuring, useDroppableMeasuring, useScrollableAncestors, useClientRect, @@ -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 { @@ -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 @@ -254,7 +253,7 @@ export const DndContext = memo(function DndContext({ containerNodeRect, draggingNodeRect, over: sensorContext.current.over, - overlayNodeRect, + overlayNodeRect: dragOverlay.rect, scrollableAncestors, scrollableAncestorRects, windowRect, @@ -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, @@ -604,8 +599,7 @@ export const DndContext = memo(function DndContext({ activatorEvent, activators, containerNodeRect, - overlayNodeRect, - overlayNodeRef, + dragOverlay, dispatch, draggableNodes, draggableDescribedById, @@ -615,7 +609,6 @@ export const DndContext = memo(function DndContext({ recomputeLayouts, scrollableAncestors, scrollableAncestorRects, - setOverlayNodeRef, willRecomputeLayouts, windowRect, ]); diff --git a/packages/core/src/components/DragOverlay/DragOverlay.tsx b/packages/core/src/components/DragOverlay/DragOverlay.tsx index ae3f9c78..59be426d 100644 --- a/packages/core/src/components/DragOverlay/DragOverlay.tsx +++ b/packages/core/src/components/DragOverlay/DragOverlay.tsx @@ -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 @@ -56,7 +56,7 @@ export const DragOverlay = React.memo( draggableNodes, activatorEvent, over, - overlayNode, + dragOverlay, scrollableAncestors, scrollableAncestorRects, windowRect, @@ -67,28 +67,25 @@ 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, }; + + dragOverlay.transform.current = finalTransform; + const initialNodeRect = useLazyMemo( (previousValue) => { if (isDragging) { @@ -116,7 +113,7 @@ export const DragOverlay = React.memo( initialNodeRect ) : undefined, - transition: derivedTransform + transition: modifiedTransform ? undefined : typeof transition === 'function' ? transition(activatorEvent) @@ -145,7 +142,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( @@ -176,7 +173,7 @@ export const DragOverlay = React.memo( wrapperElement, { ...otherAttributes, - ref: overlayNode.setRef, + ref: dragOverlay.setRef, }, finalChildren ); diff --git a/packages/core/src/components/DragOverlay/hooks/index.ts b/packages/core/src/components/DragOverlay/hooks/index.ts index ed2e49d7..21a81cdd 100644 --- a/packages/core/src/components/DragOverlay/hooks/index.ts +++ b/packages/core/src/components/DragOverlay/hooks/index.ts @@ -1,2 +1,2 @@ -export {useDerivedTransform} from './useDerivedTransform'; -export {useDropAnimation, DropAnimation} from './useDropAnimation'; +export {useDropAnimation} from './useDropAnimation'; +export type {DropAnimation} from './useDropAnimation'; diff --git a/packages/core/src/components/DragOverlay/hooks/useDerivedTransform.ts b/packages/core/src/components/DragOverlay/hooks/useDerivedTransform.ts deleted file mode 100644 index 5f524d56..00000000 --- a/packages/core/src/components/DragOverlay/hooks/useDerivedTransform.ts +++ /dev/null @@ -1,44 +0,0 @@ -import {useRef} from 'react'; -import {Transform, useLazyMemo} from '@dnd-kit/utilities'; - -import type {ViewRect} from '../../../types'; - -export function useDerivedTransform( - transform: Transform, - rect: ViewRect | null, - overlayNode: HTMLElement | null -) { - const prevRect = useRef(rect); - - return useLazyMemo( - (previousValue) => { - const initial = prevRect.current; - - if (rect !== initial) { - if (rect && initial) { - const layoutHasChanged = - initial.left !== rect.left || initial.top !== rect.top; - - if (layoutHasChanged && !previousValue) { - const overlayNodeRect = overlayNode?.getBoundingClientRect(); - - if (overlayNodeRect) { - const delta = { - ...transform, - x: overlayNodeRect.left - rect.left, - y: overlayNodeRect.top - rect.top, - }; - - return delta; - } - } - } - - prevRect.current = rect; - } - - return undefined; - }, - [rect, transform, overlayNode] - ); -} diff --git a/packages/core/src/hooks/index.ts b/packages/core/src/hooks/index.ts index 35d3f3ad..b778fc8e 100644 --- a/packages/core/src/hooks/index.ts +++ b/packages/core/src/hooks/index.ts @@ -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, diff --git a/packages/core/src/hooks/utilities/index.ts b/packages/core/src/hooks/utilities/index.ts index e322f082..120f2d2e 100644 --- a/packages/core/src/hooks/utilities/index.ts +++ b/packages/core/src/hooks/utilities/index.ts @@ -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'; diff --git a/packages/core/src/hooks/utilities/useDragOverlayMeasuring.ts b/packages/core/src/hooks/utilities/useDragOverlayMeasuring.ts new file mode 100644 index 00000000..2bf4bba5 --- /dev/null +++ b/packages/core/src/hooks/utilities/useDragOverlayMeasuring.ts @@ -0,0 +1,51 @@ +import {useMemo, useRef} from 'react'; +import {Transform, useNodeRef} from '@dnd-kit/utilities'; + +import {getMeasurableNode} from '../../utilities/nodes'; +import type {DndContextDescriptor} from '../../store'; +import type {ClientRect} from '../../types'; + +import {useClientRect} from './useRect'; + +interface Arguments { + disabled: boolean; + forceRecompute: boolean; +} + +export function useDragOverlayMeasuring({ + disabled, + forceRecompute, +}: Arguments): DndContextDescriptor['dragOverlay'] { + const transform = useRef(null); + const [nodeRef, setRef] = useNodeRef(); + const measuredOverlayRect = useClientRect( + disabled ? null : getMeasurableNode(nodeRef.current), + forceRecompute + ); + + const rect = useMemo(() => { + return measuredOverlayRect && transform.current + ? add(measuredOverlayRect, transform.current) + : null; + }, [measuredOverlayRect]); + + return useMemo( + () => ({ + nodeRef, + rect, + setRef, + transform, + }), + [rect, transform, nodeRef, setRef] + ); +} + +function add(rect: ClientRect, transform: Transform) { + return { + ...rect, + left: Math.round(rect.left - transform.x), + right: Math.round(rect.right - transform.x), + top: Math.round(rect.top - transform.y), + bottom: Math.round(rect.bottom - transform.y), + }; +} diff --git a/packages/core/src/hooks/utilities/useRect.ts b/packages/core/src/hooks/utilities/useRect.ts index 47f131ea..6eacc536 100644 --- a/packages/core/src/hooks/utilities/useRect.ts +++ b/packages/core/src/hooks/utilities/useRect.ts @@ -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 = (element: U) => T; +export const useViewRect = createUseRectFn(getViewRect); export const useClientRect = createUseRectFn(getBoundingClientRect); export const useClientRects = createUseRectsFn(getBoundingClientRect); diff --git a/packages/core/src/store/context.ts b/packages/core/src/store/context.ts index b50cb6a8..6a4ec18d 100644 --- a/packages/core/src/store/context.ts +++ b/packages/core/src/store/context.ts @@ -20,12 +20,15 @@ export const Context = createContext({ droppableRects: new Map(), droppableContainers: new DroppableContainersMap(), over: null, - overlayNode: { + dragOverlay: { nodeRef: { current: null, }, rect: null, setRef: noop, + transform: { + current: null, + }, }, scrollableAncestors: [], scrollableAncestorRects: [], diff --git a/packages/core/src/store/types.ts b/packages/core/src/store/types.ts index 01b2c6eb..e003e31d 100644 --- a/packages/core/src/store/types.ts +++ b/packages/core/src/store/types.ts @@ -1,4 +1,5 @@ import type {MutableRefObject} from 'react'; +import type {Transform} from '@dnd-kit/utilities'; import type { Coordinates, @@ -92,9 +93,10 @@ export interface DndContextDescriptor { droppableContainers: DroppableContainers; droppableRects: LayoutRectMap; over: Over | null; - overlayNode: { + dragOverlay: { nodeRef: MutableRefObject; rect: ViewRect | null; + transform: MutableRefObject; setRef: (element: HTMLElement | null) => void; }; scrollableAncestors: Element[]; diff --git a/packages/sortable/src/components/SortableContext.tsx b/packages/sortable/src/components/SortableContext.tsx index 77f9c298..fb412070 100644 --- a/packages/sortable/src/components/SortableContext.tsx +++ b/packages/sortable/src/components/SortableContext.tsx @@ -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) => From 5e74d1b58f8325260e5e99e55613c128fa6e1b5d Mon Sep 17 00:00:00 2001 From: Clauderic Demers Date: Wed, 18 Aug 2021 16:23:34 -0400 Subject: [PATCH 2/4] Fix leftover logic from previous derived transform strategy --- packages/core/src/components/DragOverlay/DragOverlay.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/core/src/components/DragOverlay/DragOverlay.tsx b/packages/core/src/components/DragOverlay/DragOverlay.tsx index 59be426d..a1224f48 100644 --- a/packages/core/src/components/DragOverlay/DragOverlay.tsx +++ b/packages/core/src/components/DragOverlay/DragOverlay.tsx @@ -113,11 +113,10 @@ export const DragOverlay = React.memo( initialNodeRect ) : undefined, - transition: modifiedTransform - ? undefined - : typeof transition === 'function' - ? transition(activatorEvent) - : transition, + transition: + typeof transition === 'function' + ? transition(activatorEvent) + : transition, ...styleProp, } : undefined; From 534ab34fa837941734f732d4c1237ee1ebdf0063 Mon Sep 17 00:00:00 2001 From: Clauderic Demers Date: Wed, 18 Aug 2021 17:02:55 -0400 Subject: [PATCH 3/4] Simplified measuring strategy for DragOverlay This approach is transform agnostic to keep things simple and reliable. It can be improved later down the road. --- .../components/DragOverlay/DragOverlay.tsx | 2 - .../utilities/useDragOverlayMeasuring.ts | 48 +++++++++---------- packages/core/src/hooks/utilities/useRect.ts | 2 +- packages/core/src/store/context.ts | 3 -- packages/core/src/store/types.ts | 2 - 5 files changed, 25 insertions(+), 32 deletions(-) diff --git a/packages/core/src/components/DragOverlay/DragOverlay.tsx b/packages/core/src/components/DragOverlay/DragOverlay.tsx index a1224f48..5ad14802 100644 --- a/packages/core/src/components/DragOverlay/DragOverlay.tsx +++ b/packages/core/src/components/DragOverlay/DragOverlay.tsx @@ -84,8 +84,6 @@ export const DragOverlay = React.memo( scaleY: 1, }; - dragOverlay.transform.current = finalTransform; - const initialNodeRect = useLazyMemo( (previousValue) => { if (isDragging) { diff --git a/packages/core/src/hooks/utilities/useDragOverlayMeasuring.ts b/packages/core/src/hooks/utilities/useDragOverlayMeasuring.ts index 2bf4bba5..b9b3d1da 100644 --- a/packages/core/src/hooks/utilities/useDragOverlayMeasuring.ts +++ b/packages/core/src/hooks/utilities/useDragOverlayMeasuring.ts @@ -1,51 +1,51 @@ -import {useMemo, useRef} from 'react'; -import {Transform, useNodeRef} from '@dnd-kit/utilities'; +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 {ClientRect} from '../../types'; +import type {ViewRect} from '../../types'; -import {useClientRect} from './useRect'; +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 transform = useRef(null); const [nodeRef, setRef] = useNodeRef(); - const measuredOverlayRect = useClientRect( + const rect = useDragOverlayRect( disabled ? null : getMeasurableNode(nodeRef.current), forceRecompute ); - const rect = useMemo(() => { - return measuredOverlayRect && transform.current - ? add(measuredOverlayRect, transform.current) - : null; - }, [measuredOverlayRect]); - return useMemo( () => ({ nodeRef, rect, setRef, - transform, }), - [rect, transform, nodeRef, setRef] + [rect, nodeRef, setRef] ); } - -function add(rect: ClientRect, transform: Transform) { - return { - ...rect, - left: Math.round(rect.left - transform.x), - right: Math.round(rect.right - transform.x), - top: Math.round(rect.top - transform.y), - bottom: Math.round(rect.bottom - transform.y), - }; -} diff --git a/packages/core/src/hooks/utilities/useRect.ts b/packages/core/src/hooks/utilities/useRect.ts index 6eacc536..74bcaa8a 100644 --- a/packages/core/src/hooks/utilities/useRect.ts +++ b/packages/core/src/hooks/utilities/useRect.ts @@ -41,7 +41,7 @@ export function useRect( ); } -function createUseRectFn( +export function createUseRectFn( getRect: RectFn ) { return (element: U | null, forceRecompute?: boolean) => diff --git a/packages/core/src/store/context.ts b/packages/core/src/store/context.ts index 6a4ec18d..e6f052b9 100644 --- a/packages/core/src/store/context.ts +++ b/packages/core/src/store/context.ts @@ -26,9 +26,6 @@ export const Context = createContext({ }, rect: null, setRef: noop, - transform: { - current: null, - }, }, scrollableAncestors: [], scrollableAncestorRects: [], diff --git a/packages/core/src/store/types.ts b/packages/core/src/store/types.ts index e003e31d..e0bcfa5e 100644 --- a/packages/core/src/store/types.ts +++ b/packages/core/src/store/types.ts @@ -1,5 +1,4 @@ import type {MutableRefObject} from 'react'; -import type {Transform} from '@dnd-kit/utilities'; import type { Coordinates, @@ -96,7 +95,6 @@ export interface DndContextDescriptor { dragOverlay: { nodeRef: MutableRefObject; rect: ViewRect | null; - transform: MutableRefObject; setRef: (element: HTMLElement | null) => void; }; scrollableAncestors: Element[]; From d7a4949251e1f90eadf4d80a21336807e6404b7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claud=C3=A9ric=20Demers?= Date: Wed, 18 Aug 2021 17:44:24 -0400 Subject: [PATCH 4/4] Add changeset --- .changeset/drag-overlay-measuring.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/drag-overlay-measuring.md diff --git a/.changeset/drag-overlay-measuring.md b/.changeset/drag-overlay-measuring.md new file mode 100644 index 00000000..16a07d53 --- /dev/null +++ b/.changeset/drag-overlay-measuring.md @@ -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.