diff --git a/packages/excalidraw/actions/actionDuplicateSelection.tsx b/packages/excalidraw/actions/actionDuplicateSelection.tsx index 3cd45e08e..b66a1572b 100644 --- a/packages/excalidraw/actions/actionDuplicateSelection.tsx +++ b/packages/excalidraw/actions/actionDuplicateSelection.tsx @@ -52,24 +52,21 @@ export const actionDuplicateSelection = register({ } } - const duplicatedElements = duplicateElements(elements, { - idsOfElementsToDuplicate: arrayToMap( - getSelectedElements(elements, appState, { - includeBoundTextElement: true, - includeElementsInFrames: true, + let { newElements: duplicatedElements, elementsWithClones: nextElements } = + duplicateElements(elements, { + idsOfElementsToDuplicate: arrayToMap( + getSelectedElements(elements, appState, { + includeBoundTextElement: true, + includeElementsInFrames: true, + }), + ), + appState, + randomizeSeed: true, + overrides: (element) => ({ + x: element.x + DEFAULT_GRID_SIZE / 2, + y: element.y + DEFAULT_GRID_SIZE / 2, }), - ), - appState, - randomizeSeed: true, - overrides: (element) => ({ - x: element.x + DEFAULT_GRID_SIZE / 2, - y: element.y + DEFAULT_GRID_SIZE / 2, - }), - }); - - let nextElements = (elements as ExcalidrawElement[]) - .slice() - .concat(duplicatedElements); + }); if (app.props.onDuplicate && nextElements) { const mappedElements = app.props.onDuplicate(nextElements, elements); diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 7373991f2..f30ccc0e1 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -3219,7 +3219,7 @@ class App extends React.Component { const [gridX, gridY] = getGridPoint(dx, dy, this.getEffectiveGridSize()); - const newElements = duplicateElements( + const { newElements } = duplicateElements( elements.map((element) => { return newElementWith(element, { x: element.x + gridX - minX, @@ -8419,6 +8419,7 @@ class App extends React.Component { pointerDownState.hit.hasBeenDuplicated = true; + const elements = this.scene.getElementsIncludingDeleted(); const hitElement = pointerDownState.hit.element; const selectedElements = this.scene.getSelectedElements({ selectedElementIds: this.state.selectedElementIds, @@ -8431,13 +8432,18 @@ class App extends React.Component { ) { selectedElements.push(hitElement); } - const clonedElements = duplicateElements(selectedElements, { - appState: this.state, - randomizeSeed: true, - }); + + const { newElements: clonedElements, elementsWithClones } = + duplicateElements(elements, { + appState: this.state, + randomizeSeed: true, + idsOfElementsToDuplicate: new Map( + selectedElements.map((el) => [el.id, el]), + ), + }); const nextSceneElements = syncMovedIndices( - [...clonedElements, ...this.scene.getElementsIncludingDeleted()], + elementsWithClones, arrayToMap(clonedElements), ); diff --git a/packages/excalidraw/components/LibraryMenuItems.tsx b/packages/excalidraw/components/LibraryMenuItems.tsx index 7c8b5877f..8be1fe354 100644 --- a/packages/excalidraw/components/LibraryMenuItems.tsx +++ b/packages/excalidraw/components/LibraryMenuItems.tsx @@ -162,7 +162,8 @@ export default function LibraryMenuItems({ ...item, // duplicate each library item before inserting on canvas to confine // ids and bindings to each library item. See #6465 - elements: duplicateElements(item.elements, { randomizeSeed: true }), + elements: duplicateElements(item.elements, { randomizeSeed: true }) + .newElements, }; }); }, diff --git a/packages/excalidraw/element/duplicate.test.ts b/packages/excalidraw/element/duplicate.test.ts index cd5290fbc..d61de6687 100644 --- a/packages/excalidraw/element/duplicate.test.ts +++ b/packages/excalidraw/element/duplicate.test.ts @@ -46,7 +46,7 @@ describe("duplicating single elements", () => { points: [pointFrom(1, 2), pointFrom(3, 4)], }); - const copy = duplicateElement(null, new Map(), element); + const copy = duplicateElement(null, new Map(), element, undefined, true); assertCloneObjects(element, copy); @@ -65,6 +65,8 @@ describe("duplicating single elements", () => { ...element, id: copy.id, seed: copy.seed, + version: copy.version, + versionNonce: copy.versionNonce, }); }); @@ -150,7 +152,7 @@ describe("duplicating multiple elements", () => { // ------------------------------------------------------------------------- const origElements = [rectangle1, text1, arrow1, arrow2, text2] as const; - const clonedElements = duplicateElements(origElements); + const { newElements: clonedElements } = duplicateElements(origElements); // generic id in-equality checks // -------------------------------------------------------------------------- @@ -369,7 +371,9 @@ describe("duplicating multiple elements", () => { groupIds: ["g1"], }); - const [clonedRectangle1] = duplicateElements([rectangle1]); + const { + newElements: [clonedRectangle1], + } = duplicateElements([rectangle1]); expect(typeof clonedRectangle1.groupIds[0]).toBe("string"); expect(rectangle1.groupIds[0]).not.toBe(clonedRectangle1.groupIds[0]); diff --git a/packages/excalidraw/element/duplicate.ts b/packages/excalidraw/element/duplicate.ts index c12bcb1c1..a3a42fdda 100644 --- a/packages/excalidraw/element/duplicate.ts +++ b/packages/excalidraw/element/duplicate.ts @@ -130,7 +130,8 @@ export const duplicateElements = ( const duplicatedElementsMap = new Map(); const elementsMap = arrayToMap(elements) as ElementsMap; const _idsOfElementsToDuplicate = - opts?.idsOfElementsToDuplicate ?? new Set(elements.map((el) => el.id)); + opts?.idsOfElementsToDuplicate ?? + new Map(elements.map((el) => [el.id, el])); elements = normalizeElementOrder(elements); @@ -397,138 +398,12 @@ export const duplicateElements = ( oldIdToDuplicatedId, ); - return newElements; + return { + newElements, + elementsWithClones, + }; }; -/** - * Clones elements, regenerating their ids (including bindings) and group ids. - * - * If bindings don't exist in the elements array, they are removed. Therefore, - * it's advised to supply the whole elements array, or sets of elements that - * are encapsulated (such as library items), if the purpose is to retain - * bindings to the cloned elements intact. - * - * NOTE by default does not randomize or regenerate anything except the id. - */ -// export const duplicateElements = ( -// elements: readonly ExcalidrawElement[], -// opts?: { -// /** NOTE also updates version flags and `updated` */ -// randomizeSeed?: boolean; -// overrides?: (element: ExcalidrawElement) => Partial; -// }, -// ) => { -// const clonedElements: ExcalidrawElement[] = []; - -// const origElementsMap = arrayToMap(elements); - -// // used for for migrating old ids to new ids -// const elementNewIdsMap = new Map< -// /* orig */ ExcalidrawElement["id"], -// /* new */ ExcalidrawElement["id"] -// >(); - -// const maybeGetNewIdFor = (id: ExcalidrawElement["id"]) => { -// // if we've already migrated the element id, return the new one directly -// if (elementNewIdsMap.has(id)) { -// return elementNewIdsMap.get(id)!; -// } -// // if we haven't migrated the element id, but an old element with the same -// // id exists, generate a new id for it and return it -// if (origElementsMap.has(id)) { -// const newId = randomId(); -// elementNewIdsMap.set(id, newId); -// return newId; -// } -// // if old element doesn't exist, return null to mark it for removal -// return null; -// }; - -// const groupNewIdsMap = new Map(); - -// for (const element of elements) { -// let clonedElement: Mutable = _deepCopyElement(element); - -// if (opts?.overrides) { -// clonedElement = Object.assign( -// clonedElement, -// opts.overrides(clonedElement), -// ); -// } - -// clonedElement.id = maybeGetNewIdFor(element.id)!; -// if (isTestEnv()) { -// __test__defineOrigId(clonedElement, element.id); -// } - -// if (opts?.randomizeSeed) { -// clonedElement.seed = randomInteger(); -// bumpVersion(clonedElement); -// } - -// if (clonedElement.groupIds) { -// clonedElement.groupIds = clonedElement.groupIds.map((groupId) => { -// if (!groupNewIdsMap.has(groupId)) { -// groupNewIdsMap.set(groupId, randomId()); -// } -// return groupNewIdsMap.get(groupId)!; -// }); -// } - -// if ("containerId" in clonedElement && clonedElement.containerId) { -// const newContainerId = maybeGetNewIdFor(clonedElement.containerId); -// clonedElement.containerId = newContainerId; -// } - -// if ("boundElements" in clonedElement && clonedElement.boundElements) { -// clonedElement.boundElements = clonedElement.boundElements.reduce( -// ( -// acc: Mutable>, -// binding, -// ) => { -// const newBindingId = maybeGetNewIdFor(binding.id); -// if (newBindingId) { -// acc.push({ ...binding, id: newBindingId }); -// } -// return acc; -// }, -// [], -// ); -// } - -// if ("endBinding" in clonedElement && clonedElement.endBinding) { -// const newEndBindingId = maybeGetNewIdFor( -// clonedElement.endBinding.elementId, -// ); -// clonedElement.endBinding = newEndBindingId -// ? { -// ...clonedElement.endBinding, -// elementId: newEndBindingId, -// } -// : null; -// } -// if ("startBinding" in clonedElement && clonedElement.startBinding) { -// const newEndBindingId = maybeGetNewIdFor( -// clonedElement.startBinding.elementId, -// ); -// clonedElement.startBinding = newEndBindingId -// ? { -// ...clonedElement.startBinding, -// elementId: newEndBindingId, -// } -// : null; -// } - -// if (clonedElement.frameId) { -// clonedElement.frameId = maybeGetNewIdFor(clonedElement.frameId); -// } - -// clonedElements.push(clonedElement); -// } - -// return clonedElements; -// }; - // Simplified deep clone for the purpose of cloning ExcalidrawElement. // // Only clones plain objects and arrays. Doesn't clone Date, RegExp, Map, Set, diff --git a/packages/excalidraw/tests/__snapshots__/contextmenu.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/contextmenu.test.tsx.snap index f77eb8ddb..89629b93e 100644 --- a/packages/excalidraw/tests/__snapshots__/contextmenu.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/contextmenu.test.tsx.snap @@ -2606,8 +2606,8 @@ exports[`contextMenu element > selecting 'Duplicate' in context menu duplicates "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 4, - "versionNonce": 238820263, + "version": 5, + "versionNonce": 400692809, "width": 20, "x": 0, "y": 10, diff --git a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap index d740e975c..3f523d005 100644 --- a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap @@ -15172,9 +15172,11 @@ History { "selectedElementIds": { "id61": true, }, + "selectedLinearElementId": "id61", }, "inserted": { "selectedElementIds": {}, + "selectedLinearElementId": null, }, }, }, @@ -18946,7 +18948,7 @@ exports[`history > singleplayer undo/redo > should support duplication of groups "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 3, + "version": 4, "width": 100, "x": 10, "y": 10, @@ -18980,7 +18982,7 @@ exports[`history > singleplayer undo/redo > should support duplication of groups "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 3, + "version": 4, "width": 100, "x": 110, "y": 110, @@ -19014,7 +19016,7 @@ exports[`history > singleplayer undo/redo > should support duplication of groups "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 6, + "version": 7, "width": 100, "x": 10, "y": 10, @@ -19048,7 +19050,7 @@ exports[`history > singleplayer undo/redo > should support duplication of groups "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 6, + "version": 7, "width": 100, "x": 110, "y": 110, diff --git a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap index 90236a4dd..c52ce2f0d 100644 --- a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap @@ -1,73 +1,5 @@ // 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": "id2", - "index": "a0", - "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": 5, - "versionNonce": 400692809, - "width": 30, - "x": 30, - "y": 20, -} -`; - -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": "id0", - "index": "a1", - "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": 6, - "versionNonce": 23633383, - "width": 30, - "x": -10, - "y": 60, -} -`; - 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 5d48ead6c..397df2e81 100644 --- a/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap @@ -2139,7 +2139,7 @@ History { "frameId": null, "groupIds": [], "height": 10, - "index": "a0", + "index": "a1", "isDeleted": false, "link": null, "locked": false, @@ -2153,8 +2153,8 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 10, - "y": 10, + "x": 20, + "y": 20, }, "inserted": { "isDeleted": true, @@ -2164,12 +2164,10 @@ History { "updated": Map { "id0" => Delta { "deleted": { - "index": "a1", "x": 20, "y": 20, }, "inserted": { - "index": "a0", "x": 10, "y": 10, }, @@ -10631,7 +10629,7 @@ History { "id7", ], "height": 10, - "index": "a0", + "index": "a3", "isDeleted": false, "link": null, "locked": false, @@ -10645,8 +10643,8 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 10, - "y": 10, + "x": 20, + "y": 20, }, "inserted": { "isDeleted": true, @@ -10664,7 +10662,7 @@ History { "id7", ], "height": 10, - "index": "a1", + "index": "a4", "isDeleted": false, "link": null, "locked": false, @@ -10678,8 +10676,8 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 30, - "y": 10, + "x": 40, + "y": 20, }, "inserted": { "isDeleted": true, @@ -10697,7 +10695,7 @@ History { "id7", ], "height": 10, - "index": "a2", + "index": "a5", "isDeleted": false, "link": null, "locked": false, @@ -10711,8 +10709,8 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 50, - "y": 10, + "x": 60, + "y": 20, }, "inserted": { "isDeleted": true, @@ -10722,36 +10720,30 @@ History { "updated": Map { "id0" => Delta { "deleted": { - "index": "a3", "x": 20, "y": 20, }, "inserted": { - "index": "a0", "x": 10, "y": 10, }, }, "id1" => Delta { "deleted": { - "index": "a4", "x": 40, "y": 20, }, "inserted": { - "index": "a1", "x": 30, "y": 10, }, }, "id2" => Delta { "deleted": { - "index": "a5", "x": 60, "y": 20, }, "inserted": { - "index": "a2", "x": 50, "y": 10, },