fixes and cleanup

Author: devongovett

packages/@react-aria/dnd/src/index.ts

@@ -64,6 +64,5 @@ export {useDraggableCollection} from './useDraggableCollection';
 export {useClipboard} from './useClipboard';
 export {DragPreview} from './DragPreview';
 export {ListDropTargetDelegate} from './ListDropTargetDelegate';
-export {TreeDropTargetDelegate} from './TreeDropTargetDelegate';
 export {isVirtualDragging} from './DragManager';
 export {isDirectoryDropItem, isFileDropItem, isTextDropItem} from './utils';

packages/@react-stately/dnd/src/useDroppableCollectionState.ts

@@ -72,14 +72,14 @@ export function useDroppableCollectionState(props: DroppableCollectionStateOptio
 
   let getOppositeTarget = (target: ItemDropTarget): ItemDropTarget | null => {
     if (target.dropPosition === 'before') {
-      let key = collection.getKeyBefore(target.key);
-      return key != null && collection.getItem(key)?.level === collection.getItem(target.key)?.level 
-        ? {type: 'item', key, dropPosition: 'after'} 
+      let node = collection.getItem(target.key);
+      return node && node.prevKey != null
+        ? {type: 'item', key: node.prevKey, dropPosition: 'after'} 
         : null;
     } else if (target.dropPosition === 'after') {
-      let key = collection.getKeyAfter(target.key);
-      return key != null && collection.getItem(key)?.level === collection.getItem(target.key)?.level
-        ? {type: 'item', key, dropPosition: 'before'} 
+      let node = collection.getItem(target.key);
+      return node && node.nextKey != null
+        ? {type: 'item', key: node.nextKey, dropPosition: 'before'} 
         : null;
     }
     return null;

packages/react-aria-components/example/index.css

@@ -13,7 +13,7 @@ html {
   overflow: auto;
 
   :global(.react-aria-DropIndicator) {
-    margin-left: calc(var(--tree-item-level) * 15px);
+    margin-inline-start: calc(var(--tree-item-level) * 15px);
   }
 
   :global(.react-aria-DropIndicator[data-drop-target]) {

packages/react-aria-components/src/Tree.tsx

@@ -21,8 +21,8 @@ import {DragAndDropContext, DropIndicatorContext, useDndPersistedKeys, useRender
 import {DragAndDropHooks} from './useDragAndDrop';
 import {DraggableCollectionState, DroppableCollectionState, Collection as ICollection, Node, SelectionBehavior, TreeState, useTreeState} from 'react-stately';
 import {filterDOMProps, useObjectRef} from '@react-aria/utils';
-import React, {createContext, ForwardedRef, forwardRef, JSX, ReactNode, useContext, useEffect, useMemo, useRef} from 'react';
-import {TreeDropTargetDelegate} from '@react-aria/dnd';
+import React, {createContext, ForwardedRef, forwardRef, JSX, ReactNode, useContext, useEffect, useMemo, useRef, useState} from 'react';
+import {TreeDropTargetDelegate} from './TreeDropTargetDelegate';
 import {useControlledState} from '@react-stately/utils';
 
 class TreeCollection<T> implements ICollection<Node<T>> {
@@ -253,12 +253,15 @@ function TreeInner<T extends object>({props, collection, treeRef: ref}: TreeInne
       : null;
   }
 
+  let [treeDropTargetDelegate] = useState(() => new TreeDropTargetDelegate());
   if (hasDropHooks && dragAndDropHooks) {
     dropState = dragAndDropHooks.useDroppableCollectionState!({
       collection: state.collection,
       selectionManager: state.selectionManager
     });
-    let dropTargetDelegate = dragAndDropHooks.dropTargetDelegate || ctxDropTargetDelegate || new TreeDropTargetDelegate(state, ref, {direction});
+    let dropTargetDelegate = dragAndDropHooks.dropTargetDelegate || ctxDropTargetDelegate || new dragAndDropHooks.ListDropTargetDelegate(state.collection, ref, {direction});
+    treeDropTargetDelegate.setup(dropTargetDelegate, state, direction);
+
     let keyboardDelegate = props.keyboardDelegate ||
       new ListKeyboardDelegate({
         collection: state.collection,
@@ -272,7 +275,7 @@ function TreeInner<T extends object>({props, collection, treeRef: ref}: TreeInne
     droppableCollection = dragAndDropHooks.useDroppableCollection!(
       {
         keyboardDelegate,
-        dropTargetDelegate,
+        dropTargetDelegate: treeDropTargetDelegate,
         onDropActivate: (e) => {
           // Expand collapsed item when dragging over. For keyboard, allow collapsing.
           if (e.target.type === 'item') {

packages/@react-aria/dnd/src/TreeDropTargetDelegate.ts renamed to packages/react-aria-components/src/TreeDropTargetDelegate.ts

@@ -1,13 +1,4 @@
-import {Direction, DropTarget, ItemDropTarget, Key, Node, RefObject} from '@react-types/shared';
-import {ListDropTargetDelegate} from './ListDropTargetDelegate';
-
-interface TreeDropTargetDelegateOptions {
-  /**
-   * The horizontal layout direction.
-   * @default 'ltr'
-   */
-  direction?: Direction
-}
+import {Direction, DropTarget, DropTargetDelegate, ItemDropTarget, Key, Node} from '@react-types/shared';
 
 interface TreeCollection<T> extends Iterable<Node<T>> {
   getItem(key: Key): Node<T> | null,
@@ -36,11 +27,12 @@ interface PointerTracking {
   } | null
 }
 
-const X_SWITCH_THRESHOLD = 3;
-const Y_SWITCH_THRESHOLD = 2;
-
-export class TreeDropTargetDelegate<T> extends ListDropTargetDelegate {
-  private state: TreeState<T>;
+const X_SWITCH_THRESHOLD = 5;
+const Y_SWITCH_THRESHOLD = 5;
+export class TreeDropTargetDelegate<T> {
+  private delegate: DropTargetDelegate | null = null;
+  private state: TreeState<T> | null = null;
+  private direction: Direction = 'ltr';
   private pointerTracking: PointerTracking = {
     lastY: 0,
     lastX: 0,
@@ -49,26 +41,20 @@ export class TreeDropTargetDelegate<T> extends ListDropTargetDelegate {
     boundaryContext: null
   };
 
-  constructor(state: TreeState<T>, ref: RefObject<HTMLElement | null>, options?: TreeDropTargetDelegateOptions) {
-    super(state.collection as Iterable<Node<unknown>>, ref, {
-      direction: options?.direction || 'ltr',
-      orientation: 'vertical',
-      layout: 'stack'
-    });
+  setup(delegate: DropTargetDelegate, state: TreeState<T>, direction: Direction): void {
+    this.delegate = delegate;
     this.state = state;
+    this.direction = direction;
   }
 
-  getDropTargetFromPoint(x: number, y: number, isValidDropTarget: (target: DropTarget) => boolean): DropTarget {
-    let baseTarget = super.getDropTargetFromPoint(x, y, isValidDropTarget);
+  getDropTargetFromPoint(x: number, y: number, isValidDropTarget: (target: DropTarget) => boolean): DropTarget | null {
+    let baseTarget = this.delegate!.getDropTargetFromPoint(x, y, isValidDropTarget);
 
     if (!baseTarget || baseTarget.type === 'root') {
       return baseTarget;
     }
 
-    let target = this.resolveDropTarget(baseTarget, x, y, isValidDropTarget);
-    console.log(target);
-    
-    return target;
+    return this.resolveDropTarget(baseTarget, x, y, isValidDropTarget);    
   }
 
   private resolveDropTarget(
@@ -82,21 +68,33 @@ export class TreeDropTargetDelegate<T> extends ListDropTargetDelegate {
     // Calculate movement directions
     let deltaY = y - tracking.lastY;
     let deltaX = x - tracking.lastX;
-    let currentYMovement: 'up' | 'down' | null = null;
-    let currentXMovement: 'left' | 'right' | null = null;
+    let currentYMovement: 'up' | 'down' | null = tracking.yDirection;
+    let currentXMovement: 'left' | 'right' | null = tracking.xDirection;
 
-    if (Math.abs(deltaY) > 2) {
+    if (Math.abs(deltaY) > Y_SWITCH_THRESHOLD) {
       currentYMovement = deltaY > 0 ? 'down' : 'up';
       tracking.yDirection = currentYMovement;
       tracking.lastY = y;
     }
 
-    if (Math.abs(deltaX) > 2) {
+    if (Math.abs(deltaX) > X_SWITCH_THRESHOLD) {
       currentXMovement = deltaX > 0 ? 'right' : 'left';
       tracking.xDirection = currentXMovement;
       tracking.lastX = x;
     }
 
+    // Normalize to 'after'
+    if (target.dropPosition === 'before') {
+      let keyBefore = this.state!.collection.getKeyBefore(target.key);
+      if (keyBefore != null) {
+        target = {
+          type: 'item',
+          key: keyBefore,
+          dropPosition: 'after'
+        } as const;
+      }
+    }
+
     let potentialTargets = this.getPotentialTargets(target, isValidDropTarget);
 
     if (potentialTargets.length > 1) {
@@ -118,32 +116,20 @@ export class TreeDropTargetDelegate<T> extends ListDropTargetDelegate {
     }
 
     let target = originalTarget;
+    let collection = this.state!.collection;
 
-    // Normalize to 'after'
-    if (originalTarget.dropPosition === 'before') {
-      let keyBefore = this.state.collection.getKeyBefore(originalTarget.key);
-      if (keyBefore == null) {
-        return [originalTarget];
-      }
-      target = {
-        type: 'item',
-        key: keyBefore,
-        dropPosition: 'after'
-      } as const;
-    }
-
-    let currentItem = this.state.collection.getItem(target.key);
+    let currentItem = collection.getItem(target.key);
     while (currentItem && currentItem?.type !== 'item' && currentItem.nextKey != null) {
       target.key = currentItem.nextKey;
-      currentItem = this.state.collection.getItem(currentItem.nextKey);
+      currentItem = collection.getItem(currentItem.nextKey);
     }
 
     let potentialTargets = [target];
 
     // If target has children and is expanded, use "before first child"
-    if (currentItem && currentItem.hasChildNodes && this.state.expandedKeys.has(currentItem.key) && this.state.collection.getChildren) {
+    if (currentItem && currentItem.hasChildNodes && this.state!.expandedKeys.has(currentItem.key) && collection.getChildren) {
       let firstChildItemNode: Node<any> | null = null;
-      for (let child of this.state.collection.getChildren(currentItem.key)) {
+      for (let child of collection.getChildren(currentItem.key)) {
         if (child.type === 'item') {
           firstChildItemNode = child;
           break;
@@ -172,8 +158,8 @@ export class TreeDropTargetDelegate<T> extends ListDropTargetDelegate {
     let ancestorTargets: ItemDropTarget[] = [];
 
     while (parentKey) {
-      let parentItem = this.state.collection.getItem(parentKey);                
-      let nextItem = parentItem?.nextKey ? this.state.collection.getItem(parentItem.nextKey) : null;
+      let parentItem = collection.getItem(parentKey);                
+      let nextItem = parentItem?.nextKey ? collection.getItem(parentItem.nextKey) : null;
       let isLastChildAtLevel = !nextItem || nextItem.parentKey !== parentKey;
 
       if (isLastChildAtLevel) {
@@ -200,8 +186,8 @@ export class TreeDropTargetDelegate<T> extends ListDropTargetDelegate {
 
     // Handle converting "after" to "before next" for non-ambiguous cases
     if (potentialTargets.length === 1) {
-      let nextKey = this.state.collection.getKeyAfter(target.key);
-      let nextNode = nextKey ? this.state.collection.getItem(nextKey) : null;
+      let nextKey = collection.getKeyAfter(target.key);
+      let nextNode = nextKey ? collection.getItem(nextKey) : null;
       if (nextKey != null && nextNode && currentItem && nextNode.level != null && currentItem.level != null && nextNode.level > currentItem.level) {
         let beforeTarget = {
           type: 'item',
@@ -230,67 +216,18 @@ export class TreeDropTargetDelegate<T> extends ListDropTargetDelegate {
     }
 
     let tracking = this.pointerTracking;
-    let currentItem = this.state.collection.getItem(originalTarget.key);
+    let currentItem = this.state!.collection.getItem(originalTarget.key);
     let parentKey = currentItem?.parentKey;
 
     if (!parentKey) {
       return potentialTargets[0];
     }
-
-    // Case 1: Exactly 2 potential targets - use Y movement only
-    if (potentialTargets.length === 2) {
-      // Initialize boundary context if needed
-      if (!tracking.boundaryContext || tracking.boundaryContext.parentKey !== parentKey) {
-        let initialTargetIndex = currentYMovement === 'up' ? 1 : 0;
-        
-        tracking.boundaryContext = {
-          parentKey,
-          lastChildKey: originalTarget.key,
-          preferredTargetIndex: initialTargetIndex,
-          lastSwitchY: y,
-          lastSwitchX: x,
-          entryDirection: tracking.yDirection
-        };
-      }
-      
-      let boundaryContext = tracking.boundaryContext;
-      let distanceFromLastYSwitch = Math.abs(y - boundaryContext.lastSwitchY);
-      
-      // Toggle between targets based on Y movement
-      if (distanceFromLastYSwitch > Y_SWITCH_THRESHOLD && currentYMovement) {
-        let currentIndex = boundaryContext.preferredTargetIndex || 0;
-        
-        if (currentYMovement === 'down' && currentIndex === 0) {
-          // Moving down from inner-most, switch to outer-most
-          boundaryContext.preferredTargetIndex = 1;
-          boundaryContext.lastSwitchY = y;
-        } else if (currentYMovement === 'down' && currentIndex === 1) {
-          // Moving down from outer-most, switch back to inner-most
-          boundaryContext.preferredTargetIndex = 0;
-          boundaryContext.lastSwitchY = y;
-        } else if (currentYMovement === 'up' && currentIndex === 1) {
-          // Moving up from outer-most, switch to inner-most
-          boundaryContext.preferredTargetIndex = 0;
-          boundaryContext.lastSwitchY = y;
-        } else if (currentYMovement === 'up' && currentIndex === 0) {
-          // Moving up from inner-most, switch to outer-most
-          boundaryContext.preferredTargetIndex = 1;
-          boundaryContext.lastSwitchY = y;
-        }
-      }
-      
-      return potentialTargets[boundaryContext.preferredTargetIndex || 0];
-    }
 
-    // Case 2: More than 2 potential targets - use Y for initial target, then X for switching levels
+    // More than 1 potential target - use Y for initial target, then X for switching levels
     // Initialize boundary context if needed
     if (!tracking.boundaryContext || tracking.boundaryContext.parentKey !== parentKey) {
-      let initialTargetIndex = 0; // Default to inner-most
-      if (tracking.yDirection === 'up') {
-        // If entering from below, start with outer-most
-        initialTargetIndex = potentialTargets.length - 1;
-      }
-      
+      // If entering from below, start with outer-most
+      let initialTargetIndex = tracking.yDirection === 'up' ? potentialTargets.length - 1 : 0;      
       tracking.boundaryContext = {
         parentKey,
         lastChildKey: originalTarget.key,
@@ -303,6 +240,23 @@ export class TreeDropTargetDelegate<T> extends ListDropTargetDelegate {
 
     let boundaryContext = tracking.boundaryContext;
     let distanceFromLastXSwitch = Math.abs(x - boundaryContext.lastSwitchX);
+    let distanceFromLastYSwitch = Math.abs(y - boundaryContext.lastSwitchY);
+
+    // Toggle between targets based on Y movement
+    if (distanceFromLastYSwitch > Y_SWITCH_THRESHOLD && currentYMovement) {
+      let currentIndex = boundaryContext.preferredTargetIndex || 0;
+      
+      if (currentYMovement === 'down' && currentIndex === 0) {
+        // Moving down from inner-most, switch to outer-most
+        boundaryContext.preferredTargetIndex = potentialTargets.length - 1;
+      } else if (currentYMovement === 'up' && currentIndex === potentialTargets.length - 1) {
+        // Moving up from outer-most, switch to inner-most
+        boundaryContext.preferredTargetIndex = 0;
+      }
+
+      // Reset x tracking so that moving diagonally doesn't cause flickering.
+      tracking.xDirection = null;
+    }
 
     // X movement controls level selection
     if (distanceFromLastXSwitch > X_SWITCH_THRESHOLD && currentXMovement) {
@@ -337,6 +291,9 @@ export class TreeDropTargetDelegate<T> extends ListDropTargetDelegate {
           }
         }
       }
+
+      // Reset y tracking so that moving diagonally doesn't cause flickering.
+      tracking.yDirection = null;
     }
 
     let targetIndex = Math.max(0, Math.min(boundaryContext.preferredTargetIndex || 0, potentialTargets.length - 1));

packages/react-aria-components/src/useDragAndDrop.tsx

@@ -25,7 +25,6 @@ import {
   DropTarget,
   DropTargetDelegate,
   ListDropTargetDelegate,
-  TreeDropTargetDelegate,
   useDraggableCollection,
   useDraggableItem,
   useDropIndicator,
@@ -62,8 +61,7 @@ interface DropHooks {
   useDropIndicator?: (props: AriaDropIndicatorProps, state: DroppableCollectionState, ref: RefObject<HTMLElement | null>) => DropIndicatorAria,
   renderDropIndicator?: (target: DropTarget) => JSX.Element,
   dropTargetDelegate?: DropTargetDelegate,
-  ListDropTargetDelegate: typeof ListDropTargetDelegate,
-  TreeDropTargetDelegate: typeof TreeDropTargetDelegate
+  ListDropTargetDelegate: typeof ListDropTargetDelegate
 }
 
 export type DragAndDropHooks = DragHooks & DropHooks
@@ -141,7 +139,6 @@ export function useDragAndDrop(options: DragAndDropOptions): DragAndDrop {
       hooks.renderDropIndicator = renderDropIndicator;
       hooks.dropTargetDelegate = dropTargetDelegate;
       hooks.ListDropTargetDelegate = ListDropTargetDelegate;
-      hooks.TreeDropTargetDelegate = TreeDropTargetDelegate;
     }
 
     return hooks;

packages/react-aria-components/stories/Tree.stories.tsx

@@ -582,6 +582,11 @@ function TreeDragAndDropExample(args) {
   let {dragAndDropHooks} = useDragAndDrop({
     getItems,
     getAllowedDropOperations: () => ['move'],
+    renderDragPreview(items) {
+      return (
+        <div style={{background: 'blue', color: 'white', padding: '4px'}}>{items.length} items</div>
+      );
+    },
     shouldAcceptItemDrop: (target) => {
       if (args.shouldAcceptItemDrop === 'folders') {
         let item = treeData.getItem(target.key);

packages/react-aria/src/index.ts

@@ -19,7 +19,7 @@ export {useComboBox} from '@react-aria/combobox';
 export {useDateField, useDatePicker, useDateRangePicker, useDateSegment, useTimeField} from '@react-aria/datepicker';
 export {useDialog} from '@react-aria/dialog';
 export {useDisclosure} from '@react-aria/disclosure';
-export {useDrag, useDrop, useDraggableCollection, useDroppableCollection, useDroppableItem, useDropIndicator, useDraggableItem, useClipboard, DragPreview, ListDropTargetDelegate, TreeDropTargetDelegate, DIRECTORY_DRAG_TYPE, isDirectoryDropItem, isFileDropItem, isTextDropItem} from '@react-aria/dnd';
+export {useDrag, useDrop, useDraggableCollection, useDroppableCollection, useDroppableItem, useDropIndicator, useDraggableItem, useClipboard, DragPreview, ListDropTargetDelegate, DIRECTORY_DRAG_TYPE, isDirectoryDropItem, isFileDropItem, isTextDropItem} from '@react-aria/dnd';
 export {FocusRing, FocusScope, useFocusManager, useFocusRing} from '@react-aria/focus';
 export {I18nProvider, useCollator, useDateFormatter, useFilter, useLocale, useLocalizedStringFormatter, useMessageFormatter, useNumberFormatter} from '@react-aria/i18n';
 export {useFocus, useFocusVisible, useFocusWithin, useHover, useInteractOutside, useKeyboard, useMove, usePress, useLongPress, useFocusable, Pressable, Focusable} from '@react-aria/interactions';