fix: navigator dnd bugs (#4546)

- fixed dropping into non-container instances (like image)
- fixed dropping instances from another block

Author: TrySound

apps/builder/app/builder/features/navigator/navigator-tree.tsx

@@ -59,7 +59,7 @@ import {
   getInstanceKey,
   selectInstance,
 } from "~/shared/awareness";
-import { isTreeMatching } from "~/shared/matcher";
+import { findClosestContainer, isTreeMatching } from "~/shared/matcher";
 
 type TreeItemAncestor =
   | undefined
@@ -465,20 +465,35 @@ const canDrop = (
 ) => {
   const dropSelector = dropTarget.itemSelector;
   const instances = $instances.get();
-
-  // in content mode allow drop only inside of block
+  const metas = $registeredComponentMetas.get();
+  // in content mode allow drop only within same block
   if ($isContentMode.get()) {
     const parentInstance = instances.get(dropSelector[0]);
     if (parentInstance?.component !== blockComponent) {
       return false;
     }
+    // parent of dragging item should be the same as drop target
+    if (dropSelector[0] !== dragSelector[1]) {
+      return false;
+    }
+  }
+  // prevent dropping into non-container instances
+  const closestContainerIndex = findClosestContainer({
+    metas,
+    instances,
+    instanceSelector: dropSelector,
+  });
+  if (closestContainerIndex !== 0) {
+    return false;
   }
-
   return isTreeMatching({
     instances,
-    metas: $registeredComponentMetas.get(),
+    metas,
     // make sure dragging tree can be put inside of drop instance
-    instanceSelector: [dragSelector[0], ...dropSelector],
+    instanceSelector: [
+      dragSelector[0],
+      ...dropSelector.slice(closestContainerIndex),
+    ],
   });
 };
 

apps/builder/app/shared/instance-utils.ts

@@ -64,7 +64,7 @@ import { setDifference, setUnion } from "./shim";
 import { breakCyclesMutable, findCycles } from "@webstudio-is/project-build";
 import { $awareness, $selectedPage, selectInstance } from "./awareness";
 import {
-  findClosestContainer,
+  findClosestNonTextualContainer,
   findClosestInstanceMatchingFragment,
   isTreeMatching,
 } from "./matcher";
@@ -1436,7 +1436,7 @@ export const findClosestInsertable = (
   }
   const metas = $registeredComponentMetas.get();
   const instances = $instances.get();
-  const closestContainerIndex = findClosestContainer({
+  const closestContainerIndex = findClosestNonTextualContainer({
     metas,
     instances,
     instanceSelector,

apps/builder/app/shared/matcher.test.tsx

@@ -5,10 +5,11 @@ import * as baseMetas from "@webstudio-is/sdk-components-react/metas";
 import type { WsComponentMeta } from "@webstudio-is/react-sdk";
 import type { Matcher, WebstudioFragment } from "@webstudio-is/sdk";
 import {
-  findClosestContainer,
+  findClosestNonTextualContainer,
   findClosestInstanceMatchingFragment,
   isInstanceMatching,
   isTreeMatching,
+  findClosestContainer,
 } from "./matcher";
 
 const metas = new Map(Object.entries({ ...coreMetas, ...baseMetas }));
@@ -885,7 +886,7 @@ describe("find closest container", () => {
     ).toEqual(1);
   });
 
-  test("skips containers with text", () => {
+  test("allow containers with text", () => {
     expect(
       findClosestContainer({
         ...renderJsx(
@@ -898,12 +899,74 @@ describe("find closest container", () => {
         metas,
         instanceSelector: ["box-with-text", "box", "body"],
       })
+    ).toEqual(0);
+  });
+
+  test("allow containers with expression", () => {
+    expect(
+      findClosestContainer({
+        ...renderJsx(
+          <$.Body ws:id="body">
+            <$.Box ws:id="box">
+              <$.Box ws:id="box-with-expr">
+                {new ExpressionValue("1 + 1")}
+              </$.Box>
+            </$.Box>
+          </$.Body>
+        ),
+        metas,
+        instanceSelector: ["box-with-expr", "box", "body"],
+      })
+    ).toEqual(0);
+  });
+
+  test("allow root with text", () => {
+    expect(
+      findClosestContainer({
+        ...renderJsx(<$.Body ws:id="body">text</$.Body>),
+        metas,
+        instanceSelector: ["body"],
+      })
+    ).toEqual(0);
+  });
+});
+
+describe("find closest non textual container", () => {
+  test("skips non-container instances", () => {
+    expect(
+      findClosestNonTextualContainer({
+        ...renderJsx(
+          <$.Body ws:id="body">
+            <$.Box ws:id="box">
+              <$.Image ws:id="image" />
+            </$.Box>
+          </$.Body>
+        ),
+        metas,
+        instanceSelector: ["image", "box", "body"],
+      })
+    ).toEqual(1);
+  });
+
+  test("skips containers with text", () => {
+    expect(
+      findClosestNonTextualContainer({
+        ...renderJsx(
+          <$.Body ws:id="body">
+            <$.Box ws:id="box">
+              <$.Box ws:id="box-with-text">text</$.Box>
+            </$.Box>
+          </$.Body>
+        ),
+        metas,
+        instanceSelector: ["box-with-text", "box", "body"],
+      })
     ).toEqual(1);
   });
 
   test("skips containers with expression", () => {
     expect(
-      findClosestContainer({
+      findClosestNonTextualContainer({
         ...renderJsx(
           <$.Body ws:id="body">
             <$.Box ws:id="box">
@@ -921,7 +984,7 @@ describe("find closest container", () => {
 
   test("allow root with text", () => {
     expect(
-      findClosestContainer({
+      findClosestNonTextualContainer({
         ...renderJsx(<$.Body ws:id="body">text</$.Body>),
         metas,
         instanceSelector: ["body"],

apps/builder/app/shared/matcher.ts

@@ -301,7 +301,31 @@ export const findClosestContainer = ({
       continue;
     }
     const meta = metas.get(instance.component);
-    if (meta === undefined) {
+    if (meta?.type === "container") {
+      return index;
+    }
+  }
+  return -1;
+};
+
+export const findClosestNonTextualContainer = ({
+  metas,
+  instances,
+  instanceSelector,
+}: {
+  metas: Map<string, WsComponentMeta>;
+  instances: Instances;
+  instanceSelector: InstanceSelector;
+}) => {
+  // page root with text can be used as container
+  if (instanceSelector.length === 1) {
+    return 0;
+  }
+  for (let index = 0; index < instanceSelector.length; index += 1) {
+    const instanceId = instanceSelector[index];
+    const instance = instances.get(instanceId);
+    // collection item can be undefined
+    if (instance === undefined) {
       continue;
     }
     let hasText = false;
@@ -313,7 +337,8 @@ export const findClosestContainer = ({
     if (hasText) {
       continue;
     }
-    if (meta.type === "container") {
+    const meta = metas.get(instance.component);
+    if (meta?.type === "container") {
       return index;
     }
   }

packages/react-sdk/src/core-components.ts

@@ -151,7 +151,7 @@ const blockMeta: WsComponentMeta = {
   icon: EditIcon,
   constraints: {
     relation: "ancestor",
-    component: { $neq: collectionComponent },
+    component: { $nin: [collectionComponent, blockComponent] },
   },
   stylable: false,
   template: [