From ec2de7205f2293e7c3beaf3266dfca0f491471f0 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Fri, 27 Oct 2023 12:06:11 +0530 Subject: [PATCH] fix: don't update label position when dragging labelled arrows (#6891) * fix: don't update label position when dragging labelled arrows * lint * add test * don't update coords for label when labelled arrow inside frame * increase locales bundle size limit --- src/element/dragElements.ts | 23 ++++++++++++----------- src/packages/excalidraw/.size-limit.json | 2 +- src/tests/linearElementEditor.test.tsx | 24 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/element/dragElements.ts b/src/element/dragElements.ts index 54b46aa0..89f30bcb 100644 --- a/src/element/dragElements.ts +++ b/src/element/dragElements.ts @@ -8,7 +8,11 @@ import { getBoundTextElement } from "./textElement"; import { isSelectedViaGroup } from "../groups"; import { getGridPoint } from "../math"; import Scene from "../scene/Scene"; -import { isFrameElement } from "./typeChecks"; +import { + isArrowElement, + isBoundToContainer, + isFrameElement, +} from "./typeChecks"; export const dragSelectedElements = ( pointerDownState: PointerDownState, @@ -35,6 +39,7 @@ export const dragSelectedElements = ( if (frames.length > 0) { const elementsInFrames = scene .getNonDeletedElements() + .filter((e) => !isBoundToContainer(e)) .filter((e) => e.frameId !== null) .filter((e) => frames.includes(e.frameId!)); @@ -58,20 +63,16 @@ export const dragSelectedElements = ( // update coords of bound text only if we're dragging the container directly // (we don't drag the group that it's part of) if ( + // Don't update coords of arrow label since we calculate its position during render + !isArrowElement(element) && // container isn't part of any group // (perf optim so we don't check `isSelectedViaGroup()` in every case) - !element.groupIds.length || - // container is part of a group, but we're dragging the container directly - (appState.editingGroupId && !isSelectedViaGroup(appState, element)) + (!element.groupIds.length || + // container is part of a group, but we're dragging the container directly + (appState.editingGroupId && !isSelectedViaGroup(appState, element))) ) { const textElement = getBoundTextElement(element); - if ( - textElement && - // when container is added to a frame, so will its bound text - // so the text is already in `elementsToUpdate` and we should avoid - // updating its coords again - (!textElement.frameId || !frames.includes(textElement.frameId)) - ) { + if (textElement) { updateElementCoords(pointerDownState, textElement, adjustedOffset); } } diff --git a/src/packages/excalidraw/.size-limit.json b/src/packages/excalidraw/.size-limit.json index 242d0614..76c3be30 100644 --- a/src/packages/excalidraw/.size-limit.json +++ b/src/packages/excalidraw/.size-limit.json @@ -6,7 +6,7 @@ { "path": "dist/excalidraw-assets/locales", "name": "dist/excalidraw-assets/locales", - "limit": "270 kB" + "limit": "290 kB" }, { "path": "dist/excalidraw-assets/vendor-*.js", diff --git a/src/tests/linearElementEditor.test.tsx b/src/tests/linearElementEditor.test.tsx index 920ed884..9b11bfbd 100644 --- a/src/tests/linearElementEditor.test.tsx +++ b/src/tests/linearElementEditor.test.tsx @@ -1202,5 +1202,29 @@ describe("Test Linear Elements", () => { }), ); }); + + it("should not update label position when arrow dragged", () => { + createTwoPointerLinearElement("arrow"); + let arrow = h.elements[0] as ExcalidrawLinearElement; + createBoundTextElement(DEFAULT_TEXT, arrow); + let label = h.elements[1] as ExcalidrawTextElementWithContainer; + expect(arrow.x).toBe(20); + expect(arrow.y).toBe(20); + expect(label.x).toBe(0); + expect(label.y).toBe(0); + mouse.reset(); + mouse.select(arrow); + mouse.select(label); + mouse.downAt(arrow.x, arrow.y); + mouse.moveTo(arrow.x + 20, arrow.y + 30); + mouse.up(arrow.x + 20, arrow.y + 30); + + arrow = h.elements[0] as ExcalidrawLinearElement; + label = h.elements[1] as ExcalidrawTextElementWithContainer; + expect(arrow.x).toBe(80); + expect(arrow.y).toBe(100); + expect(label.x).toBe(0); + expect(label.y).toBe(0); + }); }); });