From 703a8f0e78191b450347625a0109a64215cc2a34 Mon Sep 17 00:00:00 2001 From: Marcel Mraz Date: Tue, 8 Apr 2025 23:31:44 +0000 Subject: [PATCH] Separate mutation of elbow arrows from mutateElement --- packages/element/src/binding.ts | 17 ++- packages/element/src/elbowArrow.ts | 67 ++++++++++- packages/element/src/linearElementEditor.ts | 57 +++++++--- packages/element/src/mutateElement.ts | 60 +++------- packages/element/src/resizeElements.ts | 25 +++-- .../excalidraw/actions/actionBoundText.tsx | 20 +++- .../actions/actionDeleteSelected.tsx | 31 +++--- .../excalidraw/actions/actionFinalize.tsx | 24 +++- packages/excalidraw/components/App.tsx | 105 ++++++++++++------ .../components/Stats/MultiDimension.tsx | 11 +- packages/excalidraw/index.tsx | 2 + 11 files changed, 293 insertions(+), 126 deletions(-) diff --git a/packages/element/src/binding.ts b/packages/element/src/binding.ts index 7a67cf0a1..410d46624 100644 --- a/packages/element/src/binding.ts +++ b/packages/element/src/binding.ts @@ -66,7 +66,7 @@ import { } from "./typeChecks"; import { aabbForElement, getElementShape, pointInsideBounds } from "./shapes"; -import { updateElbowArrowPoints } from "./elbowArrow"; +import { mutateElbowArrow, updateElbowArrowPoints } from "./elbowArrow"; import type { Bounds } from "./bounds"; import type { ElementUpdate } from "./mutateElement"; @@ -796,7 +796,20 @@ export const updateBoundElements = ( // `linearElement` is being moved/scaled already, just update the binding if (simultaneouslyUpdatedElementIds.has(element.id)) { - mutateElement(element, bindings, true); + if (isElbowArrow(element)) { + mutateElbowArrow( + element, + bindings as { + startBinding: FixedPointBinding; + endBinding: FixedPointBinding; + }, + true, + elementsMap, + ); + } else { + mutateElement(element, bindings, true); + } + return; } diff --git a/packages/element/src/elbowArrow.ts b/packages/element/src/elbowArrow.ts index a70e265bc..f33fef8dd 100644 --- a/packages/element/src/elbowArrow.ts +++ b/packages/element/src/elbowArrow.ts @@ -22,6 +22,8 @@ import { isDevEnv, } from "@excalidraw/common"; +import type { Radians } from "@excalidraw/math"; + import type { AppState } from "@excalidraw/excalidraw/types"; import { @@ -45,8 +47,8 @@ import { vectorToHeading, headingForPoint, } from "./heading"; -import { type ElementUpdate } from "./mutateElement"; -import { isBindableElement } from "./typeChecks"; +import { mutateElement, type ElementUpdate } from "./mutateElement"; +import { isBindableElement, isElbowArrow } from "./typeChecks"; import { type ExcalidrawElbowArrowElement, type NonDeletedSceneElementsMap, @@ -61,6 +63,7 @@ import type { Arrowhead, ElementsMap, ExcalidrawBindableElement, + ExcalidrawElement, FixedPointBinding, FixedSegment, NonDeletedExcalidrawElement, @@ -879,6 +882,64 @@ const handleEndpointDrag = ( const MAX_POS = 1e6; +export const elbowArrowNeedsToGetNormalized = ( + element: Readonly, + updates: { + points?: readonly LocalPoint[]; + fixedSegments?: readonly FixedSegment[] | null; + startBinding?: FixedPointBinding | null; + endBinding?: FixedPointBinding | null; + }, +) => { + const { points, fixedSegments, startBinding, endBinding } = updates; + + return ( + isElbowArrow(element) && + (Object.keys(updates).length === 0 || // normalization case + typeof points !== "undefined" || // repositioning + typeof fixedSegments !== "undefined" || // segment fixing + typeof startBinding !== "undefined" || + typeof endBinding !== "undefined") // manual binding to element + ); +}; + +/** + * Mutates an elbow arrow element and renormalizes it's properties if necessary. + */ +export const mutateElbowArrow = ( + element: Readonly, + updates: ElementUpdate, + informMutation: boolean = true, + elementsMap: NonDeletedSceneElementsMap | SceneElementsMap | ElementsMap, + options?: { + isDragging?: boolean; + }, +): ElementUpdate => { + invariant( + !isElbowArrow(element), + `Element "${element.type}" is not an elbow arrow! Use \`mutateElement\` instead`, + ); + + if (!elbowArrowNeedsToGetNormalized(element, updates)) { + return mutateElement(element, updates, informMutation); + } + + return mutateElement( + element, + { + ...updates, + angle: 0 as Radians, + ...updateElbowArrowPoints( + element, + elementsMap as NonDeletedSceneElementsMap, + updates, + options, + ), + }, + informMutation, + ); +}; + /** * */ @@ -887,7 +948,7 @@ export const updateElbowArrowPoints = ( elementsMap: NonDeletedSceneElementsMap, updates: { points?: readonly LocalPoint[]; - fixedSegments?: FixedSegment[] | null; + fixedSegments?: readonly FixedSegment[] | null; startBinding?: FixedPointBinding | null; endBinding?: FixedPointBinding | null; }, diff --git a/packages/element/src/linearElementEditor.ts b/packages/element/src/linearElementEditor.ts index 8a9117bf8..002738710 100644 --- a/packages/element/src/linearElementEditor.ts +++ b/packages/element/src/linearElementEditor.ts @@ -22,7 +22,7 @@ import { // TODO: remove direct dependency on the scene, should be passed in or injected instead // eslint-disable-next-line @typescript-eslint/no-restricted-imports -import Scene from "@excalidraw/excalidraw/scene/Scene"; +import type Scene from "@excalidraw/excalidraw/scene/Scene"; import type { Store } from "@excalidraw/excalidraw/store"; @@ -50,7 +50,7 @@ import { getMinMaxXYFromCurvePathOps, } from "./bounds"; -import { updateElbowArrowPoints } from "./elbowArrow"; +import { mutateElbowArrow, updateElbowArrowPoints } from "./elbowArrow"; import { headingIsHorizontal, vectorToHeading } from "./heading"; import { bumpVersion, mutateElement } from "./mutateElement"; @@ -794,7 +794,10 @@ export class LinearElementEditor { elementsMap, ); } else if (event.altKey && appState.editingLinearElement) { - if (linearElementEditor.lastUncommittedPoint == null) { + if ( + linearElementEditor.lastUncommittedPoint == null && + !isElbowArrow(element) + ) { mutateElement(element, { points: [ ...element.points, @@ -1219,7 +1222,12 @@ export class LinearElementEditor { return acc; }, []); - mutateElement(element, { points: nextPoints }); + const updates = { points: nextPoints }; + if (isElbowArrow(element)) { + mutateElbowArrow(element, updates, true, elementsMap); + } else { + mutateElement(element, updates); + } // temp hack to ensure the line doesn't move when adding point to the end, // potentially expanding the bounding box @@ -1425,9 +1433,12 @@ export class LinearElementEditor { ...element.points.slice(segmentMidpoint.index!), ]; - mutateElement(element, { - points, - }); + const updates = { points }; + if (isElbowArrow(element)) { + mutateElbowArrow(element, updates, true, elementsMap); + } else { + mutateElement(element, updates); + } ret.pointerDownState = { ...linearElementEditor.pointerDownState, @@ -1479,8 +1490,8 @@ export class LinearElementEditor { updates.points = Array.from(nextPoints); - if (!options?.sceneElementsMap || Scene.getScene(element)) { - mutateElement(element, updates, true, { + if (!options?.sceneElementsMap) { + mutateElbowArrow(element, updates, true, options?.sceneElementsMap!, { isDragging: options?.isDragging, }); } else { @@ -1825,9 +1836,14 @@ export class LinearElementEditor { .map((segment) => segment.index) .reduce((count, idx) => (idx < index ? count + 1 : count), 0); - mutateElement(element, { - fixedSegments: nextFixedSegments, - }); + mutateElbowArrow( + element, + { + fixedSegments: nextFixedSegments, + }, + true, + elementsMap, + ); const point = pointFrom( element.x + @@ -1859,14 +1875,19 @@ export class LinearElementEditor { static deleteFixedSegment( element: ExcalidrawElbowArrowElement, + elementsMap: NonDeletedSceneElementsMap, index: number, ): void { - mutateElement(element, { - fixedSegments: element.fixedSegments?.filter( - (segment) => segment.index !== index, - ), - }); - mutateElement(element, {}, true); + mutateElbowArrow( + element, + { + fixedSegments: element.fixedSegments?.filter( + (segment) => segment.index !== index, + ), + }, + true, + elementsMap, + ); } } diff --git a/packages/element/src/mutateElement.ts b/packages/element/src/mutateElement.ts index d87007369..ebc5882ad 100644 --- a/packages/element/src/mutateElement.ts +++ b/packages/element/src/mutateElement.ts @@ -2,23 +2,20 @@ import { getSizeFromPoints, randomInteger, getUpdatedTimestamp, - toBrandedType, + invariant, } from "@excalidraw/common"; // TODO: remove direct dependency on the scene, should be passed in or injected instead // eslint-disable-next-line @typescript-eslint/no-restricted-imports import Scene from "@excalidraw/excalidraw/scene/Scene"; -import type { Radians } from "@excalidraw/math"; - import type { Mutable } from "@excalidraw/common/utility-types"; import { ShapeCache } from "./ShapeCache"; -import { updateElbowArrowPoints } from "./elbowArrow"; -import { isElbowArrow } from "./typeChecks"; +import { elbowArrowNeedsToGetNormalized } from "./elbowArrow"; -import type { ExcalidrawElement, NonDeletedSceneElementsMap } from "./types"; +import type { ExcalidrawElement } from "./types"; export type ElementUpdate = Omit< Partial, @@ -33,54 +30,25 @@ export const mutateElement = >( element: TElement, updates: ElementUpdate, informMutation = true, - options?: { - // Currently only for elbow arrows. - // If true, the elbow arrow tries to bind to the nearest element. If false - // it tries to keep the same bound element, if any. - isDragging?: boolean; - }, ): TElement => { let didChange = false; // casting to any because can't use `in` operator // (see https://github.com/microsoft/TypeScript/issues/21732) - const { points, fixedSegments, fileId, startBinding, endBinding } = + const { points, fileId, fixedSegments, startBinding, endBinding } = updates as any; - if ( - isElbowArrow(element) && - (Object.keys(updates).length === 0 || // normalization case - typeof points !== "undefined" || // repositioning - typeof fixedSegments !== "undefined" || // segment fixing - typeof startBinding !== "undefined" || - typeof endBinding !== "undefined") // manual binding to element - ) { - const elementsMap = toBrandedType( - Scene.getScene(element)?.getNonDeletedElementsMap() ?? new Map(), - ); + invariant( + elbowArrowNeedsToGetNormalized(element, { + points, + fixedSegments, + startBinding, + endBinding, + }), + "Elbow arrow should get normalized! Use `mutateElbowArrow` instead.", + ); - updates = { - ...updates, - angle: 0 as Radians, - ...updateElbowArrowPoints( - { - ...element, - x: updates.x || element.x, - y: updates.y || element.y, - }, - elementsMap, - { - fixedSegments, - points, - startBinding, - endBinding, - }, - { - isDragging: options?.isDragging, - }, - ), - }; - } else if (typeof points !== "undefined") { + if (typeof points !== "undefined") { updates = { ...getSizeFromPoints(points), ...updates }; } diff --git a/packages/element/src/resizeElements.ts b/packages/element/src/resizeElements.ts index 3ff405603..77d3df400 100644 --- a/packages/element/src/resizeElements.ts +++ b/packages/element/src/resizeElements.ts @@ -60,6 +60,8 @@ import { import { isInGroup } from "./groups"; +import { mutateElbowArrow } from "./elbowArrow"; + import type { BoundingBox } from "./bounds"; import type { MaybeTransformHandleType, @@ -545,9 +547,14 @@ const rotateMultipleElements = ( if (isElbowArrow(element)) { // Needed to re-route the arrow - mutateElement(element, { - points: getArrowLocalFixedPoints(element, elementsMap), - }); + mutateElbowArrow( + element, + { + points: getArrowLocalFixedPoints(element, elementsMap), + }, + false, + elementsMap, + ); } else { mutateElement( element, @@ -1527,10 +1534,14 @@ export const resizeMultipleElements = ( } of elementsAndUpdates) { const { width, height, angle } = update; - mutateElement(element, update, false, { - // needed for the fixed binding point udpate to take effect - isDragging: true, - }); + if (isElbowArrow(element)) { + mutateElbowArrow(element, update, false, elementsMap, { + // needed for the fixed binding point udpate to take effect + isDragging: true, + }); + } else { + mutateElement(element, update, false); + } updateBoundElements(element, elementsMap as SceneElementsMap, { simultaneouslyUpdated: elementsToUpdate, diff --git a/packages/excalidraw/actions/actionBoundText.tsx b/packages/excalidraw/actions/actionBoundText.tsx index ae18c0d98..bada6fc44 100644 --- a/packages/excalidraw/actions/actionBoundText.tsx +++ b/packages/excalidraw/actions/actionBoundText.tsx @@ -21,6 +21,7 @@ import { import { hasBoundTextElement, + isElbowArrow, isTextBindableContainer, isTextElement, isUsingAdaptiveRadius, @@ -33,11 +34,14 @@ import { syncMovedIndices } from "@excalidraw/element/fractionalIndex"; import { newElement } from "@excalidraw/element/newElement"; +import { mutateElbowArrow } from "@excalidraw/element/elbowArrow"; + import type { ExcalidrawElement, ExcalidrawLinearElement, ExcalidrawTextContainer, ExcalidrawTextElement, + FixedPointBinding, } from "@excalidraw/element/types"; import type { Mutable } from "@excalidraw/common/utility-types"; @@ -297,7 +301,21 @@ export const actionWrapTextInContainer = register({ } if (startBinding || endBinding) { - mutateElement(ele, { startBinding, endBinding }, false); + const updates = { startBinding, endBinding }; + + if (isElbowArrow(ele)) { + mutateElbowArrow( + ele, + updates as { + startBinding: FixedPointBinding; + endBinding: FixedPointBinding; + }, + false, + app.scene.getNonDeletedElementsMap(), + ); + } else { + mutateElement(ele, updates, false); + } } }); } diff --git a/packages/excalidraw/actions/actionDeleteSelected.tsx b/packages/excalidraw/actions/actionDeleteSelected.tsx index 75e666df0..d861390b3 100644 --- a/packages/excalidraw/actions/actionDeleteSelected.tsx +++ b/packages/excalidraw/actions/actionDeleteSelected.tsx @@ -3,10 +3,7 @@ import { KEYS, updateActiveTool } from "@excalidraw/common"; import { getNonDeletedElements } from "@excalidraw/element"; import { fixBindingsAfterDeletion } from "@excalidraw/element/binding"; import { LinearElementEditor } from "@excalidraw/element/linearElementEditor"; -import { - mutateElement, - newElementWith, -} from "@excalidraw/element/mutateElement"; +import { newElementWith } from "@excalidraw/element/mutateElement"; import { getContainerElement } from "@excalidraw/element/textElement"; import { isBoundToContainer, @@ -20,6 +17,8 @@ import { selectGroupsForSelectedElements, } from "@excalidraw/element/groups"; +import { mutateElbowArrow } from "@excalidraw/element/elbowArrow"; + import type { ExcalidrawElement } from "@excalidraw/element/types"; import { t } from "../i18n"; @@ -94,15 +93,21 @@ const deleteSelectedElements = ( el.boundElements.forEach((candidate) => { const bound = app.scene.getNonDeletedElementsMap().get(candidate.id); if (bound && isElbowArrow(bound)) { - mutateElement(bound, { - startBinding: - el.id === bound.startBinding?.elementId - ? null - : bound.startBinding, - endBinding: - el.id === bound.endBinding?.elementId ? null : bound.endBinding, - }); - mutateElement(bound, { points: bound.points }); + mutateElbowArrow( + bound, + { + startBinding: + el.id === bound.startBinding?.elementId + ? null + : bound.startBinding, + endBinding: + el.id === bound.endBinding?.elementId + ? null + : bound.endBinding, + }, + true, + app.scene.getNonDeletedElementsMap(), + ); } }); } diff --git a/packages/excalidraw/actions/actionFinalize.tsx b/packages/excalidraw/actions/actionFinalize.tsx index 984961656..f78592326 100644 --- a/packages/excalidraw/actions/actionFinalize.tsx +++ b/packages/excalidraw/actions/actionFinalize.tsx @@ -5,9 +5,12 @@ import { bindOrUnbindLinearElement, } from "@excalidraw/element/binding"; import { LinearElementEditor } from "@excalidraw/element/linearElementEditor"; + import { mutateElement } from "@excalidraw/element/mutateElement"; +import { mutateElbowArrow } from "@excalidraw/element/elbowArrow"; import { isBindingElement, + isElbowArrow, isLinearElement, } from "@excalidraw/element/typeChecks"; @@ -16,6 +19,13 @@ import { isPathALoop } from "@excalidraw/element/shapes"; import { isInvisiblySmallElement } from "@excalidraw/element/sizeHelpers"; +import type { + ExcalidrawElbowArrowElement, + ExcalidrawElement, +} from "@excalidraw/element/types"; + +import type { ElementUpdate } from "@excalidraw/element/mutateElement"; + import { t } from "../i18n"; import { resetCursor } from "../cursor"; import { done } from "../components/icons"; @@ -85,6 +95,16 @@ export const actionFinalize = register({ ? appState.newElement : null; + const mutate = (updates: ElementUpdate) => + isElbowArrow(multiPointElement as ExcalidrawElbowArrowElement) + ? mutateElbowArrow( + multiPointElement as ExcalidrawElbowArrowElement, + updates, + true, + elementsMap, + ) + : mutateElement(multiPointElement as ExcalidrawElement, updates); + if (multiPointElement) { // pen and mouse have hover if ( @@ -96,7 +116,7 @@ export const actionFinalize = register({ !lastCommittedPoint || points[points.length - 1] !== lastCommittedPoint ) { - mutateElement(multiPointElement, { + mutate({ points: multiPointElement.points.slice(0, -1), }); } @@ -120,7 +140,7 @@ export const actionFinalize = register({ if (isLoop) { const linePoints = multiPointElement.points; const firstPoint = linePoints[0]; - mutateElement(multiPointElement, { + mutate({ points: linePoints.map((p, index) => index === linePoints.length - 1 ? pointFrom(firstPoint[0], firstPoint[1]) diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 276cde027..6bd547bce 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -302,6 +302,10 @@ import { import { isNonDeletedElement } from "@excalidraw/element"; +import { mutateElbowArrow } from "@excalidraw/element/elbowArrow"; + +import type { ElementUpdate } from "@excalidraw/element/mutateElement"; + import type { LocalPoint, Radians } from "@excalidraw/math"; import type { @@ -327,6 +331,7 @@ import type { MagicGenerationData, ExcalidrawNonSelectionElement, ExcalidrawArrowElement, + ExcalidrawElbowArrowElement, } from "@excalidraw/element/types"; import type { ValueOf } from "@excalidraw/common/utility-types"; @@ -5485,7 +5490,11 @@ class App extends React.Component { if (midPoint && midPoint > -1) { this.store.shouldCaptureIncrement(); - LinearElementEditor.deleteFixedSegment(selectedElements[0], midPoint); + LinearElementEditor.deleteFixedSegment( + selectedElements[0], + this.scene.getNonDeletedElementsMap(), + midPoint, + ); const nextCoords = LinearElementEditor.getSegmentMidpointHitCoords( { @@ -5991,23 +6000,30 @@ class App extends React.Component { if (isPathALoop(points, this.state.zoom.value)) { setCursor(this.interactiveCanvas, CURSOR_TYPE.POINTER); } - // update last uncommitted point - mutateElement( - multiElement, - { - points: [ - ...points.slice(0, -1), - pointFrom( - lastCommittedX + dxFromLastCommitted, - lastCommittedY + dyFromLastCommitted, - ), - ], - }, - false, - { - isDragging: true, - }, - ); + const updates = { + points: [ + ...points.slice(0, -1), + pointFrom( + lastCommittedX + dxFromLastCommitted, + lastCommittedY + dyFromLastCommitted, + ), + ], + }; + + if (isElbowArrow(multiElement)) { + mutateElbowArrow( + multiElement, + updates, + false, + this.scene.getNonDeletedElementsMap(), + { + isDragging: true, + }, + ); + } else { + // update last uncommitted point + mutateElement(multiElement, updates, false); + } // in this path, we're mutating multiElement to reflect // how it will be after adding pointer position as the next point @@ -8672,25 +8688,33 @@ class App extends React.Component { )); } + const mutate = ( + updates: ElementUpdate, + options: { isDragging?: boolean } = {}, + ) => + isElbowArrow(newElement) + ? mutateElbowArrow( + newElement, + updates as ElementUpdate, + false, + this.scene.getNonDeletedElementsMap(), + options, + ) + : mutateElement(newElement, updates, false); + if (points.length === 1) { - mutateElement( - newElement, - { - points: [...points, pointFrom(dx, dy)], - }, - false, - ); + mutate({ + points: [...points, pointFrom(dx, dy)], + }); } else if ( points.length === 2 || (points.length > 1 && isElbowArrow(newElement)) ) { - mutateElement( - newElement, + mutate( { points: [...points.slice(0, -1), pointFrom(dx, dy)], }, - false, - { isDragging: true }, + { isDragging: true } ); } @@ -8937,7 +8961,12 @@ class App extends React.Component { this.scene.getNonDeletedElementsMap(), ); if (element) { - mutateElement(element, {}, true); + mutateElbowArrow( + element as ExcalidrawElbowArrowElement, + {}, + true, + this.scene.getNonDeletedElementsMap(), + ); } } @@ -9080,7 +9109,7 @@ class App extends React.Component { ); if (!pointerDownState.drag.hasOccurred && newElement && !multiElement) { - mutateElement(newElement, { + const updates = { points: [ ...newElement.points, pointFrom( @@ -9088,7 +9117,19 @@ class App extends React.Component { pointerCoords.y - newElement.y, ), ], - }); + }; + + if (isElbowArrow(newElement)) { + mutateElbowArrow( + newElement, + updates, + false, + this.scene.getNonDeletedElementsMap(), + ); + } else { + mutateElement(newElement, updates, false); + } + this.setState({ multiElement: newElement, newElement, diff --git a/packages/excalidraw/components/Stats/MultiDimension.tsx b/packages/excalidraw/components/Stats/MultiDimension.tsx index b482611af..301eb1e79 100644 --- a/packages/excalidraw/components/Stats/MultiDimension.tsx +++ b/packages/excalidraw/components/Stats/MultiDimension.tsx @@ -13,10 +13,12 @@ import { handleBindTextResize, } from "@excalidraw/element/textElement"; -import { isTextElement } from "@excalidraw/element/typeChecks"; +import { isElbowArrow, isTextElement } from "@excalidraw/element/typeChecks"; import { getCommonBounds } from "@excalidraw/utils"; +import { mutateElbowArrow } from "@excalidraw/element/elbowArrow"; + import type { ElementsMap, ExcalidrawElement, @@ -80,7 +82,12 @@ const resizeElementInGroup = ( ) => { const updates = getResizedUpdates(anchorX, anchorY, scale, origElement); - mutateElement(latestElement, updates, false); + if (isElbowArrow(latestElement)) { + mutateElbowArrow(latestElement, updates, false, elementsMap); + } else { + mutateElement(latestElement, updates, false); + } + const boundTextElement = getBoundTextElement( origElement, originalElementsMap, diff --git a/packages/excalidraw/index.tsx b/packages/excalidraw/index.tsx index 5ea746754..0f68ef07c 100644 --- a/packages/excalidraw/index.tsx +++ b/packages/excalidraw/index.tsx @@ -264,6 +264,8 @@ export { bumpVersion, } from "@excalidraw/element/mutateElement"; +export { mutateElbowArrow } from "@excalidraw/element/elbowArrow"; + export { CaptureUpdateAction } from "./store"; export { parseLibraryTokensFromUrl, useHandleLibrary } from "./data/library";