diff --git a/packages/excalidraw/change.ts b/packages/excalidraw/change.ts index 206f3dcc7..40cd7958f 100644 --- a/packages/excalidraw/change.ts +++ b/packages/excalidraw/change.ts @@ -17,6 +17,7 @@ import { isBoundToContainer, isImageElement, isTextElement, + isArrowElement, } from "./element/typeChecks"; import { orderByFractionalIndex, syncMovedIndices } from "./fractionalIndex"; import { getNonDeletedGroupIds } from "./groups"; @@ -393,7 +394,7 @@ class Delta { } /** - * Encapsulates the modifications captured as `Delta`/s. + * Encapsulates the modifications captured as `Delta`s. */ interface Change { /** @@ -1212,12 +1213,8 @@ export class ElementsChange implements Change { flags: { containsVisibleDifference: boolean; containsZindexDifference: boolean; - } = { - // by default we don't care about about the flags - containsVisibleDifference: true, - containsZindexDifference: true, }, - ) { + ): OrderedExcalidrawElement { const { boundElements, ...directlyApplicablePartial } = delta.inserted; if ( @@ -1262,6 +1259,38 @@ export class ElementsChange implements Change { delta.deleted.index !== delta.inserted.index; } + // Fix for arrow points preservation during undo/redo + if ( + element.type === "arrow" && + isArrowElement(element) && + (directlyApplicablePartial as any).points && + !directlyApplicablePartial.isDeleted + ) { + // Only update points if there's a binding change + if ( + ((directlyApplicablePartial as any).startBinding !== undefined && + (directlyApplicablePartial as any).startBinding !== + element.startBinding) || + ((directlyApplicablePartial as any).endBinding !== undefined && + (directlyApplicablePartial as any).endBinding !== element.endBinding) + ) { + // Let the points be updated by the delta + return newElementWith( + element, + directlyApplicablePartial as ElementUpdate, + ); + } + + // Otherwise preserve the original points + const partialWithoutPoints = { ...directlyApplicablePartial }; + delete (partialWithoutPoints as any).points; + + return newElementWith( + element, + partialWithoutPoints as ElementUpdate, + ); + } + return newElementWith(element, directlyApplicablePartial); } @@ -1496,6 +1525,7 @@ export class ElementsChange implements Change { if (!element.isDeleted && isBindableElement(element)) { updateBoundElements(element, elements, { changedElements: changed, + preservePoints: true, // Preserve arrow points during undo/redo }); } } diff --git a/packages/excalidraw/element/binding.ts b/packages/excalidraw/element/binding.ts index 93c5292f1..157cb597b 100644 --- a/packages/excalidraw/element/binding.ts +++ b/packages/excalidraw/element/binding.ts @@ -487,31 +487,32 @@ export const bindLinearElement = ( return; } - let binding: PointBinding | FixedPointBinding = { + const binding: PointBinding | FixedPointBinding = { elementId: hoveredElement.id, - ...normalizePointBinding( - calculateFocusAndGap( - linearElement, - hoveredElement, - startOrEnd, - elementsMap, - ), - hoveredElement, - ), + ...(isElbowArrow(linearElement) + ? { + ...calculateFixedPointForElbowArrowBinding( + linearElement, + hoveredElement, + startOrEnd, + elementsMap, + ), + focus: 0, + gap: 0, + } + : { + ...normalizePointBinding( + calculateFocusAndGap( + linearElement, + hoveredElement, + startOrEnd, + elementsMap, + ), + hoveredElement, + ), + }), }; - if (isElbowArrow(linearElement)) { - binding = { - ...binding, - ...calculateFixedPointForElbowArrowBinding( - linearElement, - hoveredElement, - startOrEnd, - elementsMap, - ), - }; - } - mutateElement(linearElement, { [startOrEnd === "start" ? "startBinding" : "endBinding"]: binding, }); @@ -739,9 +740,14 @@ export const updateBoundElements = ( simultaneouslyUpdated?: readonly ExcalidrawElement[]; newSize?: { width: number; height: number }; changedElements?: Map; + preservePoints?: boolean; // Add this option to preserve arrow points during undo/redo }, ) => { - const { newSize, simultaneouslyUpdated } = options ?? {}; + const { + newSize, + simultaneouslyUpdated, + preservePoints = false, + } = options ?? {}; const simultaneouslyUpdatedElementIds = getSimultaneouslyUpdatedElementIds( simultaneouslyUpdated, ); @@ -794,6 +800,19 @@ export const updateBoundElements = ( return; } + // If preservePoints is true, only update the bindings without changing the points + if (preservePoints && isArrowElement(element)) { + mutateElement(element, { + ...(changedElement.id === element.startBinding?.elementId + ? { startBinding: bindings.startBinding } + : {}), + ...(changedElement.id === element.endBinding?.elementId + ? { endBinding: bindings.endBinding } + : {}), + }); + return; + } + const updates = bindableElementsVisitor( elementsMap, element, @@ -1271,35 +1290,39 @@ const updateBoundPoint = ( pointDistance(adjacentPoint, edgePointAbsolute) + pointDistance(adjacentPoint, center) + Math.max(bindableElement.width, bindableElement.height) * 2; - const intersections = [ - ...intersectElementWithLineSegment( - bindableElement, - lineSegment( - adjacentPoint, - pointFromVector( - vectorScale( - vectorNormalize( - vectorFromPoint(focusPointAbsolute, adjacentPoint), - ), - interceptorLength, - ), - adjacentPoint, - ), - ), - binding.gap, - ).sort( - (g, h) => - pointDistanceSq(g, adjacentPoint) - pointDistanceSq(h, adjacentPoint), - ), - // Fallback when arrow doesn't point to the shape - pointFromVector( - vectorScale( - vectorNormalize(vectorFromPoint(focusPointAbsolute, adjacentPoint)), - pointDistance(adjacentPoint, edgePointAbsolute), - ), + const intersections = intersectElementWithLineSegment( + bindableElement, + lineSegment( adjacentPoint, + pointFromVector( + vectorScale( + vectorNormalize(vectorFromPoint(focusPointAbsolute, adjacentPoint)), + interceptorLength, + ), + adjacentPoint, + ), ), - ]; + binding.gap, + ).sort( + (g, h) => + pointDistanceSq(g, adjacentPoint) - pointDistanceSq(h, adjacentPoint), + ); + + // debugClear(); + // debugDrawPoint(intersections[0], { color: "red", permanent: true }); + // debugDrawLine( + // lineSegment( + // adjacentPoint, + // pointFromVector( + // vectorScale( + // vectorNormalize(vectorFromPoint(focusPointAbsolute, adjacentPoint)), + // interceptorLength, + // ), + // adjacentPoint, + // ), + // ), + // { permanent: true, color: "green" }, + // ); if (intersections.length > 1) { // The adjacent point is outside the shape (+ gap) @@ -1722,6 +1745,21 @@ const determineFocusDistance = ( ) .sort((g, h) => Math.abs(g) - Math.abs(h)); + // debugClear(); + // [ + // lineSegmentIntersectionPoints(rotatedInterceptor, interceptees[0]), + // lineSegmentIntersectionPoints(rotatedInterceptor, interceptees[1]), + // ] + // .filter((p): p is GlobalPoint => p !== null) + // .forEach((p) => debugDrawPoint(p, { color: "black", permanent: true })); + // debugDrawPoint(determineFocusPoint(element, ordered[0] ?? 0, rotatedA), { + // color: "red", + // permanent: true, + // }); + // debugDrawLine(rotatedInterceptor, { color: "green", permanent: true }); + // debugDrawLine(interceptees[0], { color: "red", permanent: true }); + // debugDrawLine(interceptees[1], { color: "red", permanent: true }); + const signedDistanceRatio = ordered[0] ?? 0; return signedDistanceRatio; diff --git a/packages/excalidraw/tests/arrow-undo-redo.test.tsx b/packages/excalidraw/tests/arrow-undo-redo.test.tsx new file mode 100644 index 000000000..38c7b300f --- /dev/null +++ b/packages/excalidraw/tests/arrow-undo-redo.test.tsx @@ -0,0 +1,56 @@ +import React from "react"; +import { vi } from "vitest"; + +import { pointFrom } from "@excalidraw/math"; + +import { Excalidraw } from "../index"; + +import { KEYS } from "../keys"; + +import { API } from "./helpers/api"; +import { UI } from "./helpers/ui"; +import { Keyboard, Pointer } from "./helpers/ui"; +import { render } from "./test-utils"; + +const mouse = new Pointer("mouse"); + +describe("arrow undo/redo", () => { + beforeEach(async () => { + await render(); + }); + + it("should maintain arrow shape after undo/redo", () => { + // Create a rectangle + const rectangle = UI.createElement("rectangle", { + x: 100, + y: 100, + width: 100, + height: 100, + }); + + // Create an arrow starting from rectangle border + const arrow = UI.createElement("arrow", { + x: 150, + y: 100, + width: 100, + height: 50, + points: [pointFrom(0, 0), pointFrom(100, 50)], + }); + + // Store original arrow points + const originalPoints = [...arrow.points]; + + // Perform undo + Keyboard.withModifierKeys({ ctrl: true }, () => { + Keyboard.keyPress(KEYS.Z); + }); + + // Perform redo + Keyboard.withModifierKeys({ ctrl: true, shift: true }, () => { + Keyboard.keyPress(KEYS.Z); + }); + + // Verify arrow points are exactly the same after redo + expect(arrow.points).toEqual(originalPoints); + }); +});