From 1a916f587f49d35b1ee3c4c1f4fa2ab864803123 Mon Sep 17 00:00:00 2001 From: Mark Tolmacs Date: Sat, 15 Mar 2025 09:30:35 +0100 Subject: [PATCH] Refactor bound elements handling --- .../actions/actionDuplicateSelection.tsx | 293 +----------------- packages/excalidraw/element/duplicate.ts | 63 ++-- 2 files changed, 41 insertions(+), 315 deletions(-) diff --git a/packages/excalidraw/actions/actionDuplicateSelection.tsx b/packages/excalidraw/actions/actionDuplicateSelection.tsx index 32954471b..2d61d1c0b 100644 --- a/packages/excalidraw/actions/actionDuplicateSelection.tsx +++ b/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 { - bindTextToShapeAfterDuplication, - getBoundTextElement, - getContainerElement, -} from "../element/textElement"; -import { - hasBoundTextElement, - isBoundToContainer, - isFrameLikeElement, -} from "../element/typeChecks"; -import { normalizeElementOrder } from "../element/sortElements"; +import { getNonDeletedElements } from "../element"; +import { isBoundToContainer } from "../element/typeChecks"; import { LinearElementEditor } from "../element/linearElementEditor"; -import { - bindElementsToFramesAfterDuplication, - getFrameChildren, -} from "../frame"; -import { - selectGroupsForSelectedElements, - getSelectedGroupForElement, - getElementsInGroup, -} from "../groups"; +import { selectGroupsForSelectedElements } 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 { - arrayToMap, - castArray, - findLastIndex, - getShortcutKey, - invariant, -} from "../utils"; +import { arrayToMap, getShortcutKey } 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> => { - // --------------------------------------------------------------------------- - - const groupIdMap = new Map(); - const newElements: ExcalidrawElement[] = []; - const oldElements: ExcalidrawElement[] = []; - const oldIdToDuplicatedId = new Map(); - const duplicatedElementsMap = new Map(); - - 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(); - - 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, element) => { - if (!isBoundToContainer(element)) { - acc[element.id] = true; - } - return acc; - }, - {}, - ), - }, - getNonDeletedElements(elementsWithClones), - appState, - null, - ), - }, - }; -}; diff --git a/packages/excalidraw/element/duplicate.ts b/packages/excalidraw/element/duplicate.ts index 66eb77292..74cc5d5ae 100644 --- a/packages/excalidraw/element/duplicate.ts +++ b/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[], + newElements: ExcalidrawElement[], oldIdToDuplicatedId: Map, duplicatedElementsMap: NonDeletedSceneElementsMap, ) => { for (const element of newElements) { + if (!isArrowElement(element)) { + continue; + } + if ("boundElements" in element && element.boundElements) { - element.boundElements = element.boundElements.reduce( - ( - acc: Mutable>, - binding, - ) => { - const newBindingId = oldIdToDuplicatedId.get(binding.id); - if (newBindingId) { - acc.push({ ...binding, id: newBindingId }); - } - return acc; - }, - [], - ); + Object.assign(element, { + boundElements: element.boundElements.reduce( + ( + acc: Mutable>, + binding, + ) => { + const newBindingId = oldIdToDuplicatedId.get(binding.id); + if (newBindingId) { + acc.push({ ...binding, id: newBindingId }); + } + return acc; + }, + [], + ), + }); } if ("endBinding" in element && element.endBinding) { const newEndBindingId = oldIdToDuplicatedId.get( element.endBinding.elementId, ); - element.endBinding = newEndBindingId - ? { - ...element.endBinding, - elementId: newEndBindingId, - } - : null; + Object.assign(element, { + endBinding: newEndBindingId + ? { + ...element.endBinding, + elementId: newEndBindingId, + } + : null, + }); } if ("startBinding" in element && element.startBinding) { const newEndBindingId = oldIdToDuplicatedId.get( element.startBinding.elementId, ); - element.startBinding = newEndBindingId - ? { - ...element.startBinding, - elementId: newEndBindingId, - } - : null; + Object.assign(element, { + startBinding: newEndBindingId + ? { + ...element.startBinding, + elementId: newEndBindingId, + } + : null, + }); } if (isElbowArrow(element)) {