fix: arrow restoration during undo/redo

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<T> {
 }
 
 /**
- * Encapsulates the modifications captured as `Delta`/s.
+ * Encapsulates the modifications captured as `Delta`s.
  */
 interface Change<T> {
   /**
@@ -1212,12 +1213,8 @@ export class ElementsChange implements Change<SceneElementsMap> {
     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<SceneElementsMap> {
         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<typeof element>,
+        );
+      }
+
+      // Otherwise preserve the original points
+      const partialWithoutPoints = { ...directlyApplicablePartial };
+      delete (partialWithoutPoints as any).points;
+
+      return newElementWith(
+        element,
+        partialWithoutPoints as ElementUpdate<typeof element>,
+      );
+    }
+
     return newElementWith(element, directlyApplicablePartial);
   }
 
@@ -1496,6 +1525,7 @@ export class ElementsChange implements Change<SceneElementsMap> {
       if (!element.isDeleted && isBindableElement(element)) {
         updateBoundElements(element, elements, {
           changedElements: changed,
+          preservePoints: true, // Preserve arrow points during undo/redo
         });
       }
     }

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<string, OrderedExcalidrawElement>;
+    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<GlobalPoint>(
-          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<GlobalPoint>(
         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<GlobalPoint>(
+    //     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;

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(<Excalidraw handleKeyboardGlobally={true} />);
+  });
+
+  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);
+  });
+});