fix: canDrop API is not called when dragging an external node over a …

…tree
minop1205 · Mar 9, 2023 · 8a7033d · 8a7033d
1 parent ec56053
commit 8a7033d
Show file tree

Hide file tree

Showing 12 changed files with 87 additions and 42 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,13 @@
 # Change Log
 
+## 3.4.2
+
+_Mar 9, 2023_
+
+### Fixed
+
+- `canDrop` API is not called when dragging an external node over a tree.
+
 ## 3.4.1
 
 _Dec 31, 2022_

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@minoru/react-dnd-treeview",
   "description": "A draggable / droppable React-based treeview component.",
-  "version": "3.4.1",
+  "version": "3.4.2",
   "license": "MIT",
   "repository": {
     "type": "git",

diff --git a/src/Container.tsx b/src/Container.tsx
@@ -36,11 +36,11 @@ export const Container = <T,>(props: Props): ReactElement => {
     }
   }
 
-  const [isOver, dragSource, drop] = useDropRoot(ref);
+  const [isOver, dragSource, drop] = useDropRoot<T>(ref);
 
   if (
     props.parentId === treeContext.rootId &&
-    isDroppable(dragSource?.id, treeContext.rootId, treeContext)
+    isDroppable<T>(dragSource, treeContext.rootId, treeContext)
   ) {
     drop(ref);
   }

diff --git a/src/Node.tsx b/src/Node.tsx
@@ -30,11 +30,11 @@ export const Node = <T,>(props: Props): ReactElement | null => {
   const open = openIds.includes(props.id);
 
   const [isDragging, drag, preview] = useDragNode(item, containerRef);
-  const [isOver, dragSource, drop] = useDropNode(item, containerRef);
+  const [isOver, dragSource, drop] = useDropNode<T>(item, containerRef);
 
   useDragHandle(containerRef, handleRef, drag);
 
-  if (isDroppable(dragSource?.id, props.id, treeContext)) {
+  if (isDroppable(dragSource, props.id, treeContext)) {
     drop(containerRef);
   }
 

diff --git a/src/hooks/useDropNode.ts b/src/hooks/useDropNode.ts
@@ -9,7 +9,7 @@ import { useTreeContext } from "~/hooks";
 export const useDropNode = <T>(
   item: NodeModel<T>,
   ref: React.RefObject<HTMLElement>
-): [boolean, NodeModel, DragElementWrapper<HTMLElement>] => {
+): [boolean, NodeModel<T>, DragElementWrapper<HTMLElement>] => {
   const treeContext = useTreeContext<T>();
   const placeholderContext = useContext(PlaceholderContext);
   const [{ isOver, dragSource }, drop] = useDrop({
@@ -46,11 +46,7 @@ export const useDropNode = <T>(
           return false;
         }
 
-        return isDroppable(
-          isNodeModel<T>(dragItem) ? dragItem.id : undefined,
-          dropTarget.id,
-          treeContext
-        );
+        return isDroppable(dragItem, dropTarget.id, treeContext);
       }
 
       return false;
@@ -69,11 +65,7 @@ export const useDropNode = <T>(
 
         if (
           dropTarget === null ||
-          !isDroppable(
-            isNodeModel<T>(dragItem) ? dragItem.id : undefined,
-            dropTarget.id,
-            treeContext
-          )
+          !isDroppable(dragItem, dropTarget.id, treeContext)
         ) {
           hidePlaceholder();
           return;

diff --git a/src/hooks/useDropRoot.ts b/src/hooks/useDropRoot.ts
@@ -8,7 +8,7 @@ import { useTreeContext } from "~/hooks";
 
 export const useDropRoot = <T>(
   ref: React.RefObject<HTMLElement>
-): [boolean, NodeModel, DragElementWrapper<HTMLElement>] => {
+): [boolean, NodeModel<T>, DragElementWrapper<HTMLElement>] => {
   const treeContext = useTreeContext<T>();
   const placeholderContext = useContext(PlaceholderContext);
   const [{ isOver, dragSource }, drop] = useDrop({
@@ -37,11 +37,7 @@ export const useDropRoot = <T>(
           return false;
         }
 
-        return isDroppable(
-          isNodeModel<T>(dragItem) ? dragItem.id : undefined,
-          rootId,
-          treeContext
-        );
+        return isDroppable(dragItem, rootId, treeContext);
       }
 
       return false;
@@ -61,11 +57,7 @@ export const useDropRoot = <T>(
 
         if (
           dropTarget === null ||
-          !isDroppable(
-            isNodeModel<T>(dragItem) ? dragItem.id : undefined,
-            rootId,
-            treeContext
-          )
+          !isDroppable(dragItem, rootId, treeContext)
         ) {
           hidePlaceholder();
           return;

diff --git a/src/providers/TreeProvider.tsx b/src/providers/TreeProvider.tsx
@@ -118,7 +118,7 @@ export const TreeProvider = <T,>(props: Props<T>): ReactElement => {
     canDrop: canDropCallback
       ? (dragSourceId, dropTargetId) =>
           canDropCallback(props.tree, {
-            dragSourceId,
+            dragSourceId: dragSourceId ?? undefined,
             dropTargetId,
             dragSource: monitor.getItem(),
             dropTarget: getTreeItem(props.tree, dropTargetId),

diff --git a/src/types.ts b/src/types.ts
@@ -15,6 +15,10 @@ export type DragItem<T> = NodeModel<T> & {
   ref: RefObject<HTMLElement>;
 };
 
+export type NativeDragItem = {
+  dataTransfer: DataTransfer;
+};
+
 export type RenderParams = {
   depth: number;
   isOpen: boolean;
@@ -41,7 +45,7 @@ export type DropHandler<T> = (
 ) => void;
 
 export type CanDropHandler = (
-  dragSourceId: NodeModel["id"],
+  dragSourceId: NodeModel["id"] | null,
   dropTargetId: NodeModel["id"]
 ) => boolean | void;
 

diff --git a/src/utils/getDropTarget.ts b/src/utils/getDropTarget.ts
@@ -119,7 +119,7 @@ export const getDropTarget = <T>(
       };
     }
 
-    if (isDroppable(dragSource.id, node.parent, context)) {
+    if (isDroppable(dragSource, node.parent, context)) {
       const outerIndex = getOuterIndex(node, nodeEl, monitor);
 
       if (outerIndex === null) {
@@ -135,7 +135,7 @@ export const getDropTarget = <T>(
     return null;
   } else {
     if (hoverPosition === "upper") {
-      if (isDroppable(dragSource.id, node.parent, context)) {
+      if (isDroppable(dragSource, node.parent, context)) {
         const outerIndex = getOuterIndex(node, nodeEl, monitor);
 
         if (outerIndex === null) {

diff --git a/src/utils/isDroppable.test.tsx b/src/utils/isDroppable.test.tsx
@@ -1,5 +1,5 @@
 import React from "react";
-import { NodeRender, TreeState } from "~/types";
+import { NodeRender, TreeState, NativeDragItem } from "~/types";
 import { isDroppable } from "./isDroppable";
 import treeData from "../stories/assets/sample-default.json";
 
@@ -27,9 +27,55 @@ describe("isDroppable", () => {
       onToggle: () => undefined,
     };
 
-    expect(isDroppable(7, 7, treeContext)).toBe(false);
-    expect(isDroppable(7, 1, treeContext)).toBe(true);
-    expect(isDroppable(1, 1, treeContext)).toBe(false);
-    expect(isDroppable(4, 5, treeContext)).toBe(false);
+    const nativeDragSource: NativeDragItem = {
+      dataTransfer: {} as DataTransfer,
+    };
+
+    expect(isDroppable(treeData[6], 7, treeContext)).toBe(false);
+    expect(isDroppable(treeData[6], 1, treeContext)).toBe(true);
+    expect(isDroppable(treeData[0], 1, treeContext)).toBe(false);
+    expect(isDroppable(treeData[3], 5, treeContext)).toBe(false);
+    expect(isDroppable(treeData[6], 0, treeContext)).toBe(false);
+    expect(isDroppable(treeData[1], 0, treeContext)).toBe(true);
+    expect(isDroppable(null, 0, treeContext)).toBe(true);
+    expect(isDroppable(null, 1, treeContext)).toBe(true);
+    expect(isDroppable(null, 2, treeContext)).toBe(false);
+    expect(isDroppable(nativeDragSource, 0, treeContext)).toBe(true);
+    expect(isDroppable(nativeDragSource, 1, treeContext)).toBe(true);
+    expect(isDroppable(nativeDragSource, 2, treeContext)).toBe(false);
+
+    treeContext.canDrop = () => {
+      return false;
+    };
+
+    expect(isDroppable(treeData[6], 7, treeContext)).toBe(false);
+    expect(isDroppable(treeData[6], 1, treeContext)).toBe(false);
+    expect(isDroppable(treeData[0], 1, treeContext)).toBe(false);
+    expect(isDroppable(treeData[3], 5, treeContext)).toBe(false);
+    expect(isDroppable(treeData[6], 0, treeContext)).toBe(false);
+    expect(isDroppable(treeData[1], 0, treeContext)).toBe(false);
+    expect(isDroppable(null, 0, treeContext)).toBe(true);
+    expect(isDroppable(null, 1, treeContext)).toBe(true);
+    expect(isDroppable(null, 2, treeContext)).toBe(false);
+    expect(isDroppable(nativeDragSource, 0, treeContext)).toBe(false);
+    expect(isDroppable(nativeDragSource, 1, treeContext)).toBe(false);
+    expect(isDroppable(nativeDragSource, 2, treeContext)).toBe(false);
+
+    treeContext.canDrop = () => {
+      return true;
+    };
+
+    expect(isDroppable(treeData[6], 7, treeContext)).toBe(true);
+    expect(isDroppable(treeData[6], 1, treeContext)).toBe(true);
+    expect(isDroppable(treeData[0], 1, treeContext)).toBe(true);
+    expect(isDroppable(treeData[3], 5, treeContext)).toBe(true);
+    expect(isDroppable(treeData[6], 0, treeContext)).toBe(true);
+    expect(isDroppable(treeData[1], 0, treeContext)).toBe(true);
+    expect(isDroppable(null, 0, treeContext)).toBe(true);
+    expect(isDroppable(null, 1, treeContext)).toBe(true);
+    expect(isDroppable(null, 2, treeContext)).toBe(false);
+    expect(isDroppable(nativeDragSource, 0, treeContext)).toBe(true);
+    expect(isDroppable(nativeDragSource, 1, treeContext)).toBe(true);
+    expect(isDroppable(nativeDragSource, 2, treeContext)).toBe(true);
   });
 });
diff --git a/src/utils/isDroppable.ts b/src/utils/isDroppable.ts
@@ -1,14 +1,15 @@
 import { isAncestor } from "./isAncestor";
-import { NodeModel, TreeState } from "~/types";
+import { NodeModel, NativeDragItem, TreeState } from "~/types";
+import { isNodeModel } from "./isNodeModel";
 
 export const isDroppable = <T>(
-  dragSourceId: NodeModel["id"] | undefined,
+  dragSource: NodeModel<T> | NativeDragItem | null,
   dropTargetId: NodeModel["id"],
   treeContext: TreeState<T>
 ): boolean => {
   const { tree, rootId, canDrop } = treeContext;
 
-  if (dragSourceId === undefined) {
+  if (dragSource === null) {
     // Dropability judgment of each node in the undragged state.
     // Without this process, the newly mounted node will not be able to be dropped unless it is re-rendered
     if (dropTargetId === rootId) {
@@ -23,6 +24,8 @@ export const isDroppable = <T>(
 
     return false;
   } else {
+    const dragSourceId = isNodeModel<T>(dragSource) ? dragSource.id : null;
+
     if (canDrop) {
       const result = canDrop(dragSourceId, dropTargetId);
 
@@ -39,7 +42,7 @@ export const isDroppable = <T>(
     const dropTargetNode = tree.find((node) => node.id === dropTargetId);
 
     // dragSource is external node
-    if (dragSourceNode === undefined) {
+    if (dragSourceNode === undefined || dragSourceId === null) {
       return dropTargetId === rootId || !!dropTargetNode?.droppable;
     }