Find closest rich text with instances index (#1093)

Ref https://github.com/webstudio-is/webstudio-designer/issues/696

Previous findClosestNonInlineParent required additional check to handle
drag of root instance or editing non rich text.

Now we can simplify logic because we know exact component type of
matched instance.

apps/builder/app/canvas/shared/use-drag-drop.ts

@@ -25,7 +25,10 @@ import {
   getInstanceElementById,
   getInstanceIdFromElement,
 } from "~/shared/dom-utils";
-import { getInstanceAncestorsAndSelf } from "~/shared/tree-utils";
+import {
+  findClosestRichTextInstance,
+  getInstanceAncestorsAndSelf,
+} from "~/shared/tree-utils";
 
 declare module "~/shared/pubsub" {
   export interface PubsubMap {
@@ -189,22 +192,11 @@ export const useDragAndDrop = () => {
         return false;
       }
 
-      // When trying to drag an inline instance, drag its parent instead
-      if (getComponentMeta(instance.component)?.type === "rich-text-child") {
-        const nonInlineParent = utils.tree.findClosestNonInlineParent(
-          rootInstance,
-          instance.id
-        );
-        if (
-          nonInlineParent !== undefined &&
-          rootInstance.id !== nonInlineParent.id
-        ) {
-          return nonInlineParent;
-        }
-        return false;
-      }
-
-      return instance;
+      // When trying to drag an instance inside editor, drag the editor instead
+      return (
+        findClosestRichTextInstance(instancesIndexStore.get(), instance.id) ??
+        instance
+      );
     },
     onStart({ data: instance }) {
       state.current.dragItem = instance;

apps/builder/app/canvas/shared/use-track-selected-element.ts

@@ -1,14 +1,13 @@
 import { useEffect, useRef } from "react";
 import { useStore } from "@nanostores/react";
-import type { Instance } from "@webstudio-is/project-build";
-import { getComponentMeta } from "@webstudio-is/react-sdk";
 import {
+  instancesIndexStore,
   selectedInstanceIdStore,
   useRootInstance,
   useTextEditingInstanceId,
 } from "~/shared/nano-states";
-import { utils } from "@webstudio-is/project";
 import { publish } from "~/shared/pubsub";
+import { findClosestRichTextInstance } from "~/shared/tree-utils";
 
 declare module "~/shared/pubsub" {
   export interface PubsubMap {
@@ -54,40 +53,26 @@ export const useTrackSelectedElement = () => {
         element = instanceElement;
       }
 
-      const { id, dataset } = element;
+      const { id } = element;
 
       // Enable clicking inside of content editable without trying to select the element as an instance.
       if (editingInstanceIdRef.current === id) {
         return;
       }
 
-      // It's the second click in a double click.
+      // the second click in a double click.
       if (event.detail === 2) {
-        const component = dataset.wsComponent as Instance["component"];
-        if (component === undefined) {
-          return;
-        }
-        const meta = getComponentMeta(component);
-
-        // When user double clicks on an inline instance, we need to select the parent instance and put it indo text editing mode.
-        // Inline instances are not editable directly, only through parent instance.
-        if (meta?.type === "rich-text-child") {
-          const parent =
-            rootInstance &&
-            utils.tree.findClosestNonInlineParent(rootInstance, id);
-          if (
-            parent &&
-            getComponentMeta(parent.component)?.type === "rich-text"
-          ) {
-            selectedInstanceIdStore.set(parent.id);
-            setEditingInstanceId(parent.id);
-          }
-          return;
-        }
-
-        if (meta?.type === "rich-text") {
-          setEditingInstanceId(id);
+        const instancesIndex = instancesIndexStore.get();
+        // enable text editor when double click on its instance or one of its descendants
+        const richTextInstance = findClosestRichTextInstance(
+          instancesIndex,
+          id
+        );
+        if (richTextInstance) {
+          selectedInstanceIdStore.set(richTextInstance.id);
+          setEditingInstanceId(richTextInstance.id);
         }
+        return;
       }
 
       selectedInstanceIdStore.set(id);

apps/builder/app/shared/tree-utils.test.ts

@@ -15,6 +15,7 @@ import {
   cloneStyleSourceSelections,
   createInstancesIndex,
   findClosestDroppableTarget,
+  findClosestRichTextInstance,
   findSubtree,
   findSubtreeLocalStyleSources,
   getInstanceAncestorsAndSelf,
@@ -282,6 +283,30 @@ test("get path from instance and its ancestors", () => {
   ]);
 });
 
+test("find closest rich text to instance", () => {
+  const rootInstance: Instance = createInstance("root", "Box", [
+    createInstance("box1", "Box", []),
+    createInstance("paragraph2", "Paragraph", [
+      createInstance("italic3", "Italic", [
+        createInstance("bold4", "Bold", []),
+        createInstance("box5", "Box", []),
+      ]),
+    ]),
+    createInstance("box6", "Box", []),
+  ]);
+  const instancesIndex = createInstancesIndex(rootInstance);
+
+  expect(findClosestRichTextInstance(instancesIndex, "bold4")?.id).toEqual(
+    "paragraph2"
+  );
+  expect(findClosestRichTextInstance(instancesIndex, "paragraph2")?.id).toEqual(
+    "paragraph2"
+  );
+  expect(findClosestRichTextInstance(instancesIndex, "box6")?.id).toEqual(
+    undefined
+  );
+});
+
 test("find subtree with all descendants and parent instance", () => {
   const rootInstance: Instance = createInstance("root", "Box", [
     createInstance("box1", "Box", []),

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

@@ -178,6 +178,17 @@ export const getInstanceAncestorsAndSelf = (
   return path;
 };
 
+export const findClosestRichTextInstance = (
+  instancesIndex: InstancesIndex,
+  instanceId: Instance["id"]
+) => {
+  return getInstanceAncestorsAndSelf(instancesIndex, instanceId)
+    .reverse()
+    .find(
+      (instance) => getComponentMeta(instance.component)?.type === "rich-text"
+    );
+};
+
 export const findSubtree = (
   rootInstance: Instance,
   targetInstanceId: Instance["id"]

packages/project/src/shared/tree-utils/find-closest-non-inline-parent.test.ts

@@ -1,52 +0,0 @@
-import { describe, test, expect } from "@jest/globals";
-import { createInstance } from "./create-instance";
-import { findClosestNonInlineParent } from "./find-closest-non-inline-parent";
-
-describe("findClosestNonInlineParent", () => {
-  const italic = createInstance({ component: "Italic" });
-
-  const boldWithItalic = createInstance({
-    component: "Bold",
-    children: [italic],
-  });
-
-  test("when there's no non-inline parent, returns undefined", () => {
-    expect(
-      findClosestNonInlineParent(boldWithItalic, italic.id)
-    ).toBeUndefined();
-    expect(findClosestNonInlineParent(italic, italic.id)).toBeUndefined();
-  });
-
-  test("when the given instance isn't in the tree, returns undefined", () => {
-    const root = createInstance({ component: "Body" });
-    expect(findClosestNonInlineParent(root, italic.id)).toBeUndefined();
-  });
-
-  test("retrunrns the first non-inline parent", () => {
-    const root = createInstance({
-      component: "Body",
-      children: [
-        createInstance({
-          component: "Heading",
-          id: "the-heading",
-          children: [italic],
-        }),
-      ],
-    });
-    expect(findClosestNonInlineParent(root, italic.id)?.id).toBe("the-heading");
-  });
-
-  test("retrunrns the first non-inline parent (nested)", () => {
-    const root = createInstance({
-      component: "Body",
-      children: [
-        createInstance({
-          component: "Heading",
-          id: "the-heading",
-          children: [boldWithItalic],
-        }),
-      ],
-    });
-    expect(findClosestNonInlineParent(root, italic.id)?.id).toBe("the-heading");
-  });
-});

packages/project/src/shared/tree-utils/find-closest-non-inline-parent.ts

@@ -1,15 +0,0 @@
-import type { Instance } from "@webstudio-is/project-build";
-import { getComponentMeta } from "@webstudio-is/react-sdk";
-import { getInstancePath } from "./get-instance-path";
-
-export const findClosestNonInlineParent = (
-  rootInstance: Instance,
-  instanceId: Instance["id"]
-): Instance | undefined => {
-  const path = getInstancePath(rootInstance, instanceId);
-  path.reverse();
-  return path.find((item) => {
-    const meta = getComponentMeta(item.component);
-    return meta?.type !== "rich-text-child";
-  });
-};

packages/project/src/shared/tree-utils/find-instance.ts

@@ -1,23 +0,0 @@
-import type { Instance } from "@webstudio-is/project-build";
-
-// @todo should be optimized because it will be a frequent operation on a potentially large tree.
-export const findInstanceById = (
-  instance: Instance,
-  id: Instance["id"]
-): Instance | undefined => {
-  if (instance.id === id) {
-    return instance;
-  }
-
-  if (instance.children !== undefined) {
-    for (const child of instance.children) {
-      if (child.type === "text") {
-        continue;
-      }
-      const foundInstance = findInstanceById(child, id);
-      if (foundInstance !== undefined) {
-        return foundInstance;
-      }
-    }
-  }
-};

packages/project/src/shared/tree-utils/get-instance-path.test.ts

@@ -1,78 +1,6 @@
 import { describe, test, expect } from "@jest/globals";
 import type { Instance } from "@webstudio-is/project-build";
-import {
-  getInstancePath,
-  getInstancePathWithPositions,
-} from "./get-instance-path";
-
-const getIds = (array: Array<{ id: string }>) => array.map((item) => item.id);
-
-describe("Get instance path", () => {
-  test("single possible path", () => {
-    const instance: Instance = {
-      type: "instance",
-      component: "Box",
-      id: "1",
-      children: [
-        {
-          type: "instance",
-          component: "Box",
-          id: "2",
-          children: [
-            {
-              type: "instance",
-              component: "Box",
-              id: "3",
-              children: [],
-            },
-          ],
-        },
-      ],
-    };
-    const path = getInstancePath(instance, "3");
-    expect(getIds(path)).toEqual(["1", "2", "3"]);
-  });
-
-  test("two possible paths", () => {
-    const instance: Instance = {
-      type: "instance",
-      component: "Box",
-      id: "1",
-      children: [
-        {
-          type: "instance",
-          component: "Box",
-          id: "2a",
-          children: [
-            {
-              type: "instance",
-              component: "Box",
-              id: "3a",
-              children: [],
-            },
-          ],
-        },
-        {
-          type: "instance",
-          component: "Box",
-          id: "2b",
-          children: [
-            {
-              type: "instance",
-              component: "Box",
-              id: "3b",
-              children: [],
-            },
-          ],
-        },
-      ],
-    };
-    const path1 = getInstancePath(instance, "3b");
-    expect(getIds(path1)).toEqual(["1", "2b", "3b"]);
-    const path2 = getInstancePath(instance, "3a");
-    expect(getIds(path2)).toEqual(["1", "2a", "3a"]);
-  });
-});
+import { getInstancePathWithPositions } from "./get-instance-path";
 
 const getIdAndPosition = (
   array: Array<{ item: { id: string }; position: number }>

packages/project/src/shared/tree-utils/get-instance-path.ts

@@ -1,34 +1,5 @@
 import type { Instance } from "@webstudio-is/project-build";
 
-export const getInstancePath = (
-  instance: Instance,
-  instanceId: Instance["id"]
-) => {
-  const path = [];
-
-  const find = (instance: Instance) => {
-    if (instance.id === instanceId) {
-      return true;
-    }
-    for (const child of instance.children) {
-      if (child.type === "text") {
-        continue;
-      }
-      const found = find(child);
-      if (found) {
-        path.push(child);
-        return true;
-      }
-    }
-  };
-
-  if (find(instance)) {
-    path.push(instance);
-  }
-
-  return path.reverse();
-};
-
 export const getInstancePathWithPositions = (
   instance: Instance,
   instanceId: Instance["id"]

packages/project/src/shared/tree-utils/index.ts

@@ -1,5 +1,2 @@
 export * from "./create-instance";
-export * from "./find-instance";
 export * from "./get-instance-path";
-export * from "./find-closest-non-inline-parent";
-export * from "./find-instance";