Merge pull request #749 from clauderic/improve-keyboard-sorting-strategy

Improve default keyboard sorting strategy

.changeset/accessibility-related-props.md

@@ -2,6 +2,8 @@
 '@dnd-kit/core': major
 ---
 
+Accessibility related changes.
+
 #### Regrouping accessibility-related props
 
 Accessibility-related props have been regrouped under the `accessibility` prop of `<DndContext>`:

.changeset/export-data-types.md

@@ -0,0 +1,5 @@
+---
+'@dnd-kit/core': patch
+---
+
+The `Data` and `DataRef` types are now exported by `@dnd-kit/core`.

.changeset/expose-activator-event.md

@@ -0,0 +1,5 @@
+---
+'@dnd-kit/core': minor
+---
+
+The `onDragStart`, `onDragMove`, `onDragOver`, `onDragEnd` and `onDragCancel` events of `<DndContext>` and `useDndMonitor` now expose the `activatorEvent` event that instantiated the activated sensor.

.changeset/sortable-keyboard-coordinates.md

@@ -2,6 +2,14 @@
 '@dnd-kit/sortable': major
 ---
 
+Changes to the default `sortableKeyboardCoordinates` KeyboardSensor coordinate getter.
+
+#### Better handling of variable sizes
+
+The default `sortableKeyboardCoordinates` function now has better handling of lists that have items of variable sizes. We recommend that consumers re-order lists `onDragOver` instead of `onDragEnd` when sorting lists of variable sizes via the keyboard for optimal compatibility.
+
+#### Better handling of overlapping droppables
+
 The default `sortableKeyboardCoordinates` function that is exported from the `@dnd-kit/sortable` package has been updated to better handle cases where the collision rectangle is overlapping droppable rectangles. For example, for `down` arrow key, the default function had logic that would only consider collisions against droppables that were below the `bottom` edge of the collision rect. This was problematic when the collision rect was overlapping droppable rects, because it meant that it's bottom edge was below the top edge of the droppable, and that resulted in that droppable being skipped.
 
 ```diff

packages/core/src/components/DndContext/DndContext.tsx

@@ -217,6 +217,7 @@ export const DndContext = memo(function DndContext({
     activeNode ? activeNode.parentElement : null
   );
   const sensorContext = useRef<SensorContext>({
+    activatorEvent: null,
     active: null,
     activeNode,
     collisionRect: null,
@@ -338,10 +339,12 @@ export const DndContext = memo(function DndContext({
         return;
       }
 
+      const activatorEvent = event.nativeEvent;
+
       const sensorInstance = new Sensor({
         active: activeRef.current,
         activeNode,
-        event: event.nativeEvent,
+        event: activatorEvent,
         options,
         // Sensors need to be instantiated with refs for arguments that change over time
         // otherwise they are frozen in time with the stale arguments
@@ -404,6 +407,7 @@ export const DndContext = memo(function DndContext({
             const {cancelDrop} = latestProps.current;
 
             event = {
+              activatorEvent,
               active: active,
               collisions,
               delta: scrollAdjustedTranslate,
@@ -506,14 +510,15 @@ export const DndContext = memo(function DndContext({
   useEffect(
     () => {
       const {onDragMove} = latestProps.current;
-      const {active, collisions, over} = sensorContext.current;
+      const {active, activatorEvent, collisions, over} = sensorContext.current;
 
-      if (!active) {
+      if (!active || !activatorEvent) {
         return;
       }
 
       const event: DragMoveEvent = {
         active,
+        activatorEvent,
         collisions,
         delta: {
           x: scrollAdjustedTranslate.x,
@@ -533,12 +538,18 @@ export const DndContext = memo(function DndContext({
     () => {
       const {
         active,
+        activatorEvent,
         collisions,
         droppableContainers,
         scrollAdjustedTranslate,
       } = sensorContext.current;
 
-      if (!active || !activeRef.current || !scrollAdjustedTranslate) {
+      if (
+        !active ||
+        !activeRef.current ||
+        !activatorEvent ||
+        !scrollAdjustedTranslate
+      ) {
         return;
       }
 
@@ -555,6 +566,7 @@ export const DndContext = memo(function DndContext({
           : null;
       const event: DragOverEvent = {
         active,
+        activatorEvent,
         collisions,
         delta: {
           x: scrollAdjustedTranslate.x,
@@ -575,6 +587,7 @@ export const DndContext = memo(function DndContext({
 
   useIsomorphicLayoutEffect(() => {
     sensorContext.current = {
+      activatorEvent,
       active,
       activeNode,
       collisionRect,

packages/core/src/index.ts

@@ -80,6 +80,8 @@ export type {
 
 export type {
   Active,
+  Data,
+  DataRef,
   PublicContextDescriptor as DndContextDescriptor,
   DraggableNode,
   DroppableContainers,

packages/core/src/sensors/types.ts

@@ -23,6 +23,7 @@ export enum Response {
 }
 
 export type SensorContext = {
+  activatorEvent: Event | null;
   active: Active | null;
   activeNode: HTMLElement | null;
   collisionRect: ClientRect | null;

packages/core/src/store/types.ts

@@ -17,9 +17,11 @@ export interface DraggableElement {
   disabled: boolean;
 }
 
-export type Data = Record<string, any>;
+type AnyData = Record<string, any>;
 
-export type DataRef = MutableRefObject<Data | undefined>;
+export type Data<T = AnyData> = T & AnyData;
+
+export type DataRef<T = AnyData> = MutableRefObject<Data<T> | undefined>;
 
 export interface DroppableContainer {
   id: UniqueIdentifier;

packages/core/src/types/events.ts

@@ -4,6 +4,7 @@ import type {Collision} from '../utilities/algorithms';
 import type {Translate} from './coordinates';
 
 interface DragEvent {
+  activatorEvent: Event;
   active: Active;
   collisions: Collision[] | null;
   delta: Translate;

packages/sortable/src/hooks/useSortable.ts

@@ -5,10 +5,11 @@ import {
   UseDraggableArguments,
   UseDroppableArguments,
 } from '@dnd-kit/core';
+import type {Data} from '@dnd-kit/core';
 import {CSS, isKeyboardEvent, useCombinedRefs} from '@dnd-kit/utilities';
 
 import {Context} from '../components';
-import type {SortingStrategy} from '../types';
+import type {SortableData, SortingStrategy} from '../types';
 import {isValidIndex} from '../utilities';
 import {
   defaultAnimateLayoutChanges,
@@ -56,7 +57,7 @@ export function useSortable({
     strategy: globalStrategy,
   } = useContext(Context);
   const index = items.indexOf(id);
-  const data = useMemo(
+  const data = useMemo<SortableData & Data>(
     () => ({sortable: {containerId, index, items}, ...customData}),
     [containerId, customData, index, items]
   );
@@ -168,6 +169,7 @@ export function useSortable({
     active,
     activeIndex,
     attributes,
+    data,
     rect,
     index,
     newIndex,

packages/sortable/src/index.ts

@@ -18,4 +18,5 @@ export {
 } from './strategies';
 export {sortableKeyboardCoordinates} from './sensors';
 export {arrayMove, arraySwap} from './utilities';
-export type {SortingStrategy} from './types';
+export {hasSortableData} from './types';
+export type {SortableData, SortingStrategy} from './types';

packages/sortable/src/sensors/keyboard/sortableKeyboardCoordinates.ts

@@ -6,6 +6,9 @@ import {
   DroppableContainer,
   KeyboardCoordinateGetter,
 } from '@dnd-kit/core';
+import {subtract} from '@dnd-kit/utilities';
+
+import {hasSortableData} from '../../types';
 
 const directions: string[] = [
   KeyboardCode.Down,
@@ -41,7 +44,7 @@ export const sortableKeyboardCoordinates: KeyboardCoordinateGetter = (
         return;
       }
 
-      const rect = entry?.rect.current;
+      const rect = droppableRects.get(entry.id);
 
       if (!rect) {
         return;
@@ -85,33 +88,64 @@ export const sortableKeyboardCoordinates: KeyboardCoordinateGetter = (
     }
 
     if (closestId != null) {
+      const activeDroppable = droppableContainers.get(active.id);
       const newDroppable = droppableContainers.get(closestId);
+      const newRect = newDroppable ? droppableRects.get(newDroppable.id) : null;
       const newNode = newDroppable?.node.current;
-      const newRect = newDroppable?.rect.current;
 
-      if (newNode && newRect) {
+      if (newNode && newRect && activeDroppable && newDroppable) {
         const newScrollAncestors = getScrollableAncestors(newNode);
         const hasDifferentScrollAncestors = newScrollAncestors.some(
           (element, index) => scrollableAncestors[index] !== element
         );
-        const offset = hasDifferentScrollAncestors
-          ? {
-              x: 0,
-              y: 0,
-            }
-          : {
-              x: collisionRect.width - newRect.width,
-              y: collisionRect.height - newRect.height,
-            };
-        const newCoordinates = {
-          x: newRect.left - offset.x,
-          y: newRect.top - offset.y,
+        const hasSameContainer = isSameContainer(activeDroppable, newDroppable);
+        const isAfterActive = isAfter(activeDroppable, newDroppable);
+        const offset =
+          hasDifferentScrollAncestors || !hasSameContainer
+            ? {
+                x: 0,
+                y: 0,
+              }
+            : {
+                x: isAfterActive ? collisionRect.width - newRect.width : 0,
+                y: isAfterActive ? collisionRect.height - newRect.height : 0,
+              };
+        const rectCoordinates = {
+          x: newRect.left,
+          y: newRect.top,
         };
 
+        const newCoordinates =
+          offset.x && offset.y
+            ? rectCoordinates
+            : subtract(rectCoordinates, offset);
+
         return newCoordinates;
       }
     }
   }
 
   return undefined;
 };
+
+function isSameContainer(a: DroppableContainer, b: DroppableContainer) {
+  if (!hasSortableData(a) || !hasSortableData(b)) {
+    return false;
+  }
+
+  return (
+    a.data.current.sortable.containerId === b.data.current.sortable.containerId
+  );
+}
+
+function isAfter(a: DroppableContainer, b: DroppableContainer) {
+  if (!hasSortableData(a) || !hasSortableData(b)) {
+    return false;
+  }
+
+  if (!isSameContainer(a, b)) {
+    return false;
+  }
+
+  return a.data.current.sortable.index < b.data.current.sortable.index;
+}

packages/sortable/src/types/data.ts

@@ -0,0 +1,9 @@
+import type {UniqueIdentifier} from '@dnd-kit/core';
+
+export type SortableData = {
+  sortable: {
+    containerId: UniqueIdentifier;
+    items: UniqueIdentifier[];
+    index: number;
+  };
+};

packages/sortable/src/types/index.ts

@@ -1,10 +1,3 @@
-import type {ClientRect} from '@dnd-kit/core';
-import type {Transform} from '@dnd-kit/utilities';
-
-export type SortingStrategy = (args: {
-  activeNodeRect: ClientRect | null;
-  activeIndex: number;
-  index: number;
-  rects: ClientRect[];
-  overIndex: number;
-}) => Transform | null;
+export type {SortableData} from './data';
+export type {SortingStrategy} from './strategies';
+export {hasSortableData} from './type-guard';

packages/sortable/src/types/strategies.ts

@@ -0,0 +1,10 @@
+import type {ClientRect} from '@dnd-kit/core';
+import type {Transform} from '@dnd-kit/utilities';
+
+export type SortingStrategy = (args: {
+  activeNodeRect: ClientRect | null;
+  activeIndex: number;
+  index: number;
+  rects: ClientRect[];
+  overIndex: number;
+}) => Transform | null;

packages/sortable/src/types/type-guard.ts

@@ -0,0 +1,26 @@
+import type {Data, DroppableContainer, DraggableNode} from '@dnd-kit/core';
+
+import type {SortableData} from './data';
+
+export function hasSortableData<T extends DraggableNode | DroppableContainer>(
+  entry: T | null | undefined
+): entry is T & {data: {current: Data<SortableData>}} {
+  if (!entry) {
+    return false;
+  }
+
+  const data = entry.data.current;
+
+  if (
+    data &&
+    'sortable' in data &&
+    typeof data.sortable === 'object' &&
+    'containerId' in data.sortable &&
+    'items' in data.sortable &&
+    'index' in data.sortable
+  ) {
+    return true;
+  }
+
+  return false;
+}