fix: Tree keyboard drag and drop should skip content nodes (adobe#9724)

Author: devongovett

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

@@ -58,12 +58,7 @@ function nextDropTarget(
     } else {
       nextKey = keyboardDelegate.getKeyBelow?.(target.key);
     }
-    let nextCollectionKey = collection.getKeyAfter(target.key);
-    let nextCollectionNode = nextCollectionKey && collection.getItem(nextCollectionKey);
-    while (nextCollectionNode && nextCollectionNode.type === 'content') {
-      nextCollectionKey = nextCollectionKey ? collection.getKeyAfter(nextCollectionKey) : null;
-      nextCollectionNode = nextCollectionKey && collection.getItem(nextCollectionKey);
-    }
+    let nextCollectionKey = getNextItem(collection, target.key, key => collection.getKeyAfter(key));
 
     // If the keyboard delegate did not move to the next key in the collection,
     // jump to that key with the same drop position. Otherwise, try the other
@@ -191,7 +186,7 @@ function previousDropTarget(
     } else {
       prevKey = keyboardDelegate.getKeyAbove?.(target.key);
     }
-    let prevCollectionKey = collection.getKeyBefore(target.key);
+    let prevCollectionKey = getNextItem(collection, target.key, key => collection.getKeyBefore(key));
 
     // If the keyboard delegate did not move to the next key in the collection,
     // jump to that key with the same drop position. Otherwise, try the other
@@ -263,7 +258,7 @@ function getLastChild(collection: Collection<Node<unknown>>, key: Key): DropTarg
   // getChildNodes still returns child tree items even when the item is collapsed.
   // Checking if the next item has a greater level is a silly way to determine if the item is expanded.
   let targetNode = collection.getItem(key);
-  let nextKey = collection.getKeyAfter(key);
+  let nextKey = getNextItem(collection, key, key => collection.getKeyAfter(key));
   let nextNode = nextKey != null ? collection.getItem(nextKey) : null;
   if (targetNode && nextNode && nextNode.level > targetNode.level) {
     let children = getChildNodes(targetNode, collection);
@@ -285,3 +280,14 @@ function getLastChild(collection: Collection<Node<unknown>>, key: Key): DropTarg
 
   return null;
 }
+
+// Find the next or previous item in a collection, skipping over other types of nodes (e.g. content).
+function getNextItem(collection: Collection<Node<unknown>>, key: Key, getNextKey: (key: Key) => Key | null): Key | null {
+  let nextCollectionKey = getNextKey(key);
+  let nextCollectionNode = nextCollectionKey != null ? collection.getItem(nextCollectionKey) : null;
+  while (nextCollectionNode && nextCollectionNode.type !== 'item') {
+    nextCollectionKey = getNextKey(nextCollectionNode.key);
+    nextCollectionNode = nextCollectionKey != null ? collection.getItem(nextCollectionKey) : null;
+  }
+  return nextCollectionKey;
+}

packages/@react-aria/dnd/test/DropTargetKeyboardNavigation.test.tsx

@@ -48,7 +48,7 @@ class TestCollection implements Collection<Node<Item>> {
   constructor(items: Item[]) {
     this.map = new Map<Key, Node<Item>>();
     let visitItem = (item: Item, index: number, level = 0, parentKey: string | null = null): Node<Item> => {
-      let childNodes = item.childItems ? visitItems(item.childItems, level + 1, item.id) : [];
+      let childNodes = visitItems(item.childItems ?? [], level + 1, item.id);
       return {
         type: 'item',
         key: item.id,
@@ -67,6 +67,24 @@ class TestCollection implements Collection<Node<Item>> {
       let last: Node<Item> | null = null;
       let index = 0;
       let nodes: Node<Item>[] = [];
+      if (parentKey != null) {
+        let contentNode = {
+          type: 'content',
+          key: parentKey + '-content',
+          value: null,
+          level: level,
+          hasChildNodes: false,
+          childNodes: [],
+          rendered: null,
+          textValue: '',
+          index: index++,
+          parentKey
+        };
+        this.map.set(contentNode.key, contentNode);
+        nodes.push(contentNode);
+        last = contentNode;
+      }
+
       for (let item of items) {
         let node = visitItem(item, index, level, parentKey);
         this.map.set(item.id, node);
@@ -236,25 +254,45 @@ describe('drop target keyboard navigation', () => {
 
     let expectedKeys = [
       'projects',
+      'projects-content',
       'project-1',
+      'project-1-content',
       'project-2',
+      'project-2-content',
       'project-2A',
+      'project-2A-content',
       'project-2B',
+      'project-2B-content',
       'project-2C',
+      'project-2C-content',
       'project-3',
+      'project-3-content',
       'project-4',
+      'project-4-content',
       'project-5',
+      'project-5-content',
       'project-5A',
+      'project-5A-content',
       'project-5B',
+      'project-5B-content',
       'project-5C',
+      'project-5C-content',
       'reports',
+      'reports-content',
       'reports-1',
+      'reports-1-content',
       'reports-1A',
+      'reports-1A-content',
       'reports-1AB',
+      'reports-1AB-content',
       'reports-1ABC',
+      'reports-1ABC-content',
       'reports-1B',
+      'reports-1B-content',
       'reports-1C',
-      'reports-2'
+      'reports-1C-content',
+      'reports-2',
+      'reports-2-content'
     ];
 
     expect(nextKeys).toEqual(expectedKeys);