diff --git a/packages/excalidraw/actions/actionDuplicateSelection.tsx b/packages/excalidraw/actions/actionDuplicateSelection.tsx index b66a1572b..a6dd9d2e1 100644 --- a/packages/excalidraw/actions/actionDuplicateSelection.tsx +++ b/packages/excalidraw/actions/actionDuplicateSelection.tsx @@ -53,7 +53,9 @@ export const actionDuplicateSelection = register({ } let { newElements: duplicatedElements, elementsWithClones: nextElements } = - duplicateElements(elements, { + duplicateElements({ + type: "in-place", + elements, idsOfElementsToDuplicate: arrayToMap( getSelectedElements(elements, appState, { includeBoundTextElement: true, @@ -66,6 +68,7 @@ export const actionDuplicateSelection = register({ x: element.x + DEFAULT_GRID_SIZE / 2, y: element.y + DEFAULT_GRID_SIZE / 2, }), + reverseOrder: false, }); if (app.props.onDuplicate && nextElements) { diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index f48b736ed..8239493ba 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -3219,17 +3219,16 @@ class App extends React.Component { const [gridX, gridY] = getGridPoint(dx, dy, this.getEffectiveGridSize()); - const { newElements } = duplicateElements( - elements.map((element) => { + const { newElements } = duplicateElements({ + type: "everything", + elements: elements.map((element) => { return newElementWith(element, { x: element.x + gridX - minX, y: element.y + gridY - minY, }); }), - { - randomizeSeed: !opts.retainSeed, - }, - ); + randomizeSeed: !opts.retainSeed, + }); const prevElements = this.scene.getElementsIncludingDeleted(); let nextElements = [...prevElements, ...newElements]; @@ -8434,7 +8433,9 @@ class App extends React.Component { } const { newElements: clonedElements, elementsWithClones } = - duplicateElements(elements, { + duplicateElements({ + type: "in-place", + elements, appState: this.state, randomizeSeed: true, idsOfElementsToDuplicate: new Map( diff --git a/packages/excalidraw/components/LibraryMenuItems.tsx b/packages/excalidraw/components/LibraryMenuItems.tsx index 8be1fe354..3e32021e0 100644 --- a/packages/excalidraw/components/LibraryMenuItems.tsx +++ b/packages/excalidraw/components/LibraryMenuItems.tsx @@ -162,8 +162,11 @@ 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 }) - .newElements, + elements: duplicateElements({ + type: "everything", + elements: item.elements, + randomizeSeed: true, + }).newElements, }; }); }, diff --git a/packages/excalidraw/element/duplicate.test.tsx b/packages/excalidraw/element/duplicate.test.tsx index bb3478f6e..5b1192f13 100644 --- a/packages/excalidraw/element/duplicate.test.tsx +++ b/packages/excalidraw/element/duplicate.test.tsx @@ -7,7 +7,12 @@ import { FONT_FAMILY, ORIG_ID, ROUNDNESS } from "../constants"; import { API } from "../tests/helpers/api"; import { isPrimitive } from "../utils"; -import { act, assertElements, render } from "../tests/test-utils"; +import { + act, + assertElements, + getCloneByOrigId, + render, +} from "../tests/test-utils"; import { Excalidraw } from ".."; import { actionDuplicateSelection } from "../actions"; @@ -162,7 +167,10 @@ describe("duplicating multiple elements", () => { // ------------------------------------------------------------------------- const origElements = [rectangle1, text1, arrow1, arrow2, text2] as const; - const { newElements: clonedElements } = duplicateElements(origElements); + const { newElements: clonedElements } = duplicateElements({ + type: "everything", + elements: origElements, + }); // generic id in-equality checks // -------------------------------------------------------------------------- @@ -313,9 +321,10 @@ describe("duplicating multiple elements", () => { // ------------------------------------------------------------------------- const origElements = [rectangle1, text1, arrow1, arrow2, arrow3] as const; - const { newElements: clonedElements } = duplicateElements( - origElements, - ) as any as { newElements: typeof origElements }; + const { newElements: clonedElements } = duplicateElements({ + type: "everything", + elements: origElements, + }) as any as { newElements: typeof origElements }; const [ clonedRectangle, @@ -359,9 +368,10 @@ describe("duplicating multiple elements", () => { }); const origElements = [rectangle1, rectangle2, rectangle3] as const; - const { newElements: clonedElements } = duplicateElements( - origElements, - ) as any as { newElements: typeof origElements }; + const { newElements: clonedElements } = duplicateElements({ + type: "everything", + elements: origElements, + }) as any as { newElements: typeof origElements }; const [clonedRectangle1, clonedRectangle2, clonedRectangle3] = clonedElements; @@ -384,7 +394,7 @@ describe("duplicating multiple elements", () => { const { newElements: [clonedRectangle1], - } = duplicateElements([rectangle1]); + } = duplicateElements({ type: "everything", elements: [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 4baddae36..e08bb3b4d 100644 --- a/packages/excalidraw/element/duplicate.ts +++ b/packages/excalidraw/element/duplicate.ts @@ -96,20 +96,62 @@ export const duplicateElement = ( }; export const duplicateElements = ( - elements: readonly ExcalidrawElement[], - opts?: { - idsOfElementsToDuplicate?: Map; - appState?: { - editingGroupId: AppState["editingGroupId"]; - selectedGroupIds: AppState["selectedGroupIds"]; - }; + opts: { + elements: readonly ExcalidrawElement[]; + randomizeSeed?: boolean; overrides?: ( originalElement: ExcalidrawElement, ) => Partial; - randomizeSeed?: boolean; - reverseOrder?: boolean; - }, + } & ( + | { + /** + * Duplicates all elements in array. + * + * Use this when programmaticaly duplicating elements, without direct + * user interaction. + */ + type: "everything"; + } + | { + /** + * Duplicates specified elements and inserts them back into the array + * in specified order. + * + * Use this when duplicating Scene elements, during user interaction + * such as alt-drag or on duplicate action. + */ + type: "in-place"; + idsOfElementsToDuplicate: Map< + ExcalidrawElement["id"], + ExcalidrawElement + >; + appState: { + editingGroupId: AppState["editingGroupId"]; + selectedGroupIds: AppState["selectedGroupIds"]; + }; + /** + * If true, duplicated elements are inserted _before_ specified + * elements. Case: alt-dragging elements to duplicate them. + * + * TODO: remove this once (if) we stop replacing the original element + * with the duplicated one in the scene array. + */ + reverseOrder: boolean; + } + ), ) => { + let { elements } = opts; + + const appState = + "appState" in opts + ? opts.appState + : ({ + editingGroupId: null, + selectedGroupIds: {}, + } as const); + + const reverseOrder = opts.type === "in-place" ? opts.reverseOrder : false; + // 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 @@ -128,11 +170,12 @@ export const duplicateElements = ( const duplicatedElementsMap = new Map(); const elementsMap = arrayToMap(elements) as ElementsMap; const _idsOfElementsToDuplicate = - opts?.idsOfElementsToDuplicate ?? - new Map(elements.map((el) => [el.id, el])); + opts.type === "in-place" + ? opts.idsOfElementsToDuplicate + : new Map(elements.map((el) => [el.id, el])); // For sanity - if (opts?.appState?.selectedGroupIds) { + if (opts.type === "in-place") { for (const groupId of Object.keys(opts.appState.selectedGroupIds)) { elements .filter((el) => el.groupIds?.includes(groupId)) @@ -165,11 +208,11 @@ export const duplicateElements = ( processedIds.set(element.id, true); const newElement = duplicateElement( - opts?.appState?.editingGroupId ?? null, + appState.editingGroupId, groupIdMap, element, - opts?.overrides?.(element), - opts?.randomizeSeed, + opts.overrides?.(element), + opts.randomizeSeed, ); processedIds.set(newElement.id, true); @@ -204,18 +247,18 @@ export const duplicateElements = ( return; } - if (opts?.reverseOrder && index < 1) { + if (reverseOrder && index < 1) { elementsWithClones.unshift(...castArray(elements)); return; } - if (!opts?.reverseOrder && index > elementsWithClones.length - 1) { + if (!reverseOrder && index > elementsWithClones.length - 1) { elementsWithClones.push(...castArray(elements)); return; } elementsWithClones.splice( - index + (opts?.reverseOrder ? 0 : 1), + index + (reverseOrder ? 0 : 1), 0, ...castArray(elements), ); @@ -241,13 +284,7 @@ export const duplicateElements = ( // groups // ------------------------------------------------------------------------- - const groupId = getSelectedGroupForElement( - (opts?.appState ?? { - editingGroupId: null, - selectedGroupIds: {}, - }) as AppState, - element, - ); + const groupId = getSelectedGroupForElement(appState, element); if (groupId) { const groupElements = getElementsInGroup(elements, groupId).flatMap( (element) => @@ -256,7 +293,7 @@ export const duplicateElements = ( : [element], ); - const targetIndex = opts?.reverseOrder + const targetIndex = reverseOrder ? elementsWithClones.findIndex((el) => { return el.groupIds?.includes(groupId); }) @@ -306,7 +343,7 @@ export const duplicateElements = ( if (boundTextElement) { insertBeforeOrAfterIndex( - targetIndex + (opts?.reverseOrder ? -1 : 0), + targetIndex + (reverseOrder ? -1 : 0), copyElements([element, boundTextElement]), ); } else { diff --git a/packages/excalidraw/groups.ts b/packages/excalidraw/groups.ts index cedc4af0f..61b493fd4 100644 --- a/packages/excalidraw/groups.ts +++ b/packages/excalidraw/groups.ts @@ -214,7 +214,10 @@ export const isSelectedViaGroup = ( ) => getSelectedGroupForElement(appState, element) != null; export const getSelectedGroupForElement = ( - appState: InteractiveCanvasAppState, + appState: Pick< + InteractiveCanvasAppState, + "editingGroupId" | "selectedGroupIds" + >, element: ExcalidrawElement, ) => element.groupIds