From bfeda0ff08baa7ff905277bccfdb2970f5d6ef7b Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Tue, 21 Feb 2023 15:22:52 +0300 Subject: [PATCH] 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. --- .../app/canvas/shared/use-drag-drop.ts | 26 +++---- .../shared/use-track-selected-element.ts | 43 ++++------- apps/builder/app/shared/tree-utils.test.ts | 25 +++++++ apps/builder/app/shared/tree-utils.ts | 11 +++ .../find-closest-non-inline-parent.test.ts | 52 ------------- .../find-closest-non-inline-parent.ts | 15 ---- .../src/shared/tree-utils/find-instance.ts | 23 ------ .../tree-utils/get-instance-path.test.ts | 74 +------------------ .../shared/tree-utils/get-instance-path.ts | 29 -------- .../project/src/shared/tree-utils/index.ts | 3 - 10 files changed, 60 insertions(+), 241 deletions(-) delete mode 100644 packages/project/src/shared/tree-utils/find-closest-non-inline-parent.test.ts delete mode 100644 packages/project/src/shared/tree-utils/find-closest-non-inline-parent.ts delete mode 100644 packages/project/src/shared/tree-utils/find-instance.ts diff --git a/apps/builder/app/canvas/shared/use-drag-drop.ts b/apps/builder/app/canvas/shared/use-drag-drop.ts index 3ccd3d4df8..87f76209e6 100644 --- a/apps/builder/app/canvas/shared/use-drag-drop.ts +++ b/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; diff --git a/apps/builder/app/canvas/shared/use-track-selected-element.ts b/apps/builder/app/canvas/shared/use-track-selected-element.ts index ff88bc1fe3..8e6cdabd9b 100644 --- a/apps/builder/app/canvas/shared/use-track-selected-element.ts +++ b/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); diff --git a/apps/builder/app/shared/tree-utils.test.ts b/apps/builder/app/shared/tree-utils.test.ts index 0ade2aa7e1..76c97fa68b 100644 --- a/apps/builder/app/shared/tree-utils.test.ts +++ b/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", []), diff --git a/apps/builder/app/shared/tree-utils.ts b/apps/builder/app/shared/tree-utils.ts index 4c619e6624..50baee186c 100644 --- a/apps/builder/app/shared/tree-utils.ts +++ b/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"] diff --git a/packages/project/src/shared/tree-utils/find-closest-non-inline-parent.test.ts b/packages/project/src/shared/tree-utils/find-closest-non-inline-parent.test.ts deleted file mode 100644 index 00740aa831..0000000000 --- a/packages/project/src/shared/tree-utils/find-closest-non-inline-parent.test.ts +++ /dev/null @@ -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"); - }); -}); diff --git a/packages/project/src/shared/tree-utils/find-closest-non-inline-parent.ts b/packages/project/src/shared/tree-utils/find-closest-non-inline-parent.ts deleted file mode 100644 index 22342175d4..0000000000 --- a/packages/project/src/shared/tree-utils/find-closest-non-inline-parent.ts +++ /dev/null @@ -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"; - }); -}; diff --git a/packages/project/src/shared/tree-utils/find-instance.ts b/packages/project/src/shared/tree-utils/find-instance.ts deleted file mode 100644 index 916ac078bf..0000000000 --- a/packages/project/src/shared/tree-utils/find-instance.ts +++ /dev/null @@ -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; - } - } - } -}; diff --git a/packages/project/src/shared/tree-utils/get-instance-path.test.ts b/packages/project/src/shared/tree-utils/get-instance-path.test.ts index 41a80c02f8..a48ce7374f 100644 --- a/packages/project/src/shared/tree-utils/get-instance-path.test.ts +++ b/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 }> diff --git a/packages/project/src/shared/tree-utils/get-instance-path.ts b/packages/project/src/shared/tree-utils/get-instance-path.ts index 93bcbb3537..b49b3a2e4c 100644 --- a/packages/project/src/shared/tree-utils/get-instance-path.ts +++ b/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"] diff --git a/packages/project/src/shared/tree-utils/index.ts b/packages/project/src/shared/tree-utils/index.ts index 41effc3fab..c6e6ddd789 100644 --- a/packages/project/src/shared/tree-utils/index.ts +++ b/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";