diff --git a/packages/element/src/binding.ts b/packages/element/src/binding.ts index 5c32e8c81d..1d39ce2f06 100644 --- a/packages/element/src/binding.ts +++ b/packages/element/src/binding.ts @@ -56,7 +56,6 @@ import { getBoundTextElement, handleBindTextResize } from "./textElement"; import { isArrowElement, isBindableElement, - isBindingElement, isBoundToContainer, isElbowArrow, isFixedPointBinding, @@ -1409,19 +1408,19 @@ const getLinearElementEdgeCoors = ( }; export const fixDuplicatedBindingsAfterDuplication = ( - newElements: ExcalidrawElement[], - oldIdToDuplicatedId: Map, - duplicatedElementsMap: NonDeletedSceneElementsMap, + duplicatedElements: ExcalidrawElement[], + origIdToDuplicateId: Map, + duplicateElementsMap: NonDeletedSceneElementsMap, ) => { - for (const element of newElements) { - if ("boundElements" in element && element.boundElements) { - Object.assign(element, { - boundElements: element.boundElements.reduce( + for (const duplicateElement of duplicatedElements) { + if ("boundElements" in duplicateElement && duplicateElement.boundElements) { + Object.assign(duplicateElement, { + boundElements: duplicateElement.boundElements.reduce( ( acc: Mutable>, binding, ) => { - const newBindingId = oldIdToDuplicatedId.get(binding.id); + const newBindingId = origIdToDuplicateId.get(binding.id); if (newBindingId) { acc.push({ ...binding, id: newBindingId }); } @@ -1432,46 +1431,47 @@ export const fixDuplicatedBindingsAfterDuplication = ( }); } - if ("containerId" in element && element.containerId) { - Object.assign(element, { - containerId: oldIdToDuplicatedId.get(element.containerId) ?? null, + if ("containerId" in duplicateElement && duplicateElement.containerId) { + Object.assign(duplicateElement, { + containerId: + origIdToDuplicateId.get(duplicateElement.containerId) ?? null, }); } - if ("endBinding" in element && element.endBinding) { - const newEndBindingId = oldIdToDuplicatedId.get( - element.endBinding.elementId, + if ("endBinding" in duplicateElement && duplicateElement.endBinding) { + const newEndBindingId = origIdToDuplicateId.get( + duplicateElement.endBinding.elementId, ); - Object.assign(element, { + Object.assign(duplicateElement, { endBinding: newEndBindingId ? { - ...element.endBinding, + ...duplicateElement.endBinding, elementId: newEndBindingId, } : null, }); } - if ("startBinding" in element && element.startBinding) { - const newEndBindingId = oldIdToDuplicatedId.get( - element.startBinding.elementId, + if ("startBinding" in duplicateElement && duplicateElement.startBinding) { + const newEndBindingId = origIdToDuplicateId.get( + duplicateElement.startBinding.elementId, ); - Object.assign(element, { + Object.assign(duplicateElement, { startBinding: newEndBindingId ? { - ...element.startBinding, + ...duplicateElement.startBinding, elementId: newEndBindingId, } : null, }); } - if (isElbowArrow(element)) { + if (isElbowArrow(duplicateElement)) { Object.assign( - element, - updateElbowArrowPoints(element, duplicatedElementsMap, { + duplicateElement, + updateElbowArrowPoints(duplicateElement, duplicateElementsMap, { points: [ - element.points[0], - element.points[element.points.length - 1], + duplicateElement.points[0], + duplicateElement.points[duplicateElement.points.length - 1], ], }), ); @@ -1479,196 +1479,6 @@ export const fixDuplicatedBindingsAfterDuplication = ( } }; -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 ded1fd26c1..c2cee4c089 100644 --- a/packages/element/src/duplicate.ts +++ b/packages/element/src/duplicate.ts @@ -36,10 +36,7 @@ import { import { getBoundTextElement, getContainerElement } from "./textElement"; -import { - fixDuplicatedBindingsAfterDuplication, - fixReversedBindings, -} from "./binding"; +import { fixDuplicatedBindingsAfterDuplication } from "./binding"; import type { ElementsMap, @@ -60,16 +57,14 @@ import type { * multiple elements at once, share this map * amongst all of them * @param element Element to duplicate - * @param overrides Any element properties to override */ export const duplicateElement = ( editingGroupId: AppState["editingGroupId"], groupIdMapForOperation: Map, element: TElement, - overrides?: Partial, randomizeSeed?: boolean, ): Readonly => { - let copy = deepCopyElement(element); + const copy = deepCopyElement(element); if (isTestEnv()) { __test__defineOrigId(copy, element.id); @@ -92,9 +87,6 @@ export const duplicateElement = ( return groupIdMapForOperation.get(groupId)!; }, ); - if (overrides) { - copy = Object.assign(copy, overrides); - } return copy; }; @@ -102,9 +94,14 @@ export const duplicateElements = ( opts: { elements: readonly ExcalidrawElement[]; randomizeSeed?: boolean; - overrides?: ( - originalElement: ExcalidrawElement, - ) => Partial; + overrides?: (data: { + duplicateElement: ExcalidrawElement; + origElement: ExcalidrawElement; + origIdToDuplicateId: Map< + ExcalidrawElement["id"], + ExcalidrawElement["id"] + >; + }) => Partial; } & ( | { /** @@ -132,14 +129,6 @@ export const duplicateElements = ( 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; } ), ) => { @@ -153,8 +142,6 @@ export const duplicateElements = ( 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 @@ -167,10 +154,17 @@ export const duplicateElements = ( // loop over them. const processedIds = new Map(); const groupIdMap = new Map(); - const newElements: ExcalidrawElement[] = []; - const oldElements: ExcalidrawElement[] = []; - const oldIdToDuplicatedId = new Map(); - const duplicatedElementsMap = new Map(); + const duplicatedElements: ExcalidrawElement[] = []; + const origElements: ExcalidrawElement[] = []; + const origIdToDuplicateId = new Map< + ExcalidrawElement["id"], + ExcalidrawElement["id"] + >(); + const duplicateIdToOrigElement = new Map< + ExcalidrawElement["id"], + ExcalidrawElement + >(); + const duplicateElementsMap = new Map(); const elementsMap = arrayToMap(elements) as ElementsMap; const _idsOfElementsToDuplicate = opts.type === "in-place" @@ -188,7 +182,7 @@ export const duplicateElements = ( elements = normalizeElementOrder(elements); - const elementsWithClones: ExcalidrawElement[] = elements.slice(); + const elementsWithDuplicates: ExcalidrawElement[] = elements.slice(); // helper functions // ------------------------------------------------------------------------- @@ -214,17 +208,17 @@ export const duplicateElements = ( appState.editingGroupId, groupIdMap, element, - opts.overrides?.(element), opts.randomizeSeed, ); processedIds.set(newElement.id, true); - duplicatedElementsMap.set(newElement.id, newElement); - oldIdToDuplicatedId.set(element.id, newElement.id); + duplicateElementsMap.set(newElement.id, newElement); + origIdToDuplicateId.set(element.id, newElement.id); + duplicateIdToOrigElement.set(newElement.id, element); - oldElements.push(element); - newElements.push(newElement); + origElements.push(element); + duplicatedElements.push(newElement); acc.push(newElement); return acc; @@ -248,21 +242,12 @@ export const duplicateElements = ( return; } - if (reverseOrder && index < 1) { - elementsWithClones.unshift(...castArray(elements)); + if (index > elementsWithDuplicates.length - 1) { + elementsWithDuplicates.push(...castArray(elements)); return; } - if (!reverseOrder && index > elementsWithClones.length - 1) { - elementsWithClones.push(...castArray(elements)); - return; - } - - elementsWithClones.splice( - index + (reverseOrder ? 0 : 1), - 0, - ...castArray(elements), - ); + elementsWithDuplicates.splice(index + 1, 0, ...castArray(elements)); }; const frameIdsToDuplicate = new Set( @@ -294,13 +279,9 @@ export const duplicateElements = ( : [element], ); - const targetIndex = reverseOrder - ? elementsWithClones.findIndex((el) => { - return el.groupIds?.includes(groupId); - }) - : findLastIndex(elementsWithClones, (el) => { - return el.groupIds?.includes(groupId); - }); + const targetIndex = findLastIndex(elementsWithDuplicates, (el) => { + return el.groupIds?.includes(groupId); + }); insertBeforeOrAfterIndex(targetIndex, copyElements(groupElements)); continue; @@ -318,7 +299,7 @@ export const duplicateElements = ( const frameChildren = getFrameChildren(elements, frameId); - const targetIndex = findLastIndex(elementsWithClones, (el) => { + const targetIndex = findLastIndex(elementsWithDuplicates, (el) => { return el.frameId === frameId || el.id === frameId; }); @@ -335,7 +316,7 @@ export const duplicateElements = ( if (hasBoundTextElement(element)) { const boundTextElement = getBoundTextElement(element, elementsMap); - const targetIndex = findLastIndex(elementsWithClones, (el) => { + const targetIndex = findLastIndex(elementsWithDuplicates, (el) => { return ( el.id === element.id || ("containerId" in el && el.containerId === element.id) @@ -344,7 +325,7 @@ export const duplicateElements = ( if (boundTextElement) { insertBeforeOrAfterIndex( - targetIndex + (reverseOrder ? -1 : 0), + targetIndex, copyElements([element, boundTextElement]), ); } else { @@ -357,7 +338,7 @@ export const duplicateElements = ( if (isBoundToContainer(element)) { const container = getContainerElement(element, elementsMap); - const targetIndex = findLastIndex(elementsWithClones, (el) => { + const targetIndex = findLastIndex(elementsWithDuplicates, (el) => { return el.id === element.id || el.id === container?.id; }); @@ -377,7 +358,7 @@ export const duplicateElements = ( // ------------------------------------------------------------------------- insertBeforeOrAfterIndex( - findLastIndex(elementsWithClones, (el) => el.id === element.id), + findLastIndex(elementsWithDuplicates, (el) => el.id === element.id), copyElements(element), ); } @@ -385,28 +366,38 @@ export const duplicateElements = ( // --------------------------------------------------------------------------- fixDuplicatedBindingsAfterDuplication( - newElements, - oldIdToDuplicatedId, - duplicatedElementsMap as NonDeletedSceneElementsMap, + duplicatedElements, + origIdToDuplicateId, + duplicateElementsMap as NonDeletedSceneElementsMap, ); - if (reverseOrder) { - fixReversedBindings( - _idsOfElementsToDuplicate, - elementsWithClones, - oldIdToDuplicatedId, - ); - } - bindElementsToFramesAfterDuplication( - elementsWithClones, - oldElements, - oldIdToDuplicatedId, + elementsWithDuplicates, + origElements, + origIdToDuplicateId, ); + if (opts.overrides) { + for (const duplicateElement of duplicatedElements) { + const origElement = duplicateIdToOrigElement.get(duplicateElement.id); + if (origElement) { + Object.assign( + duplicateElement, + opts.overrides({ + duplicateElement, + origElement, + origIdToDuplicateId, + }), + ); + } + } + } + return { - newElements, - elementsWithClones, + duplicatedElements, + duplicateElementsMap, + elementsWithDuplicates, + origIdToDuplicateId, }; }; diff --git a/packages/element/src/frame.ts b/packages/element/src/frame.ts index 7f40148b7a..664c8d98fc 100644 --- a/packages/element/src/frame.ts +++ b/packages/element/src/frame.ts @@ -41,30 +41,28 @@ import type { // --------------------------- Frame State ------------------------------------ export const bindElementsToFramesAfterDuplication = ( nextElements: readonly ExcalidrawElement[], - oldElements: readonly ExcalidrawElement[], - oldIdToDuplicatedId: Map, + origElements: readonly ExcalidrawElement[], + origIdToDuplicateId: Map, ) => { const nextElementMap = arrayToMap(nextElements) as Map< ExcalidrawElement["id"], ExcalidrawElement >; - for (const element of oldElements) { + for (const element of origElements) { if (element.frameId) { // use its frameId to get the new frameId - const nextElementId = oldIdToDuplicatedId.get(element.id); - const nextFrameId = oldIdToDuplicatedId.get(element.frameId); - if (nextElementId) { - const nextElement = nextElementMap.get(nextElementId); - if (nextElement) { - mutateElement( - nextElement, - { - frameId: nextFrameId ?? element.frameId, - }, - false, - ); - } + const nextElementId = origIdToDuplicateId.get(element.id); + const nextFrameId = origIdToDuplicateId.get(element.frameId); + const nextElement = nextElementId && nextElementMap.get(nextElementId); + if (nextElement) { + mutateElement( + nextElement, + { + frameId: nextFrameId ?? null, + }, + false, + ); } } } diff --git a/packages/element/src/selection.ts b/packages/element/src/selection.ts index a767039208..e07c96d383 100644 --- a/packages/element/src/selection.ts +++ b/packages/element/src/selection.ts @@ -7,13 +7,20 @@ import type { import { getElementAbsoluteCoords, getElementBounds } from "./bounds"; import { isElementInViewport } from "./sizeHelpers"; -import { isBoundToContainer, isFrameLikeElement } from "./typeChecks"; +import { + isBoundToContainer, + isFrameLikeElement, + isLinearElement, +} from "./typeChecks"; import { elementOverlapsWithFrame, getContainingFrame, getFrameChildren, } from "./frame"; +import { LinearElementEditor } from "./linearElementEditor"; +import { selectGroupsForSelectedElements } from "./groups"; + import type { ElementsMap, ElementsMapOrArray, @@ -254,3 +261,48 @@ export const makeNextSelectedElementIds = ( return nextSelectedElementIds; }; + +const _getLinearElementEditor = ( + targetElements: readonly ExcalidrawElement[], +) => { + const linears = targetElements.filter(isLinearElement); + if (linears.length === 1) { + const linear = linears[0]; + const boundElements = linear.boundElements?.map((def) => def.id) ?? []; + const onlySingleLinearSelected = targetElements.every( + (el) => el.id === linear.id || boundElements.includes(el.id), + ); + + if (onlySingleLinearSelected) { + return new LinearElementEditor(linear); + } + } + + return null; +}; + +export const getSelectionStateForElements = ( + targetElements: readonly ExcalidrawElement[], + allElements: readonly NonDeletedExcalidrawElement[], + appState: AppState, +) => { + return { + selectedLinearElement: _getLinearElementEditor(targetElements), + ...selectGroupsForSelectedElements( + { + editingGroupId: appState.editingGroupId, + selectedElementIds: excludeElementsInFramesFromSelection( + targetElements, + ).reduce((acc: Record, element) => { + if (!isBoundToContainer(element)) { + acc[element.id] = true; + } + return acc; + }, {}), + }, + allElements, + appState, + null, + ), + }; +}; diff --git a/packages/element/tests/duplicate.test.tsx b/packages/element/tests/duplicate.test.tsx index 7492bcc586..409200b6e6 100644 --- a/packages/element/tests/duplicate.test.tsx +++ b/packages/element/tests/duplicate.test.tsx @@ -67,7 +67,7 @@ describe("duplicating single elements", () => { points: [pointFrom(1, 2), pointFrom(3, 4)], }); - const copy = duplicateElement(null, new Map(), element, undefined, true); + const copy = duplicateElement(null, new Map(), element, true); assertCloneObjects(element, copy); @@ -173,7 +173,7 @@ describe("duplicating multiple elements", () => { // ------------------------------------------------------------------------- const origElements = [rectangle1, text1, arrow1, arrow2, text2] as const; - const { newElements: clonedElements } = duplicateElements({ + const { duplicatedElements } = duplicateElements({ type: "everything", elements: origElements, }); @@ -181,10 +181,10 @@ describe("duplicating multiple elements", () => { // generic id in-equality checks // -------------------------------------------------------------------------- expect(origElements.map((e) => e.type)).toEqual( - clonedElements.map((e) => e.type), + duplicatedElements.map((e) => e.type), ); origElements.forEach((origElement, idx) => { - const clonedElement = clonedElements[idx]; + const clonedElement = duplicatedElements[idx]; expect(origElement).toEqual( expect.objectContaining({ id: expect.not.stringMatching(clonedElement.id), @@ -217,12 +217,12 @@ describe("duplicating multiple elements", () => { }); // -------------------------------------------------------------------------- - const clonedArrows = clonedElements.filter( + const clonedArrows = duplicatedElements.filter( (e) => e.type === "arrow", ) as ExcalidrawLinearElement[]; const [clonedRectangle, clonedText1, , clonedArrow2, clonedArrowLabel] = - clonedElements as any as typeof origElements; + duplicatedElements as any as typeof origElements; expect(clonedText1.containerId).toBe(clonedRectangle.id); expect( @@ -327,10 +327,10 @@ describe("duplicating multiple elements", () => { // ------------------------------------------------------------------------- const origElements = [rectangle1, text1, arrow1, arrow2, arrow3] as const; - const { newElements: clonedElements } = duplicateElements({ + const duplicatedElements = duplicateElements({ type: "everything", elements: origElements, - }) as any as { newElements: typeof origElements }; + }).duplicatedElements as any as typeof origElements; const [ clonedRectangle, @@ -338,7 +338,7 @@ describe("duplicating multiple elements", () => { clonedArrow1, clonedArrow2, clonedArrow3, - ] = clonedElements; + ] = duplicatedElements; expect(clonedRectangle.boundElements).toEqual([ { id: clonedArrow1.id, type: "arrow" }, @@ -374,12 +374,12 @@ describe("duplicating multiple elements", () => { }); const origElements = [rectangle1, rectangle2, rectangle3] as const; - const { newElements: clonedElements } = duplicateElements({ + const { duplicatedElements } = duplicateElements({ type: "everything", elements: origElements, - }) as any as { newElements: typeof origElements }; + }); const [clonedRectangle1, clonedRectangle2, clonedRectangle3] = - clonedElements; + duplicatedElements; expect(rectangle1.groupIds[0]).not.toBe(clonedRectangle1.groupIds[0]); expect(rectangle2.groupIds[0]).not.toBe(clonedRectangle2.groupIds[0]); @@ -399,7 +399,7 @@ describe("duplicating multiple elements", () => { }); const { - newElements: [clonedRectangle1], + duplicatedElements: [clonedRectangle1], } = duplicateElements({ type: "everything", elements: [rectangle1] }); expect(typeof clonedRectangle1.groupIds[0]).toBe("string"); @@ -408,6 +408,117 @@ describe("duplicating multiple elements", () => { }); }); +describe("group-related duplication", () => { + beforeEach(async () => { + await render(); + }); + + it("action-duplicating within group", async () => { + const rectangle1 = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + groupIds: ["group1"], + }); + const rectangle2 = API.createElement({ + type: "rectangle", + x: 10, + y: 10, + groupIds: ["group1"], + }); + + API.setElements([rectangle1, rectangle2]); + API.setSelectedElements([rectangle2], "group1"); + + act(() => { + h.app.actionManager.executeAction(actionDuplicateSelection); + }); + + assertElements(h.elements, [ + { id: rectangle1.id }, + { id: rectangle2.id }, + { [ORIG_ID]: rectangle2.id, selected: true, groupIds: ["group1"] }, + ]); + expect(h.state.editingGroupId).toBe("group1"); + }); + + it("alt-duplicating within group", async () => { + const rectangle1 = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + groupIds: ["group1"], + }); + const rectangle2 = API.createElement({ + type: "rectangle", + x: 10, + y: 10, + groupIds: ["group1"], + }); + + API.setElements([rectangle1, rectangle2]); + API.setSelectedElements([rectangle2], "group1"); + + Keyboard.withModifierKeys({ alt: true }, () => { + mouse.down(rectangle2.x + 5, rectangle2.y + 5); + mouse.up(rectangle2.x + 50, rectangle2.y + 50); + }); + + assertElements(h.elements, [ + { id: rectangle1.id }, + { id: rectangle2.id }, + { [ORIG_ID]: rectangle2.id, selected: true, groupIds: ["group1"] }, + ]); + expect(h.state.editingGroupId).toBe("group1"); + }); + + it.skip("alt-duplicating within group away outside frame", () => { + const frame = API.createElement({ + type: "frame", + x: 0, + y: 0, + width: 100, + height: 100, + }); + const rectangle1 = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + width: 50, + height: 50, + groupIds: ["group1"], + frameId: frame.id, + }); + const rectangle2 = API.createElement({ + type: "rectangle", + x: 10, + y: 10, + width: 50, + height: 50, + groupIds: ["group1"], + frameId: frame.id, + }); + + API.setElements([frame, rectangle1, rectangle2]); + API.setSelectedElements([rectangle2], "group1"); + + Keyboard.withModifierKeys({ alt: true }, () => { + mouse.down(rectangle2.x + 5, rectangle2.y + 5); + mouse.up(frame.x + frame.width + 50, frame.y + frame.height + 50); + }); + + // console.log(h.elements); + + assertElements(h.elements, [ + { id: frame.id }, + { id: rectangle1.id, frameId: frame.id }, + { id: rectangle2.id, frameId: frame.id }, + { [ORIG_ID]: rectangle2.id, selected: true, groupIds: [], frameId: null }, + ]); + expect(h.state.editingGroupId).toBe(null); + }); +}); + describe("duplication z-order", () => { beforeEach(async () => { await render(); @@ -503,8 +614,8 @@ describe("duplication z-order", () => { }); assertElements(h.elements, [ - { [ORIG_ID]: rectangle1.id }, - { id: rectangle1.id, selected: true }, + { id: rectangle1.id }, + { [ORIG_ID]: rectangle1.id, selected: true }, { id: rectangle2.id }, { id: rectangle3.id }, ]); @@ -538,8 +649,8 @@ describe("duplication z-order", () => { assertElements(h.elements, [ { id: rectangle1.id }, { id: rectangle2.id }, - { [ORIG_ID]: rectangle3.id }, - { id: rectangle3.id, selected: true }, + { id: rectangle3.id }, + { [ORIG_ID]: rectangle3.id, selected: true }, ]); }); @@ -569,8 +680,8 @@ describe("duplication z-order", () => { }); assertElements(h.elements, [ - { [ORIG_ID]: rectangle1.id }, - { id: rectangle1.id, selected: true }, + { id: rectangle1.id }, + { [ORIG_ID]: rectangle1.id, selected: true }, { id: rectangle2.id }, { id: rectangle3.id }, ]); @@ -605,19 +716,19 @@ describe("duplication z-order", () => { }); assertElements(h.elements, [ - { [ORIG_ID]: rectangle1.id }, - { [ORIG_ID]: rectangle2.id }, - { [ORIG_ID]: rectangle3.id }, - { id: rectangle1.id, selected: true }, - { id: rectangle2.id, selected: true }, - { id: rectangle3.id, selected: true }, + { id: rectangle1.id }, + { id: rectangle2.id }, + { id: rectangle3.id }, + { [ORIG_ID]: rectangle1.id, selected: true }, + { [ORIG_ID]: rectangle2.id, selected: true }, + { [ORIG_ID]: rectangle3.id, selected: true }, ]); }); - it("reverse-duplicating text container (in-order)", async () => { + it("alt-duplicating text container (in-order)", async () => { const [rectangle, text] = API.createTextContainer(); API.setElements([rectangle, text]); - API.setSelectedElements([rectangle, text]); + API.setSelectedElements([rectangle]); Keyboard.withModifierKeys({ alt: true }, () => { mouse.down(rectangle.x + 5, rectangle.y + 5); @@ -625,20 +736,20 @@ describe("duplication z-order", () => { }); assertElements(h.elements, [ - { [ORIG_ID]: rectangle.id }, + { id: rectangle.id }, + { id: text.id, containerId: rectangle.id }, + { [ORIG_ID]: rectangle.id, selected: true }, { [ORIG_ID]: text.id, containerId: getCloneByOrigId(rectangle.id)?.id, }, - { id: rectangle.id, selected: true }, - { id: text.id, containerId: rectangle.id, selected: true }, ]); }); - it("reverse-duplicating text container (out-of-order)", async () => { + it("alt-duplicating text container (out-of-order)", async () => { const [rectangle, text] = API.createTextContainer(); API.setElements([text, rectangle]); - API.setSelectedElements([rectangle, text]); + API.setSelectedElements([rectangle]); Keyboard.withModifierKeys({ alt: true }, () => { mouse.down(rectangle.x + 5, rectangle.y + 5); @@ -646,21 +757,21 @@ describe("duplication z-order", () => { }); assertElements(h.elements, [ - { [ORIG_ID]: rectangle.id }, + { id: rectangle.id }, + { id: text.id, containerId: rectangle.id }, + { [ORIG_ID]: rectangle.id, selected: true }, { [ORIG_ID]: text.id, containerId: getCloneByOrigId(rectangle.id)?.id, }, - { id: rectangle.id, selected: true }, - { id: text.id, containerId: rectangle.id, selected: true }, ]); }); - it("reverse-duplicating labeled arrows (in-order)", async () => { + it("alt-duplicating labeled arrows (in-order)", async () => { const [arrow, text] = API.createLabeledArrow(); API.setElements([arrow, text]); - API.setSelectedElements([arrow, text]); + API.setSelectedElements([arrow]); Keyboard.withModifierKeys({ alt: true }, () => { mouse.down(arrow.x + 5, arrow.y + 5); @@ -668,21 +779,24 @@ describe("duplication z-order", () => { }); assertElements(h.elements, [ - { [ORIG_ID]: arrow.id }, + { id: arrow.id }, + { id: text.id, containerId: arrow.id }, + { [ORIG_ID]: arrow.id, selected: true }, { [ORIG_ID]: text.id, containerId: getCloneByOrigId(arrow.id)?.id, }, - { id: arrow.id, selected: true }, - { id: text.id, containerId: arrow.id, selected: true }, ]); + expect(h.state.selectedLinearElement).toEqual( + expect.objectContaining({ elementId: getCloneByOrigId(arrow.id)?.id }), + ); }); - it("reverse-duplicating labeled arrows (out-of-order)", async () => { + it("alt-duplicating labeled arrows (out-of-order)", async () => { const [arrow, text] = API.createLabeledArrow(); API.setElements([text, arrow]); - API.setSelectedElements([arrow, text]); + API.setSelectedElements([arrow]); Keyboard.withModifierKeys({ alt: true }, () => { mouse.down(arrow.x + 5, arrow.y + 5); @@ -690,17 +804,17 @@ describe("duplication z-order", () => { }); assertElements(h.elements, [ - { [ORIG_ID]: arrow.id }, + { id: arrow.id }, + { id: text.id, containerId: arrow.id }, + { [ORIG_ID]: arrow.id, selected: true }, { [ORIG_ID]: text.id, containerId: getCloneByOrigId(arrow.id)?.id, }, - { id: arrow.id, selected: true }, - { id: text.id, containerId: arrow.id, selected: true }, ]); }); - it("reverse-duplicating bindable element with bound arrow should keep the arrow on the duplicate", () => { + it("alt-duplicating bindable element with bound arrow should keep the arrow on the duplicate", async () => { const rect = UI.createElement("rectangle", { x: 0, y: 0, @@ -722,11 +836,18 @@ describe("duplication z-order", () => { 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); + assertElements(h.elements, [ + { + id: rect.id, + boundElements: expect.arrayContaining([ + expect.objectContaining({ id: arrow.id }), + ]), + }, + { [ORIG_ID]: rect.id, boundElements: [], selected: true }, + { + id: arrow.id, + endBinding: expect.objectContaining({ elementId: rect.id }), + }, + ]); }); }); diff --git a/packages/excalidraw/actions/actionDuplicateSelection.tsx b/packages/excalidraw/actions/actionDuplicateSelection.tsx index 231fe1913b..17b452c365 100644 --- a/packages/excalidraw/actions/actionDuplicateSelection.tsx +++ b/packages/excalidraw/actions/actionDuplicateSelection.tsx @@ -7,26 +7,17 @@ import { import { getNonDeletedElements } from "@excalidraw/element"; -import { - isBoundToContainer, - isLinearElement, -} from "@excalidraw/element/typeChecks"; - import { LinearElementEditor } from "@excalidraw/element/linearElementEditor"; -import { selectGroupsForSelectedElements } from "@excalidraw/element/groups"; - import { - excludeElementsInFramesFromSelection, getSelectedElements, + getSelectionStateForElements, } from "@excalidraw/element/selection"; import { syncMovedIndices } from "@excalidraw/element/fractionalIndex"; import { duplicateElements } from "@excalidraw/element/duplicate"; -import type { ExcalidrawElement } from "@excalidraw/element/types"; - import { ToolButton } from "../components/ToolButton"; import { DuplicateIcon } from "../components/icons"; @@ -65,52 +56,49 @@ export const actionDuplicateSelection = register({ } } - let { newElements: duplicatedElements, elementsWithClones: nextElements } = - duplicateElements({ - type: "in-place", - 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, + let { duplicatedElements, elementsWithDuplicates } = duplicateElements({ + type: "in-place", + elements, + idsOfElementsToDuplicate: arrayToMap( + getSelectedElements(elements, appState, { + includeBoundTextElement: true, + includeElementsInFrames: true, }), - reverseOrder: false, - }); + ), + appState, + randomizeSeed: true, + overrides: ({ origElement, origIdToDuplicateId }) => { + const duplicateFrameId = + origElement.frameId && origIdToDuplicateId.get(origElement.frameId); + return { + x: origElement.x + DEFAULT_GRID_SIZE / 2, + y: origElement.y + DEFAULT_GRID_SIZE / 2, + frameId: duplicateFrameId ?? origElement.frameId, + }; + }, + }); - if (app.props.onDuplicate && nextElements) { - const mappedElements = app.props.onDuplicate(nextElements, elements); + if (app.props.onDuplicate && elementsWithDuplicates) { + const mappedElements = app.props.onDuplicate( + elementsWithDuplicates, + elements, + ); if (mappedElements) { - nextElements = mappedElements; + elementsWithDuplicates = mappedElements; } } return { - elements: syncMovedIndices(nextElements, arrayToMap(duplicatedElements)), + elements: syncMovedIndices( + elementsWithDuplicates, + arrayToMap(duplicatedElements), + ), appState: { ...appState, - ...updateLinearElementEditors(duplicatedElements), - ...selectGroupsForSelectedElements( - { - editingGroupId: appState.editingGroupId, - selectedElementIds: excludeElementsInFramesFromSelection( - duplicatedElements, - ).reduce((acc: Record, element) => { - if (!isBoundToContainer(element)) { - acc[element.id] = true; - } - return acc; - }, {}), - }, - getNonDeletedElements(nextElements), + ...getSelectionStateForElements( + duplicatedElements, + getNonDeletedElements(elementsWithDuplicates), appState, - null, ), }, captureUpdate: CaptureUpdateAction.IMMEDIATELY, @@ -130,24 +118,3 @@ export const actionDuplicateSelection = register({ /> ), }); - -const updateLinearElementEditors = (clonedElements: ExcalidrawElement[]) => { - const linears = clonedElements.filter(isLinearElement); - if (linears.length === 1) { - const linear = linears[0]; - const boundElements = linear.boundElements?.map((def) => def.id) ?? []; - const onlySingleLinearSelected = clonedElements.every( - (el) => el.id === linear.id || boundElements.includes(el.id), - ); - - if (onlySingleLinearSelected) { - return { - selectedLinearElement: new LinearElementEditor(linear), - }; - } - } - - return { - selectedLinearElement: null, - }; -}; diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 242fd8e46e..6a7306422b 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -279,6 +279,7 @@ import { import { excludeElementsInFramesFromSelection, + getSelectionStateForElements, makeNextSelectedElementIds, } from "@excalidraw/element/selection"; @@ -3267,7 +3268,7 @@ class App extends React.Component { const [gridX, gridY] = getGridPoint(dx, dy, this.getEffectiveGridSize()); - const { newElements } = duplicateElements({ + const { duplicatedElements } = duplicateElements({ type: "everything", elements: elements.map((element) => { return newElementWith(element, { @@ -3279,7 +3280,7 @@ class App extends React.Component { }); const prevElements = this.scene.getElementsIncludingDeleted(); - let nextElements = [...prevElements, ...newElements]; + let nextElements = [...prevElements, ...duplicatedElements]; const mappedNewSceneElements = this.props.onDuplicate?.( nextElements, @@ -3288,13 +3289,13 @@ class App extends React.Component { nextElements = mappedNewSceneElements || nextElements; - syncMovedIndices(nextElements, arrayToMap(newElements)); + syncMovedIndices(nextElements, arrayToMap(duplicatedElements)); const topLayerFrame = this.getTopLayerFrameAtSceneCoords({ x, y }); if (topLayerFrame) { const eligibleElements = filterElementsEligibleAsFrameChildren( - newElements, + duplicatedElements, topLayerFrame, ); addElementsToFrame( @@ -3307,7 +3308,7 @@ class App extends React.Component { this.scene.replaceAllElements(nextElements); - newElements.forEach((newElement) => { + duplicatedElements.forEach((newElement) => { if (isTextElement(newElement) && isBoundToContainer(newElement)) { const container = getContainerElement( newElement, @@ -3323,7 +3324,7 @@ class App extends React.Component { // paste event may not fire FontFace loadingdone event in Safari, hence loading font faces manually if (isSafari) { - Fonts.loadElementsFonts(newElements).then((fontFaces) => { + Fonts.loadElementsFonts(duplicatedElements).then((fontFaces) => { this.fonts.onLoaded(fontFaces); }); } @@ -3335,7 +3336,7 @@ class App extends React.Component { this.store.shouldCaptureIncrement(); const nextElementsToSelect = - excludeElementsInFramesFromSelection(newElements); + excludeElementsInFramesFromSelection(duplicatedElements); this.setState( { @@ -3378,7 +3379,7 @@ class App extends React.Component { this.setActiveTool({ type: "selection" }); if (opts.fitToContent) { - this.scrollToContent(newElements, { + this.scrollToContent(duplicatedElements, { fitToContent: true, canvasOffsets: this.getEditorUIOffsets(), }); @@ -6942,6 +6943,7 @@ class App extends React.Component { drag: { hasOccurred: false, offset: null, + origin: { ...origin }, }, eventListeners: { onMove: null, @@ -8236,8 +8238,8 @@ class App extends React.Component { this.state.activeEmbeddable?.state !== "active" ) { const dragOffset = { - x: pointerCoords.x - pointerDownState.origin.x, - y: pointerCoords.y - pointerDownState.origin.y, + x: pointerCoords.x - pointerDownState.drag.origin.x, + y: pointerCoords.y - pointerDownState.drag.origin.y, }; const originalElements = [ @@ -8432,52 +8434,112 @@ class App extends React.Component { selectedElements.map((el) => [el.id, el]), ); - const { newElements: clonedElements, elementsWithClones } = - duplicateElements({ - type: "in-place", - elements, - appState: this.state, - randomizeSeed: true, - idsOfElementsToDuplicate, - overrides: (el) => { - const origEl = pointerDownState.originalElements.get(el.id); - - if (origEl) { - return { - x: origEl.x, - y: origEl.y, - seed: origEl.seed, - }; - } - - return {}; - }, - reverseOrder: true, - }); - clonedElements.forEach((element) => { - pointerDownState.originalElements.set(element.id, element); + const { + duplicatedElements, + duplicateElementsMap, + elementsWithDuplicates, + origIdToDuplicateId, + } = duplicateElements({ + type: "in-place", + elements, + appState: this.state, + randomizeSeed: true, + idsOfElementsToDuplicate, + overrides: ({ duplicateElement, origElement }) => { + return { + // reset to the original element's frameId (unless we've + // duplicated alongside a frame in which case we need to + // keep the duplicate frame's id) so that the element + // frame membership is refreshed on pointerup + // NOTE this is a hacky solution and should be done + // differently + frameId: duplicateElement.frameId ?? origElement.frameId, + seed: randomInteger(), + }; + }, + }); + duplicatedElements.forEach((element) => { + pointerDownState.originalElements.set( + element.id, + deepCopyElement(element), + ); }); - const mappedNewSceneElements = this.props.onDuplicate?.( - elementsWithClones, - elements, - ); - - const nextSceneElements = syncMovedIndices( - mappedNewSceneElements || elementsWithClones, - arrayToMap(clonedElements), - ).map((el) => { + const mappedClonedElements = elementsWithDuplicates.map((el) => { if (idsOfElementsToDuplicate.has(el.id)) { - return newElementWith(el, { - seed: randomInteger(), - }); + const origEl = pointerDownState.originalElements.get(el.id); + + if (origEl) { + return newElementWith(el, { + x: origEl.x, + y: origEl.y, + }); + } } return el; }); - this.scene.replaceAllElements(nextSceneElements); - this.maybeCacheVisibleGaps(event, selectedElements, true); - this.maybeCacheReferenceSnapPoints(event, selectedElements, true); + const mappedNewSceneElements = this.props.onDuplicate?.( + mappedClonedElements, + elements, + ); + + const elementsWithIndices = syncMovedIndices( + mappedNewSceneElements || mappedClonedElements, + arrayToMap(duplicatedElements), + ); + + // we need to update synchronously so as to keep pointerDownState, + // appState, and scene elements in sync + flushSync(() => { + // swap hit element with the duplicated one + if (pointerDownState.hit.element) { + const cloneId = origIdToDuplicateId.get( + pointerDownState.hit.element.id, + ); + const clonedElement = + cloneId && duplicateElementsMap.get(cloneId); + pointerDownState.hit.element = clonedElement || null; + } + // swap hit elements with the duplicated ones + pointerDownState.hit.allHitElements = + pointerDownState.hit.allHitElements.reduce( + ( + acc: typeof pointerDownState.hit.allHitElements, + origHitElement, + ) => { + const cloneId = origIdToDuplicateId.get(origHitElement.id); + const clonedElement = + cloneId && duplicateElementsMap.get(cloneId); + if (clonedElement) { + acc.push(clonedElement); + } + + return acc; + }, + [], + ); + + // update drag origin to the position at which we started + // the duplication so that the drag offset is correct + pointerDownState.drag.origin = viewportCoordsToSceneCoords( + event, + this.state, + ); + + // switch selected elements to the duplicated ones + this.setState((prevState) => ({ + ...getSelectionStateForElements( + duplicatedElements, + this.scene.getNonDeletedElements(), + prevState, + ), + })); + + this.scene.replaceAllElements(elementsWithIndices); + this.maybeCacheVisibleGaps(event, selectedElements, true); + this.maybeCacheReferenceSnapPoints(event, selectedElements, true); + }); } return; diff --git a/packages/excalidraw/components/LibraryMenuItems.tsx b/packages/excalidraw/components/LibraryMenuItems.tsx index f70315953d..160cc26405 100644 --- a/packages/excalidraw/components/LibraryMenuItems.tsx +++ b/packages/excalidraw/components/LibraryMenuItems.tsx @@ -166,7 +166,7 @@ export default function LibraryMenuItems({ type: "everything", elements: item.elements, randomizeSeed: true, - }).newElements, + }).duplicatedElements, }; }); }, diff --git a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap index 4b863d4e78..5078a31a0e 100644 --- a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap @@ -1,40 +1,6 @@ // 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": "Zz", - "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": 1604849351, - "width": 30, - "x": 30, - "y": 20, -} -`; - -exports[`duplicate element on move when ALT is clicked > rectangle 6`] = ` { "angle": 0, "backgroundColor": "transparent", @@ -54,13 +20,47 @@ exports[`duplicate element on move when ALT is clicked > rectangle 6`] = ` "roundness": { "type": 3, }, - "seed": 1505387817, + "seed": 1278240551, "strokeColor": "#1e1e1e", "strokeStyle": "solid", "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 6, + "version": 5, + "versionNonce": 1505387817, + "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": "id2", + "index": "a1", + "isDeleted": false, + "link": null, + "locked": false, + "opacity": 100, + "roughness": 1, + "roundness": { + "type": 3, + }, + "seed": 1604849351, + "strokeColor": "#1e1e1e", + "strokeStyle": "solid", + "strokeWidth": 2, + "type": "rectangle", + "updated": 1, + "version": 7, "versionNonce": 915032327, "width": 30, "x": -10, diff --git a/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap index 319287792c..68d4d5d799 100644 --- a/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/regressionTests.test.tsx.snap @@ -2038,7 +2038,7 @@ exports[`regression tests > alt-drag duplicates an element > [end of test] appSt "scrolledOutside": false, "searchMatches": [], "selectedElementIds": { - "id0": true, + "id2": true, }, "selectedElementsAreBeingDragged": false, "selectedGroupIds": {}, @@ -2128,8 +2128,16 @@ History { HistoryEntry { "appStateChange": AppStateChange { "delta": Delta { - "deleted": {}, - "inserted": {}, + "deleted": { + "selectedElementIds": { + "id2": true, + }, + }, + "inserted": { + "selectedElementIds": { + "id0": true, + }, + }, }, }, "elementsChange": ElementsChange { @@ -2145,7 +2153,7 @@ History { "frameId": null, "groupIds": [], "height": 10, - "index": "Zz", + "index": "a1", "isDeleted": false, "link": null, "locked": false, @@ -2159,26 +2167,15 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 10, - "y": 10, + "x": 20, + "y": 20, }, "inserted": { "isDeleted": true, }, }, }, - "updated": Map { - "id0" => Delta { - "deleted": { - "x": 20, - "y": 20, - }, - "inserted": { - "x": 10, - "y": 10, - }, - }, - }, + "updated": Map {}, }, }, ], @@ -10378,13 +10375,13 @@ exports[`regression tests > make a group and duplicate it > [end of test] appSta "scrolledOutside": false, "searchMatches": [], "selectedElementIds": { - "id0": true, - "id1": true, - "id2": true, + "id6": true, + "id8": true, + "id9": true, }, "selectedElementsAreBeingDragged": false, "selectedGroupIds": { - "id4": true, + "id7": true, }, "selectedLinearElement": null, "selectionElement": null, @@ -10648,8 +10645,26 @@ History { HistoryEntry { "appStateChange": AppStateChange { "delta": Delta { - "deleted": {}, - "inserted": {}, + "deleted": { + "selectedElementIds": { + "id6": true, + "id8": true, + "id9": true, + }, + "selectedGroupIds": { + "id7": true, + }, + }, + "inserted": { + "selectedElementIds": { + "id0": true, + "id1": true, + "id2": true, + }, + "selectedGroupIds": { + "id4": true, + }, + }, }, }, "elementsChange": ElementsChange { @@ -10667,7 +10682,7 @@ History { "id7", ], "height": 10, - "index": "Zx", + "index": "a3", "isDeleted": false, "link": null, "locked": false, @@ -10681,8 +10696,8 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 10, - "y": 10, + "x": 20, + "y": 20, }, "inserted": { "isDeleted": true, @@ -10700,7 +10715,7 @@ History { "id7", ], "height": 10, - "index": "Zy", + "index": "a4", "isDeleted": false, "link": null, "locked": false, @@ -10714,8 +10729,8 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 30, - "y": 10, + "x": 40, + "y": 20, }, "inserted": { "isDeleted": true, @@ -10733,7 +10748,7 @@ History { "id7", ], "height": 10, - "index": "Zz", + "index": "a5", "isDeleted": false, "link": null, "locked": false, @@ -10747,46 +10762,15 @@ History { "strokeWidth": 2, "type": "rectangle", "width": 10, - "x": 50, - "y": 10, + "x": 60, + "y": 20, }, "inserted": { "isDeleted": true, }, }, }, - "updated": Map { - "id0" => Delta { - "deleted": { - "x": 20, - "y": 20, - }, - "inserted": { - "x": 10, - "y": 10, - }, - }, - "id1" => Delta { - "deleted": { - "x": 40, - "y": 20, - }, - "inserted": { - "x": 30, - "y": 10, - }, - }, - "id2" => Delta { - "deleted": { - "x": 60, - "y": 20, - }, - "inserted": { - "x": 50, - "y": 10, - }, - }, - }, + "updated": Map {}, }, }, ], diff --git a/packages/excalidraw/tests/clipboard.test.tsx b/packages/excalidraw/tests/clipboard.test.tsx index 0759afd94c..7d0e3906c0 100644 --- a/packages/excalidraw/tests/clipboard.test.tsx +++ b/packages/excalidraw/tests/clipboard.test.tsx @@ -307,6 +307,41 @@ describe("pasting & frames", () => { }); }); + it("should remove element from frame when pasted outside", async () => { + const frame = API.createElement({ + type: "frame", + width: 100, + height: 100, + x: 0, + y: 0, + }); + const rect = API.createElement({ + type: "rectangle", + frameId: frame.id, + x: 10, + y: 10, + width: 50, + height: 50, + }); + + API.setElements([frame]); + + const clipboardJSON = await serializeAsClipboardJSON({ + elements: [rect], + files: null, + }); + + mouse.moveTo(150, 150); + + pasteWithCtrlCmdV(clipboardJSON); + + await waitFor(() => { + expect(h.elements.length).toBe(2); + expect(h.elements[1].type).toBe(rect.type); + expect(h.elements[1].frameId).toBe(null); + }); + }); + it("should filter out elements not overlapping frame", async () => { const frame = API.createElement({ type: "frame", diff --git a/packages/excalidraw/tests/cropElement.test.tsx b/packages/excalidraw/tests/cropElement.test.tsx index 8011483fa8..8764962fe4 100644 --- a/packages/excalidraw/tests/cropElement.test.tsx +++ b/packages/excalidraw/tests/cropElement.test.tsx @@ -218,7 +218,7 @@ describe("Cropping and other features", async () => { initialHeight / 2, ]); Keyboard.keyDown(KEYS.ESCAPE); - const duplicatedImage = duplicateElement(null, new Map(), image, {}); + const duplicatedImage = duplicateElement(null, new Map(), image); act(() => { h.app.scene.insertElement(duplicatedImage); }); diff --git a/packages/excalidraw/types.ts b/packages/excalidraw/types.ts index 717993b436..4d3fe4fdc6 100644 --- a/packages/excalidraw/types.ts +++ b/packages/excalidraw/types.ts @@ -724,7 +724,8 @@ export type PointerDownState = Readonly<{ scrollbars: ReturnType; // The previous pointer position lastCoords: { x: number; y: number }; - // map of original elements data + // original element frozen snapshots so we can access the original + // element attribute values at time of pointerdown originalElements: Map>; resize: { // Handle when resizing, might change during the pointer interaction @@ -758,6 +759,9 @@ export type PointerDownState = Readonly<{ hasOccurred: boolean; // Might change during the pointer interaction offset: { x: number; y: number } | null; + // by default same as PointerDownState.origin. On alt-duplication, reset + // to current pointer position at time of duplication. + origin: { x: number; y: number }; }; // We need to have these in the state so that we can unsubscribe them eventListeners: {