fix: dropping on self-parent through keyboard interaction (#363)

Author: lukasbach

next-release-notes.md

@@ -1,2 +1,3 @@
 ### Bug Fixes and Improvements
-- Fixes an issue where not providing a submit button ref in a custom rename input renderer would prevent dismissing the input on blur (#368)
+- Fixes an issue where not providing a submit button ref in a custom rename input renderer would prevent dismissing the input on blur (#368)
+- Fixes an issue where dropping an item on its children was possible through keyboard-based dragging (#363)

packages/core/src/drag/DraggingPositionEvaluation.ts

@@ -298,7 +298,7 @@ export class DraggingPositionEvaluation {
       treeId
     );
 
-    if (potentialParents.find(p => p.index === parent.item)) {
+    if (potentialParents.some(p => p.index === parent.item)) {
       return true;
     }
 

packages/core/src/drag/useCanDropAt.ts

@@ -20,7 +20,10 @@ export const useCanDropAt = () => {
         if (
           !resolvedItem ||
           (!environment.canDropOnFolder && resolvedItem.isFolder) ||
-          (!environment.canDropOnNonFolder && !resolvedItem.isFolder)
+          (!environment.canDropOnNonFolder && !resolvedItem.isFolder) ||
+          draggingItems.some(
+            draggingItem => draggingItem.index === draggingPosition.targetItem
+          )
         ) {
           return false;
         }

packages/core/src/drag/useGetViableDragPositions.ts

@@ -1,3 +1,4 @@
+/* eslint-disable no-continue */
 import { useCallback } from 'react';
 import { DraggingPosition, TreeItem } from '../types';
 import { useGetGetParentOfLinearItem } from './useGetParentOfLinearItem';
@@ -9,54 +10,96 @@ export const useGetViableDragPositions = () => {
   const getParentOfLinearItem = useGetGetParentOfLinearItem();
   const canDropAt = useCanDropAt();
 
+  const isDescendant = useCallback(
+    (treeId: string, itemLinearIndex: number, potentialParents: TreeItem[]) => {
+      // based on DraggingPositionEvaluation.isDescendant()
+      const { parent, parentLinearIndex } = getParentOfLinearItem(
+        itemLinearIndex,
+        treeId
+      );
+      if (potentialParents.some(p => p.index === parent.item)) return true;
+      if (parent.depth === 0) return false;
+      return isDescendant(treeId, parentLinearIndex, potentialParents);
+    },
+    [getParentOfLinearItem]
+  );
+
   return useCallback(
     (treeId: string, draggingItems: TreeItem[]) => {
       const linearItems = environment.linearItems[treeId];
-      return linearItems
-        .map<DraggingPosition[]>(({ item, depth }, linearIndex) => {
-          const { parent } = getParentOfLinearItem(linearIndex, treeId);
-          const childIndex =
-            environment.items[parent.item].children!.indexOf(item);
+      const targets: DraggingPosition[] = [];
+      let skipUntilDepthIsLowerThan = -1;
+
+      for (
+        let linearIndex = 0;
+        linearIndex < linearItems.length;
+        // eslint-disable-next-line no-plusplus
+        linearIndex++
+      ) {
+        const { item, depth } = linearItems[linearIndex];
+
+        if (
+          skipUntilDepthIsLowerThan !== -1 &&
+          depth > skipUntilDepthIsLowerThan
+        ) {
+          continue;
+        } else {
+          skipUntilDepthIsLowerThan = -1;
+        }
+
+        const { parent } = getParentOfLinearItem(linearIndex, treeId);
+        const childIndex =
+          environment.items[parent.item].children!.indexOf(item);
+
+        if (isDescendant(treeId, linearIndex, draggingItems)) {
+          skipUntilDepthIsLowerThan = depth + 1;
+          continue;
+        }
+
+        const itemPosition: DraggingPosition = {
+          targetType: 'item',
+          parentItem: parent.item,
+          targetItem: item,
+          linearIndex,
+          depth,
+          treeId,
+        };
 
-          const itemPosition: DraggingPosition = {
-            targetType: 'item',
-            parentItem: parent.item,
-            targetItem: item,
-            linearIndex,
-            depth,
-            treeId,
-          };
+        const topPosition: DraggingPosition = {
+          targetType: 'between-items',
+          parentItem: parent.item,
+          linePosition: 'top',
+          childIndex,
+          depth,
+          treeId,
+          linearIndex,
+        };
 
-          const topPosition: DraggingPosition = {
-            targetType: 'between-items',
-            parentItem: parent.item,
-            linePosition: 'top',
-            childIndex,
-            depth,
-            treeId,
-            linearIndex,
-          };
+        const bottomPosition: DraggingPosition = {
+          targetType: 'between-items',
+          parentItem: parent.item,
+          linePosition: 'bottom',
+          linearIndex: linearIndex + 1,
+          childIndex: childIndex + 1,
+          depth,
+          treeId,
+        };
 
-          const bottomPosition: DraggingPosition = {
-            targetType: 'between-items',
-            parentItem: parent.item,
-            linePosition: 'bottom',
-            linearIndex: linearIndex + 1,
-            childIndex: childIndex + 1,
-            depth,
-            treeId,
-          };
+        const skipTopPosition =
+          depth === (linearItems[linearIndex - 1]?.depth ?? -1);
 
-          const skipTopPosition =
-            depth === (linearItems[linearIndex - 1]?.depth ?? -1);
+        if (!skipTopPosition && canDropAt(topPosition, draggingItems)) {
+          targets.push(topPosition);
+        }
+        if (canDropAt(itemPosition, draggingItems)) {
+          targets.push(itemPosition);
+        }
+        if (canDropAt(bottomPosition, draggingItems)) {
+          targets.push(bottomPosition);
+        }
+      }
 
-          if (skipTopPosition) {
-            return [itemPosition, bottomPosition];
-          }
-          return [topPosition, itemPosition, bottomPosition];
-        })
-        .reduce((a, b) => [...a, ...b], [])
-        .filter(position => canDropAt(position, draggingItems));
+      return targets;
     },
     [
       canDropAt,