diff --git a/packages/element/src/binding.ts b/packages/element/src/binding.ts index c84d80577c..7a67cf0a1f 100644 --- a/packages/element/src/binding.ts +++ b/packages/element/src/binding.ts @@ -55,6 +55,7 @@ import { getBoundTextElement, handleBindTextResize } from "./textElement"; import { isArrowElement, isBindableElement, + isBindingElement, isBoundToContainer, isElbowArrow, isFixedPointBinding, @@ -1422,7 +1423,7 @@ const getLinearElementEdgeCoors = ( ); }; -export const fixBindingsAfterDuplication = ( +export const fixDuplicatedBindingsAfterDuplication = ( newElements: ExcalidrawElement[], oldIdToDuplicatedId: Map, duplicatedElementsMap: NonDeletedSceneElementsMap, @@ -1493,6 +1494,196 @@ export const fixBindingsAfterDuplication = ( } }; +const fixReversedBindingsForBindables = ( + original: ExcalidrawBindableElement, + duplicate: ExcalidrawBindableElement, + originalElements: Map, + elementsWithClones: ExcalidrawElement[], + oldIdToDuplicatedId: Map, +) => { + original.boundElements?.forEach((binding, idx) => { + if (binding.type !== "arrow") { + return; + } + + const oldArrow = elementsWithClones.find((el) => el.id === binding.id); + + if (!isBindingElement(oldArrow)) { + return; + } + + if (originalElements.has(binding.id)) { + // Linked arrow is in the selection, so find the duplicate pair + const newArrowId = oldIdToDuplicatedId.get(binding.id) ?? binding.id; + const newArrow = elementsWithClones.find( + (el) => el.id === newArrowId, + )! as ExcalidrawArrowElement; + + mutateElement(newArrow, { + startBinding: + oldArrow.startBinding?.elementId === binding.id + ? { + ...oldArrow.startBinding, + elementId: duplicate.id, + } + : newArrow.startBinding, + endBinding: + oldArrow.endBinding?.elementId === binding.id + ? { + ...oldArrow.endBinding, + elementId: duplicate.id, + } + : newArrow.endBinding, + }); + mutateElement(duplicate, { + boundElements: [ + ...(duplicate.boundElements ?? []).filter( + (el) => el.id !== binding.id && el.id !== newArrowId, + ), + { + type: "arrow", + id: newArrowId, + }, + ], + }); + } else { + // Linked arrow is outside the selection, + // so we move the binding to the duplicate + mutateElement(oldArrow, { + startBinding: + oldArrow.startBinding?.elementId === original.id + ? { + ...oldArrow.startBinding, + elementId: duplicate.id, + } + : oldArrow.startBinding, + endBinding: + oldArrow.endBinding?.elementId === original.id + ? { + ...oldArrow.endBinding, + elementId: duplicate.id, + } + : oldArrow.endBinding, + }); + mutateElement(duplicate, { + boundElements: [ + ...(duplicate.boundElements ?? []), + { + type: "arrow", + id: oldArrow.id, + }, + ], + }); + mutateElement(original, { + boundElements: + original.boundElements?.filter((_, i) => i !== idx) ?? null, + }); + } + }); +}; + +const fixReversedBindingsForArrows = ( + original: ExcalidrawArrowElement, + duplicate: ExcalidrawArrowElement, + originalElements: Map, + bindingProp: "startBinding" | "endBinding", + oldIdToDuplicatedId: Map, + elementsWithClones: ExcalidrawElement[], +) => { + const oldBindableId = original[bindingProp]?.elementId; + + if (oldBindableId) { + if (originalElements.has(oldBindableId)) { + // Linked element is in the selection + const newBindableId = + oldIdToDuplicatedId.get(oldBindableId) ?? oldBindableId; + const newBindable = elementsWithClones.find( + (el) => el.id === newBindableId, + ) as ExcalidrawBindableElement; + mutateElement(duplicate, { + [bindingProp]: { + ...original[bindingProp], + elementId: newBindableId, + }, + }); + mutateElement(newBindable, { + boundElements: [ + ...(newBindable.boundElements ?? []).filter( + (el) => el.id !== original.id && el.id !== duplicate.id, + ), + { + id: duplicate.id, + type: "arrow", + }, + ], + }); + } else { + // Linked element is outside the selection + const originalBindable = elementsWithClones.find( + (el) => el.id === oldBindableId, + ); + if (originalBindable) { + mutateElement(duplicate, { + [bindingProp]: original[bindingProp], + }); + mutateElement(original, { + [bindingProp]: null, + }); + mutateElement(originalBindable, { + boundElements: [ + ...(originalBindable.boundElements?.filter( + (el) => el.id !== original.id, + ) ?? []), + { + id: duplicate.id, + type: "arrow", + }, + ], + }); + } + } + } +}; + +export const fixReversedBindings = ( + originalElements: Map, + elementsWithClones: ExcalidrawElement[], + oldIdToDuplicatedId: Map, +) => { + for (const original of originalElements.values()) { + const duplicate = elementsWithClones.find( + (el) => el.id === oldIdToDuplicatedId.get(original.id), + )!; + + if (isBindableElement(original) && isBindableElement(duplicate)) { + fixReversedBindingsForBindables( + original, + duplicate, + originalElements, + elementsWithClones, + oldIdToDuplicatedId, + ); + } else if (isArrowElement(original) && isArrowElement(duplicate)) { + fixReversedBindingsForArrows( + original, + duplicate, + originalElements, + "startBinding", + oldIdToDuplicatedId, + elementsWithClones, + ); + fixReversedBindingsForArrows( + original, + duplicate, + originalElements, + "endBinding", + oldIdToDuplicatedId, + elementsWithClones, + ); + } + } +}; + export const fixBindingsAfterDeletion = ( sceneElements: readonly ExcalidrawElement[], deletedElements: readonly ExcalidrawElement[], diff --git a/packages/element/src/duplicate.ts b/packages/element/src/duplicate.ts index 5b95f9085b..ded1fd26c1 100644 --- a/packages/element/src/duplicate.ts +++ b/packages/element/src/duplicate.ts @@ -36,7 +36,10 @@ import { import { getBoundTextElement, getContainerElement } from "./textElement"; -import { fixBindingsAfterDuplication } from "./binding"; +import { + fixDuplicatedBindingsAfterDuplication, + fixReversedBindings, +} from "./binding"; import type { ElementsMap, @@ -381,12 +384,20 @@ export const duplicateElements = ( // --------------------------------------------------------------------------- - fixBindingsAfterDuplication( + fixDuplicatedBindingsAfterDuplication( newElements, oldIdToDuplicatedId, duplicatedElementsMap as NonDeletedSceneElementsMap, ); + if (reverseOrder) { + fixReversedBindings( + _idsOfElementsToDuplicate, + elementsWithClones, + oldIdToDuplicatedId, + ); + } + bindElementsToFramesAfterDuplication( elementsWithClones, oldElements, diff --git a/packages/element/tests/duplicate.test.tsx b/packages/element/tests/duplicate.test.tsx index d3b5cee963..7492bcc586 100644 --- a/packages/element/tests/duplicate.test.tsx +++ b/packages/element/tests/duplicate.test.tsx @@ -14,7 +14,7 @@ import { actionDuplicateSelection } from "@excalidraw/excalidraw/actions"; import { API } from "@excalidraw/excalidraw/tests/helpers/api"; -import { Keyboard, Pointer } from "@excalidraw/excalidraw/tests/helpers/ui"; +import { UI, Keyboard, Pointer } from "@excalidraw/excalidraw/tests/helpers/ui"; import { act, @@ -699,4 +699,34 @@ describe("duplication z-order", () => { { id: text.id, containerId: arrow.id, selected: true }, ]); }); + + it("reverse-duplicating bindable element with bound arrow should keep the arrow on the duplicate", () => { + const rect = UI.createElement("rectangle", { + x: 0, + y: 0, + width: 100, + height: 100, + }); + + const arrow = UI.createElement("arrow", { + x: -100, + y: 50, + width: 95, + height: 0, + }); + + expect(arrow.endBinding?.elementId).toBe(rect.id); + + Keyboard.withModifierKeys({ alt: true }, () => { + mouse.down(5, 5); + mouse.up(15, 15); + }); + + expect(window.h.elements).toHaveLength(3); + + const newRect = window.h.elements[0]; + + expect(arrow.endBinding?.elementId).toBe(newRect.id); + expect(newRect.boundElements?.[0]?.id).toBe(arrow.id); + }); }); diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 0ca525828a..276cde0274 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -99,6 +99,7 @@ import { isShallowEqual, arrayToMap, type EXPORT_IMAGE_TYPES, + randomInteger, } from "@excalidraw/common"; import { @@ -8521,20 +8522,26 @@ class App extends React.Component { }); if ( hitElement && + // hit element may not end up being selected + // if we're alt-dragging a common bounding box + // over the hit element + pointerDownState.hit.wasAddedToSelection && !selectedElements.find((el) => el.id === hitElement.id) ) { selectedElements.push(hitElement); } + const idsOfElementsToDuplicate = new Map( + selectedElements.map((el) => [el.id, el]), + ); + const { newElements: clonedElements, elementsWithClones } = duplicateElements({ type: "in-place", elements, appState: this.state, randomizeSeed: true, - idsOfElementsToDuplicate: new Map( - selectedElements.map((el) => [el.id, el]), - ), + idsOfElementsToDuplicate, overrides: (el) => { const origEl = pointerDownState.originalElements.get(el.id); @@ -8542,6 +8549,7 @@ class App extends React.Component { return { x: origEl.x, y: origEl.y, + seed: origEl.seed, }; } @@ -8561,7 +8569,14 @@ class App extends React.Component { const nextSceneElements = syncMovedIndices( mappedNewSceneElements || elementsWithClones, arrayToMap(clonedElements), - ); + ).map((el) => { + if (idsOfElementsToDuplicate.has(el.id)) { + return newElementWith(el, { + seed: randomInteger(), + }); + } + return el; + }); this.scene.replaceAllElements(nextSceneElements); this.maybeCacheVisibleGaps(event, selectedElements, true); diff --git a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap index 4001c3b178..4b863d4e78 100644 --- a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap @@ -20,7 +20,7 @@ exports[`duplicate element on move when ALT is clicked > rectangle 5`] = ` "roundness": { "type": 3, }, - "seed": 238820263, + "seed": 1278240551, "strokeColor": "#1e1e1e", "strokeStyle": "solid", "strokeWidth": 2, @@ -54,14 +54,14 @@ exports[`duplicate element on move when ALT is clicked > rectangle 6`] = ` "roundness": { "type": 3, }, - "seed": 1278240551, + "seed": 1505387817, "strokeColor": "#1e1e1e", "strokeStyle": "solid", "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 5, - "versionNonce": 23633383, + "version": 6, + "versionNonce": 915032327, "width": 30, "x": -10, "y": 60,