Refactor bound elements handling

packages/excalidraw/actions/actionDuplicateSelection.tsx

@@ -1,28 +1,10 @@
 import { ToolButton } from "../components/ToolButton";
 import { DuplicateIcon } from "../components/icons";
 import { DEFAULT_GRID_SIZE } from "../constants";
-import { duplicateElement, getNonDeletedElements } from "../element";
+import { getNonDeletedElements } from "../element";
-import {
+import { isBoundToContainer } from "../element/typeChecks";
-  bindTextToShapeAfterDuplication,
-  getBoundTextElement,
-  getContainerElement,
-} from "../element/textElement";
-import {
-  hasBoundTextElement,
-  isBoundToContainer,
-  isFrameLikeElement,
-} from "../element/typeChecks";
-import { normalizeElementOrder } from "../element/sortElements";
 import { LinearElementEditor } from "../element/linearElementEditor";
-import {
+import { selectGroupsForSelectedElements } from "../groups";
-  bindElementsToFramesAfterDuplication,
-  getFrameChildren,
-} from "../frame";
-import {
-  selectGroupsForSelectedElements,
-  getSelectedGroupForElement,
-  getElementsInGroup,
-} from "../groups";
 import { t } from "../i18n";
 import { KEYS } from "../keys";
 import { isSomeElementSelected } from "../scene";
@@ -31,13 +13,7 @@ import {
   getSelectedElements,
 } from "../scene/selection";
 import { CaptureUpdateAction } from "../store";
-import {
+import { arrayToMap, getShortcutKey } from "../utils";
-  arrayToMap,
-  castArray,
-  findLastIndex,
-  getShortcutKey,
-  invariant,
-} from "../utils";
 
 import { syncMovedIndices } from "../fractionalIndex";
 
@@ -45,11 +21,8 @@ import { duplicateElements } from "../element/duplicate";
 
 import { register } from "./register";
 
-import type { ActionResult } from "./types";
 import type { ExcalidrawElement } from "../element/types";
 
-import type { AppState } from "../types";
-
 export const actionDuplicateSelection = register({
   name: "duplicateSelection",
   label: "labels.duplicateSelection",
@@ -158,261 +131,3 @@ export const actionDuplicateSelection = register({
     />
   ),
 });
-
-const _duplicateElements = (
-  elements: readonly ExcalidrawElement[],
-  appState: AppState,
-): Partial<Exclude<ActionResult, false>> => {
-  // ---------------------------------------------------------------------------
-
-  const groupIdMap = new Map();
-  const newElements: ExcalidrawElement[] = [];
-  const oldElements: ExcalidrawElement[] = [];
-  const oldIdToDuplicatedId = new Map();
-  const duplicatedElementsMap = new Map<string, ExcalidrawElement>();
-
-  const elementsMap = arrayToMap(elements);
-
-  const duplicateAndOffsetElement = <
-    T extends ExcalidrawElement | ExcalidrawElement[],
-  >(
-    element: T,
-  ): T extends ExcalidrawElement[]
-    ? ExcalidrawElement[]
-    : ExcalidrawElement | null => {
-    const elements = castArray(element);
-
-    const _newElements = elements.reduce(
-      (acc: ExcalidrawElement[], element) => {
-        if (processedIds.has(element.id)) {
-          return acc;
-        }
-
-        processedIds.set(element.id, true);
-
-        const newElement = duplicateElement(
-          appState.editingGroupId,
-          groupIdMap,
-          element,
-          {
-            x: element.x + DEFAULT_GRID_SIZE / 2,
-            y: element.y + DEFAULT_GRID_SIZE / 2,
-          },
-        );
-
-        processedIds.set(newElement.id, true);
-
-        duplicatedElementsMap.set(newElement.id, newElement);
-        oldIdToDuplicatedId.set(element.id, newElement.id);
-
-        oldElements.push(element);
-        newElements.push(newElement);
-
-        acc.push(newElement);
-        return acc;
-      },
-      [],
-    );
-
-    return (
-      Array.isArray(element) ? _newElements : _newElements[0] || null
-    ) as T extends ExcalidrawElement[]
-      ? ExcalidrawElement[]
-      : ExcalidrawElement | null;
-  };
-
-  elements = normalizeElementOrder(elements);
-
-  const idsOfElementsToDuplicate = arrayToMap(
-    getSelectedElements(elements, appState, {
-      includeBoundTextElement: true,
-      includeElementsInFrames: true,
-    }),
-  );
-
-  // Ids of elements that have already been processed so we don't push them
-  // into the array twice if we end up backtracking when retrieving
-  // discontiguous group of elements (can happen due to a bug, or in edge
-  // cases such as a group containing deleted elements which were not selected).
-  //
-  // This is not enough to prevent duplicates, so we do a second loop afterwards
-  // to remove them.
-  //
-  // For convenience we mark even the newly created ones even though we don't
-  // loop over them.
-  const processedIds = new Map<ExcalidrawElement["id"], true>();
-
-  const elementsWithClones: ExcalidrawElement[] = elements.slice();
-
-  const insertAfterIndex = (
-    index: number,
-    elements: ExcalidrawElement | null | ExcalidrawElement[],
-  ) => {
-    invariant(index !== -1, "targetIndex === -1 ");
-
-    if (!Array.isArray(elements) && !elements) {
-      return;
-    }
-
-    elementsWithClones.splice(index + 1, 0, ...castArray(elements));
-  };
-
-  const frameIdsToDuplicate = new Set(
-    elements
-      .filter(
-        (el) => idsOfElementsToDuplicate.has(el.id) && isFrameLikeElement(el),
-      )
-      .map((el) => el.id),
-  );
-
-  for (const element of elements) {
-    if (processedIds.has(element.id)) {
-      continue;
-    }
-
-    if (!idsOfElementsToDuplicate.has(element.id)) {
-      continue;
-    }
-
-    // groups
-    // -------------------------------------------------------------------------
-
-    const groupId = getSelectedGroupForElement(appState, element);
-    if (groupId) {
-      const groupElements = getElementsInGroup(elements, groupId).flatMap(
-        (element) =>
-          isFrameLikeElement(element)
-            ? [...getFrameChildren(elements, element.id), element]
-            : [element],
-      );
-
-      const targetIndex = findLastIndex(elementsWithClones, (el) => {
-        return el.groupIds?.includes(groupId);
-      });
-
-      insertAfterIndex(targetIndex, duplicateAndOffsetElement(groupElements));
-      continue;
-    }
-
-    // frame duplication
-    // -------------------------------------------------------------------------
-
-    if (element.frameId && frameIdsToDuplicate.has(element.frameId)) {
-      continue;
-    }
-
-    if (isFrameLikeElement(element)) {
-      const frameId = element.id;
-
-      const frameChildren = getFrameChildren(elements, frameId);
-
-      const targetIndex = findLastIndex(elementsWithClones, (el) => {
-        return el.frameId === frameId || el.id === frameId;
-      });
-
-      insertAfterIndex(
-        targetIndex,
-        duplicateAndOffsetElement([...frameChildren, element]),
-      );
-      continue;
-    }
-
-    // text container
-    // -------------------------------------------------------------------------
-
-    if (hasBoundTextElement(element)) {
-      const boundTextElement = getBoundTextElement(element, elementsMap);
-
-      const targetIndex = findLastIndex(elementsWithClones, (el) => {
-        return (
-          el.id === element.id ||
-          ("containerId" in el && el.containerId === element.id)
-        );
-      });
-
-      if (boundTextElement) {
-        insertAfterIndex(
-          targetIndex,
-          duplicateAndOffsetElement([element, boundTextElement]),
-        );
-      } else {
-        insertAfterIndex(targetIndex, duplicateAndOffsetElement(element));
-      }
-
-      continue;
-    }
-
-    if (isBoundToContainer(element)) {
-      const container = getContainerElement(element, elementsMap);
-
-      const targetIndex = findLastIndex(elementsWithClones, (el) => {
-        return el.id === element.id || el.id === container?.id;
-      });
-
-      if (container) {
-        insertAfterIndex(
-          targetIndex,
-          duplicateAndOffsetElement([container, element]),
-        );
-      } else {
-        insertAfterIndex(targetIndex, duplicateAndOffsetElement(element));
-      }
-
-      continue;
-    }
-
-    // default duplication (regular elements)
-    // -------------------------------------------------------------------------
-
-    insertAfterIndex(
-      findLastIndex(elementsWithClones, (el) => el.id === element.id),
-      duplicateAndOffsetElement(element),
-    );
-  }
-
-  // ---------------------------------------------------------------------------
-
-  bindTextToShapeAfterDuplication(
-    elementsWithClones,
-    oldElements,
-    oldIdToDuplicatedId,
-  );
-  // fixBindingsAfterDuplication(
-  //   elementsWithClones,
-  //   oldElements,
-  //   oldIdToDuplicatedId,
-  // );
-
-  bindElementsToFramesAfterDuplication(
-    elementsWithClones,
-    oldElements,
-    oldIdToDuplicatedId,
-  );
-
-  const nextElementsToSelect =
-    excludeElementsInFramesFromSelection(newElements);
-
-  return {
-    elements: elementsWithClones,
-    appState: {
-      ...appState,
-      ...selectGroupsForSelectedElements(
-        {
-          editingGroupId: appState.editingGroupId,
-          selectedElementIds: nextElementsToSelect.reduce(
-            (acc: Record<ExcalidrawElement["id"], true>, element) => {
-              if (!isBoundToContainer(element)) {
-                acc[element.id] = true;
-              }
-              return acc;
-            },
-            {},
-          ),
-        },
-        getNonDeletedElements(elementsWithClones),
-        appState,
-        null,
-      ),
-    },
-  };
-};

packages/excalidraw/element/duplicate.ts

@@ -27,6 +27,7 @@ import { bumpVersion } from "./mutateElement";
 
 import {
   hasBoundTextElement,
+  isArrowElement,
   isBoundToContainer,
   isElbowArrow,
   isFrameLikeElement,
@@ -315,48 +316,58 @@ export const duplicateElements = (
   // ---------------------------------------------------------------------------
 
   const fixBindingsAfterDuplication = (
-    newElements: Mutable<ExcalidrawElement>[],
+    newElements: ExcalidrawElement[],
     oldIdToDuplicatedId: Map<ExcalidrawElement["id"], ExcalidrawElement["id"]>,
     duplicatedElementsMap: NonDeletedSceneElementsMap,
   ) => {
     for (const element of newElements) {
+      if (!isArrowElement(element)) {
+        continue;
+      }
+
       if ("boundElements" in element && element.boundElements) {
-        element.boundElements = element.boundElements.reduce(
+        Object.assign(element, {
-          (
+          boundElements: element.boundElements.reduce(
-            acc: Mutable<NonNullable<ExcalidrawElement["boundElements"]>>,
+            (
-            binding,
+              acc: Mutable<NonNullable<ExcalidrawElement["boundElements"]>>,
-          ) => {
+              binding,
-            const newBindingId = oldIdToDuplicatedId.get(binding.id);
+            ) => {
-            if (newBindingId) {
+              const newBindingId = oldIdToDuplicatedId.get(binding.id);
-              acc.push({ ...binding, id: newBindingId });
+              if (newBindingId) {
-            }
+                acc.push({ ...binding, id: newBindingId });
-            return acc;
+              }
-          },
+              return acc;
-          [],
+            },
-        );
+            [],
+          ),
+        });
       }
 
       if ("endBinding" in element && element.endBinding) {
         const newEndBindingId = oldIdToDuplicatedId.get(
           element.endBinding.elementId,
         );
-        element.endBinding = newEndBindingId
+        Object.assign(element, {
-          ? {
+          endBinding: newEndBindingId
-              ...element.endBinding,
+            ? {
-              elementId: newEndBindingId,
+                ...element.endBinding,
-            }
+                elementId: newEndBindingId,
-          : null;
+              }
+            : null,
+        });
       }
       if ("startBinding" in element && element.startBinding) {
         const newEndBindingId = oldIdToDuplicatedId.get(
           element.startBinding.elementId,
         );
-        element.startBinding = newEndBindingId
+        Object.assign(element, {
-          ? {
+          startBinding: newEndBindingId
-              ...element.startBinding,
+            ? {
-              elementId: newEndBindingId,
+                ...element.startBinding,
-            }
+                elementId: newEndBindingId,
-          : null;
+              }
+            : null,
+        });
       }
 
       if (isElbowArrow(element)) {