From 07eaefa1fb7d730dd64d71cea1abf82b3ef520bb Mon Sep 17 00:00:00 2001
From: Bogdan Chadkin <opensource@trysound.io>
Date: Sat, 5 Oct 2024 22:40:20 +0300
Subject: [PATCH] fix: find proper drop target in collections (#4198)

Fixes https://github.com/webstudio-is/webstudio/issues/4192
https://github.com/webstudio-is/webstudio/issues/4202

Figured the way we address instances and render tree should be the same
to compute proper drop targets. As a result collection got extra level
of depth.

![Screenshot 2024-10-02 at 13 19
00](https://github.com/user-attachments/assets/9ff5b1ac-7515-41fc-a9f1-f9c77115ecb9)
---
 .../panels/navigator/navigator-tree.tsx       | 59 +++++++++----------
 apps/builder/app/shared/tree-utils.ts         | 16 +++++
 2 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/apps/builder/app/builder/features/sidebar-left/panels/navigator/navigator-tree.tsx b/apps/builder/app/builder/features/sidebar-left/panels/navigator/navigator-tree.tsx
index 8d70ed2abf..746831a7d7 100644
--- a/apps/builder/app/builder/features/sidebar-left/panels/navigator/navigator-tree.tsx
+++ b/apps/builder/app/builder/features/sidebar-left/panels/navigator/navigator-tree.tsx
@@ -5,6 +5,7 @@ import { mergeRefs } from "@react-aria/utils";
 import { useStore } from "@nanostores/react";
 import { isFeatureEnabled } from "@webstudio-is/feature-flags";
 import {
+  Box,
   rawTheme,
   ScrollArea,
   SmallIconButton,
@@ -113,6 +114,9 @@ const $flatTree = computed(
       if (level > 0 && instance.children.some((child) => child.type === "id")) {
         isExpanded = expandedItems.has(selector.join());
       }
+      if (instance.component === "Fragment") {
+        isExpanded = true;
+      }
 
       const treeItem: TreeItem = {
         level,
@@ -124,30 +128,12 @@ const $flatTree = computed(
         isReusable,
       };
       let lastItem = treeItem;
-      if (instance.component === "Fragment") {
-        // slot fragment component is not rendered in navigator tree
-        // so should be always expanded
-        for (let index = 0; index < instance.children.length; index += 1) {
-          const child = instance.children[index];
-          if (child.type === "id") {
-            const isLastChild = index === instance.children.length - 1;
-            lastItem = traverse(
-              child.value,
-              [child.value, ...selector],
-              isHidden,
-              isReusable,
-              isLastChild,
-              // do not increate level because Fragment is not rendered
-              level,
-              index
-            );
-          }
-        }
-        return lastItem;
+      // slot fragment component is not rendered in navigator tree
+      // so should be always expanded
+      if (instance.component !== "Fragment") {
+        flatTree.push(treeItem);
       }
 
-      flatTree.push(treeItem);
-
       // render same children for each collection item in data
       if (instance.component === collectionComponent && isExpanded) {
         const data = propValues?.get("data");
@@ -157,9 +143,7 @@ const $flatTree = computed(
             for (let index = 0; index < instance.children.length; index += 1) {
               const child = instance.children[index];
               if (child.type === "id") {
-                const isLastChild =
-                  index * dataIndex ===
-                  instance.children.length * dataIndex - 1;
+                const isLastChild = index === instance.children.length - 1;
                 lastItem = traverse(
                   child.value,
                   [
@@ -170,16 +154,17 @@ const $flatTree = computed(
                   isHidden,
                   isReusable,
                   isLastChild,
-                  level + 1,
-                  index * dataIndex
+                  // virtual item instance is hidden
+                  // but level is still increased to show proper drop indicator
+                  // and address instance selector
+                  level + 2,
+                  index
                 );
               }
             }
           });
         }
-      }
-
-      if (level === 0 || isExpanded) {
+      } else if (level === 0 || isExpanded) {
         for (let index = 0; index < instance.children.length; index += 1) {
           const child = instance.children[index];
           if (child.type === "id") {
@@ -357,7 +342,17 @@ const getBuilderDropTarget = (
   }
   const instances = $instances.get();
   const parentSelector = selector.slice(-treeDropTarget.parentLevel - 1);
-  const parentInstance = instances.get(parentSelector[0]);
+  let parentInstance = instances.get(parentSelector[0]);
+  const grandParentInstance = instances.get(parentSelector[1]);
+  // collection item fake instance
+  if (parentInstance === undefined && grandParentInstance) {
+    parentInstance = {
+      type: "instance",
+      id: parentSelector[0],
+      component: "Fragment",
+      children: grandParentInstance.children,
+    };
+  }
   if (parentInstance === undefined) {
     return;
   }
@@ -595,6 +590,8 @@ export const NavigatorTree = () => {
           );
         })}
       </TreeRoot>
+      {/* space in the end of scroll area */}
+      <Box css={{ height: theme.spacing[9] }}></Box>
     </ScrollArea>
   );
 };
diff --git a/apps/builder/app/shared/tree-utils.ts b/apps/builder/app/shared/tree-utils.ts
index 038a817e41..a7b220a2b6 100644
--- a/apps/builder/app/shared/tree-utils.ts
+++ b/apps/builder/app/shared/tree-utils.ts
@@ -49,6 +49,21 @@ export type DroppableTarget = {
   position: number | "end";
 };
 
+const getCollectionDropTarget = (
+  instances: Instances,
+  dropTarget: DroppableTarget
+) => {
+  const [parentId, grandparentId] = dropTarget.parentSelector;
+  const parent = instances.get(parentId);
+  const grandparent = instances.get(grandparentId);
+  if (parent === undefined && grandparent?.component === collectionComponent) {
+    return {
+      parentSelector: dropTarget.parentSelector.slice(1),
+      position: dropTarget.position,
+    };
+  }
+};
+
 export const getInstanceOrCreateFragmentIfNecessary = (
   instances: Instances,
   dropTarget: DroppableTarget
@@ -219,6 +234,7 @@ export const getReparentDropTargetMutable = (
     prevParent = grandparentInstance;
   }
 
+  dropTarget = getCollectionDropTarget(instances, dropTarget) ?? dropTarget;
   dropTarget =
     getInstanceOrCreateFragmentIfNecessary(instances, dropTarget) ?? dropTarget;
   dropTarget =