diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index f30ccc0e1..f86815b46 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -8440,10 +8440,25 @@ class App extends React.Component { idsOfElementsToDuplicate: new Map( selectedElements.map((el) => [el.id, el]), ), + overrides: (el) => { + const origEl = pointerDownState.originalElements.get(el.id)!; + return { + x: origEl.x, + y: origEl.y, + }; + }, }); + clonedElements.forEach((element) => { + pointerDownState.originalElements.set(element.id, element); + }); + + const mappedNewSceneElements = this.props.onDuplicate?.( + elementsWithClones, + elements, + ); const nextSceneElements = syncMovedIndices( - elementsWithClones, + mappedNewSceneElements || elementsWithClones, arrayToMap(clonedElements), ); diff --git a/packages/excalidraw/element/binding.ts b/packages/excalidraw/element/binding.ts index 32b15cc23..38b7d8636 100644 --- a/packages/excalidraw/element/binding.ts +++ b/packages/excalidraw/element/binding.ts @@ -49,7 +49,6 @@ import { getBoundTextElement, handleBindTextResize } from "./textElement"; import { isArrowElement, isBindableElement, - isBindingElement, isBoundToContainer, isElbowArrow, isFixedPointBinding, @@ -59,6 +58,10 @@ import { isTextElement, } from "./typeChecks"; +import { updateElbowArrowPoints } from "./elbowArrow"; + +import type { Mutable } from "../utility-types"; + import type { Bounds } from "./bounds"; import type { ElementUpdate } from "./mutateElement"; import type { @@ -974,7 +977,6 @@ export const bindPointToSnapToElementOutline = ( otherPoint, ), ), - FIXED_BINDING_DISTANCE, )[0]; } else { intersection = intersectElementWithLineSegment( @@ -1147,7 +1149,7 @@ export const snapToMid = ( ) { // LEFT return pointRotateRads( - pointFrom(x - 2 * FIXED_BINDING_DISTANCE, center[1]), + pointFrom(x - FIXED_BINDING_DISTANCE, center[1]), center, angle, ); @@ -1158,7 +1160,7 @@ export const snapToMid = ( ) { // TOP return pointRotateRads( - pointFrom(center[0], y - 2 * FIXED_BINDING_DISTANCE), + pointFrom(center[0], y - FIXED_BINDING_DISTANCE), center, angle, ); @@ -1169,7 +1171,7 @@ export const snapToMid = ( ) { // RIGHT return pointRotateRads( - pointFrom(x + width + 2 * FIXED_BINDING_DISTANCE, center[1]), + pointFrom(x + width + FIXED_BINDING_DISTANCE, center[1]), center, angle, ); @@ -1180,7 +1182,7 @@ export const snapToMid = ( ) { // DOWN return pointRotateRads( - pointFrom(center[0], y + height + 2 * FIXED_BINDING_DISTANCE), + pointFrom(center[0], y + height + FIXED_BINDING_DISTANCE), center, angle, ); @@ -1412,107 +1414,75 @@ const getLinearElementEdgeCoors = ( ); }; -// We need to: -// 1: Update elements not selected to point to duplicated elements -// 2: Update duplicated elements to point to other duplicated elements export const fixBindingsAfterDuplication = ( - sceneElements: readonly ExcalidrawElement[], - oldElements: readonly ExcalidrawElement[], + newElements: ExcalidrawElement[], oldIdToDuplicatedId: Map, - // There are three copying mechanisms: Copy-paste, duplication and alt-drag. - // Only when alt-dragging the new "duplicates" act as the "old", while - // the "old" elements act as the "new copy" - essentially working reverse - // to the other two. - duplicatesServeAsOld?: "duplicatesServeAsOld" | undefined, -): void => { - // First collect all the binding/bindable elements, so we only update - // each once, regardless of whether they were duplicated or not. - const allBoundElementIds: Set = new Set(); - const allBindableElementIds: Set = new Set(); - const shouldReverseRoles = duplicatesServeAsOld === "duplicatesServeAsOld"; - const duplicateIdToOldId = new Map( - [...oldIdToDuplicatedId].map(([key, value]) => [value, key]), - ); - oldElements.forEach((oldElement) => { - const { boundElements } = oldElement; - if (boundElements != null && boundElements.length > 0) { - boundElements.forEach((boundElement) => { - if (shouldReverseRoles && !oldIdToDuplicatedId.has(boundElement.id)) { - allBoundElementIds.add(boundElement.id); - } + duplicatedElementsMap: NonDeletedSceneElementsMap, +) => { + for (const element of newElements) { + if ("boundElements" in element && element.boundElements) { + 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; + }, + [], + ), }); - allBindableElementIds.add(oldIdToDuplicatedId.get(oldElement.id)!); } - if (isBindingElement(oldElement)) { - if (oldElement.startBinding != null) { - const { elementId } = oldElement.startBinding; - if (shouldReverseRoles && !oldIdToDuplicatedId.has(elementId)) { - allBindableElementIds.add(elementId); - } - } - if (oldElement.endBinding != null) { - const { elementId } = oldElement.endBinding; - if (shouldReverseRoles && !oldIdToDuplicatedId.has(elementId)) { - allBindableElementIds.add(elementId); - } - } - if (oldElement.startBinding != null || oldElement.endBinding != null) { - allBoundElementIds.add(oldIdToDuplicatedId.get(oldElement.id)!); - } + + if ("containerId" in element && element.containerId) { + Object.assign(element, { + containerId: oldIdToDuplicatedId.get(element.containerId) ?? null, + }); } - }); - // Update the linear elements - ( - sceneElements.filter(({ id }) => - allBoundElementIds.has(id), - ) as ExcalidrawLinearElement[] - ).forEach((element) => { - const { startBinding, endBinding } = element; - mutateElement(element, { - startBinding: newBindingAfterDuplication( - startBinding, - oldIdToDuplicatedId, - ), - endBinding: newBindingAfterDuplication(endBinding, oldIdToDuplicatedId), - }); - }); + if ("endBinding" in element && element.endBinding) { + const newEndBindingId = oldIdToDuplicatedId.get( + element.endBinding.elementId, + ); + Object.assign(element, { + endBinding: newEndBindingId + ? { + ...element.endBinding, + elementId: newEndBindingId, + } + : null, + }); + } + if ("startBinding" in element && element.startBinding) { + const newEndBindingId = oldIdToDuplicatedId.get( + element.startBinding.elementId, + ); + Object.assign(element, { + startBinding: newEndBindingId + ? { + ...element.startBinding, + elementId: newEndBindingId, + } + : null, + }); + } - // Update the bindable shapes - sceneElements - .filter(({ id }) => allBindableElementIds.has(id)) - .forEach((bindableElement) => { - const oldElementId = duplicateIdToOldId.get(bindableElement.id); - const boundElements = sceneElements.find( - ({ id }) => id === oldElementId, - )?.boundElements; - - if (boundElements && boundElements.length > 0) { - mutateElement(bindableElement, { - boundElements: boundElements.map((boundElement) => - oldIdToDuplicatedId.has(boundElement.id) - ? { - id: oldIdToDuplicatedId.get(boundElement.id)!, - type: boundElement.type, - } - : boundElement, - ), - }); - } - }); -}; - -const newBindingAfterDuplication = ( - binding: PointBinding | null, - oldIdToDuplicatedId: Map, -): PointBinding | null => { - if (binding == null) { - return null; + if (isElbowArrow(element)) { + Object.assign( + element, + updateElbowArrowPoints(element, duplicatedElementsMap, { + points: [ + element.points[0], + element.points[element.points.length - 1], + ], + }), + ); + } } - return { - ...binding, - elementId: oldIdToDuplicatedId.get(binding.elementId) ?? binding.elementId, - }; }; export const fixBindingsAfterDeletion = ( diff --git a/packages/excalidraw/element/duplicate.test.ts b/packages/excalidraw/element/duplicate.test.ts index d61de6687..52e4cef2e 100644 --- a/packages/excalidraw/element/duplicate.test.ts +++ b/packages/excalidraw/element/duplicate.test.ts @@ -209,6 +209,7 @@ describe("duplicating multiple elements", () => { type: clonedText1.type, }), ); + expect(clonedRectangle.type).toBe("rectangle"); clonedArrows.forEach((arrow) => { expect( @@ -302,9 +303,9 @@ describe("duplicating multiple elements", () => { // ------------------------------------------------------------------------- const origElements = [rectangle1, text1, arrow1, arrow2, arrow3] as const; - const clonedElements = duplicateElements( + const { newElements: clonedElements } = duplicateElements( origElements, - ) as any as typeof origElements; + ) as any as { newElements: typeof origElements }; const [ clonedRectangle, clonedText1, @@ -324,7 +325,7 @@ describe("duplicating multiple elements", () => { elementId: clonedRectangle.id, }); expect(clonedArrow2.endBinding).toBe(null); - + console.log(clonedArrow3); expect(clonedArrow3.startBinding).toBe(null); expect(clonedArrow3.endBinding).toEqual({ ...arrow3.endBinding, @@ -348,9 +349,9 @@ describe("duplicating multiple elements", () => { }); const origElements = [rectangle1, rectangle2, rectangle3] as const; - const clonedElements = duplicateElements( + const { newElements: clonedElements } = duplicateElements( origElements, - ) as any as typeof origElements; + ) as any as { newElements: typeof origElements }; const [clonedRectangle1, clonedRectangle2, clonedRectangle3] = clonedElements; diff --git a/packages/excalidraw/element/duplicate.ts b/packages/excalidraw/element/duplicate.ts index a3a42fdda..b5f1dd50b 100644 --- a/packages/excalidraw/element/duplicate.ts +++ b/packages/excalidraw/element/duplicate.ts @@ -28,17 +28,12 @@ import { bumpVersion } from "./mutateElement"; import { hasBoundTextElement, isBoundToContainer, - isElbowArrow, isFrameLikeElement, } from "./typeChecks"; -import { - bindTextToShapeAfterDuplication, - getBoundTextElement, - getContainerElement, -} from "./textElement"; +import { getBoundTextElement, getContainerElement } from "./textElement"; -import { updateElbowArrowPoints } from "./elbowArrow"; +import { fixBindingsAfterDuplication } from "./binding"; import type { AppState } from "../types"; import type { Mutable } from "../utility-types"; @@ -315,83 +310,12 @@ export const duplicateElements = ( // --------------------------------------------------------------------------- - const fixBindingsAfterDuplication = ( - newElements: ExcalidrawElement[], - oldIdToDuplicatedId: Map, - duplicatedElementsMap: NonDeletedSceneElementsMap, - ) => { - for (const element of newElements) { - if ("boundElements" in element && element.boundElements) { - 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, - ); - Object.assign(element, { - endBinding: newEndBindingId - ? { - ...element.endBinding, - elementId: newEndBindingId, - } - : null, - }); - } - if ("startBinding" in element && element.startBinding) { - const newEndBindingId = oldIdToDuplicatedId.get( - element.startBinding.elementId, - ); - Object.assign(element, { - startBinding: newEndBindingId - ? { - ...element.startBinding, - elementId: newEndBindingId, - } - : null, - }); - } - - if (isElbowArrow(element)) { - Object.assign( - element, - updateElbowArrowPoints(element, duplicatedElementsMap, { - points: [ - element.points[0], - element.points[element.points.length - 1], - ], - }), - ); - } - } - }; - fixBindingsAfterDuplication( newElements, oldIdToDuplicatedId, duplicatedElementsMap as NonDeletedSceneElementsMap, ); - bindTextToShapeAfterDuplication( - elementsWithClones, - oldElements, - oldIdToDuplicatedId, - ); - bindElementsToFramesAfterDuplication( elementsWithClones, oldElements, diff --git a/packages/excalidraw/element/elbowArrow.test.tsx b/packages/excalidraw/element/elbowArrow.test.tsx index 91e280d30..621d2640f 100644 --- a/packages/excalidraw/element/elbowArrow.test.tsx +++ b/packages/excalidraw/element/elbowArrow.test.tsx @@ -358,7 +358,7 @@ describe("elbow arrow ui", () => { expect(arrow.endBinding).not.toBe(null); }); - it("keeps arrow shape when only the bound arrow is duplicated", async () => { + it("changes arrow shape to unbind variant if only the connected elbow arrow is duplicated", async () => { UI.createElement("rectangle", { x: -150, y: -150, @@ -404,8 +404,8 @@ describe("elbow arrow ui", () => { expect(duplicatedArrow.elbowed).toBe(true); expect(duplicatedArrow.points).toEqual([ [0, 0], - [45, 0], - [45, 200], + [0, 100], + [90, 100], [90, 200], ]); }); diff --git a/packages/excalidraw/element/textElement.ts b/packages/excalidraw/element/textElement.ts index 9893ba5d6..b8399f038 100644 --- a/packages/excalidraw/element/textElement.ts +++ b/packages/excalidraw/element/textElement.ts @@ -6,7 +6,7 @@ import { TEXT_ALIGN, VERTICAL_ALIGN, } from "../constants"; -import { getFontString, arrayToMap } from "../utils"; +import { getFontString } from "../utils"; import { resetOriginalContainerCache, @@ -112,48 +112,6 @@ export const redrawTextBoundingBox = ( mutateElement(textElement, boundTextUpdates, informMutation); }; -export const bindTextToShapeAfterDuplication = ( - newElements: ExcalidrawElement[], - oldElements: ExcalidrawElement[], - oldIdToDuplicatedId: Map, -): void => { - const newElementsMap = arrayToMap(newElements) as Map< - ExcalidrawElement["id"], - ExcalidrawElement - >; - oldElements.forEach((element) => { - const newElementId = oldIdToDuplicatedId.get(element.id) as string; - const boundTextElementId = getBoundTextElementId(element); - - if (boundTextElementId) { - const newTextElementId = oldIdToDuplicatedId.get(boundTextElementId); - if (newTextElementId) { - const newContainer = newElementsMap.get(newElementId); - if (newContainer) { - mutateElement(newContainer, { - boundElements: (element.boundElements || []) - .filter( - (boundElement) => - boundElement.id !== newTextElementId && - boundElement.id !== boundTextElementId, - ) - .concat({ - type: "text", - id: newTextElementId, - }), - }); - } - const newTextElement = newElementsMap.get(newTextElementId); - if (newTextElement && isTextElement(newTextElement)) { - mutateElement(newTextElement, { - containerId: newContainer ? newElementId : null, - }); - } - } - } - }); -}; - export const handleBindTextResize = ( container: NonDeletedExcalidrawElement, elementsMap: ElementsMap, diff --git a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap index 3f523d005..252d7f981 100644 --- a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap @@ -924,11 +924,12 @@ History { ], ], "startBinding": null, + "y": 0, }, "inserted": { "endBinding": { "elementId": "id166", - "focus": -0, + "focus": "0.00000", "gap": 1, }, "points": [ @@ -943,9 +944,10 @@ History { ], "startBinding": { "elementId": "id165", - "focus": 0, + "focus": "-0.00000", "gap": 1, }, + "y": "0.00000", }, }, }, diff --git a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap index c52ce2f0d..bea916d63 100644 --- a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap @@ -1,5 +1,73 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`duplicate element on move when ALT is clicked > rectangle 5`] = ` +{ + "angle": 0, + "backgroundColor": "transparent", + "boundElements": null, + "customData": undefined, + "fillStyle": "solid", + "frameId": null, + "groupIds": [], + "height": 50, + "id": "id0", + "index": "a0", + "isDeleted": false, + "link": null, + "locked": false, + "opacity": 100, + "roughness": 1, + "roundness": { + "type": 3, + }, + "seed": 1278240551, + "strokeColor": "#1e1e1e", + "strokeStyle": "solid", + "strokeWidth": 2, + "type": "rectangle", + "updated": 1, + "version": 5, + "versionNonce": 23633383, + "width": 30, + "x": -10, + "y": 60, +} +`; + +exports[`duplicate element on move when ALT is clicked > rectangle 6`] = ` +{ + "angle": 0, + "backgroundColor": "transparent", + "boundElements": null, + "customData": undefined, + "fillStyle": "solid", + "frameId": null, + "groupIds": [], + "height": 50, + "id": "id2", + "index": "a1", + "isDeleted": false, + "link": null, + "locked": false, + "opacity": 100, + "roughness": 1, + "roundness": { + "type": 3, + }, + "seed": 238820263, + "strokeColor": "#1e1e1e", + "strokeStyle": "solid", + "strokeWidth": 2, + "type": "rectangle", + "updated": 1, + "version": 6, + "versionNonce": 1604849351, + "width": 30, + "x": 30, + "y": 20, +} +`; + exports[`move element > rectangle 5`] = ` { "angle": 0, diff --git a/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap index 397df2e81..2a4fcac97 100644 --- a/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap @@ -2153,8 +2153,8 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 20, - "y": 20, + "x": 10, + "y": 10, }, "inserted": { "isDeleted": true, @@ -10643,8 +10643,8 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 20, - "y": 20, + "x": 10, + "y": 10, }, "inserted": { "isDeleted": true, @@ -10676,8 +10676,8 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 40, - "y": 20, + "x": 30, + "y": 10, }, "inserted": { "isDeleted": true, @@ -10709,8 +10709,8 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 60, - "y": 20, + "x": 50, + "y": 10, }, "inserted": { "isDeleted": true, diff --git a/packages/excalidraw/tests/move.test.tsx b/packages/excalidraw/tests/move.test.tsx index 855496f44..86b72c9e9 100644 --- a/packages/excalidraw/tests/move.test.tsx +++ b/packages/excalidraw/tests/move.test.tsx @@ -174,9 +174,9 @@ describe("duplicate element on move when ALT is clicked", () => { expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(2); - // previous element should stay intact - expect([h.elements[0].x, h.elements[0].y]).toEqual([30, 20]); - expect([h.elements[1].x, h.elements[1].y]).toEqual([-10, 60]); + // behavior should be the same as Ctrl+D + expect([h.elements[0].x, h.elements[0].y]).toEqual([-10, 60]); + expect([h.elements[1].x, h.elements[1].y]).toEqual([30, 20]); h.elements.forEach((element) => expect(element).toMatchSnapshot()); }); diff --git a/packages/excalidraw/tests/resize.test.tsx b/packages/excalidraw/tests/resize.test.tsx index 055b097b3..d463d3aed 100644 --- a/packages/excalidraw/tests/resize.test.tsx +++ b/packages/excalidraw/tests/resize.test.tsx @@ -505,12 +505,12 @@ describe("arrow element", () => { h.state, )[0] as ExcalidrawElbowArrowElement; - expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1); + expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1, 0); expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75); UI.resize(rectangle, "se", [-200, -150]); - expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1); + expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1, 0); expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75); }); @@ -533,11 +533,11 @@ describe("arrow element", () => { h.state, )[0] as ExcalidrawElbowArrowElement; - expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1); + expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1, 0); expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75); UI.resize([rectangle, arrow], "nw", [300, 350]); - expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(0); + expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(0, 0); expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.25); }); });