Merge 4bb9f1ed18 into ab89d4c16f

packages/excalidraw/change.ts

@@ -16,6 +16,7 @@ import {
   isBoundToContainer,
   isImageElement,
   isTextElement,
+  isArrowElement,
 } from "./element/typeChecks";
 import { orderByFractionalIndex, syncMovedIndices } from "./fractionalIndex";
 import { getNonDeletedGroupIds } from "./groups";
@@ -1493,13 +1494,73 @@ export class ElementsChange implements Change<SceneElementsMap> {
     elements: SceneElementsMap,
     changed: Map<string, OrderedExcalidrawElement>,
   ) {
+    // First, collect all arrow elements that need to be updated
+    const arrowsToUpdate = new Set<string>();
+
+    // Check for bindable elements that were changed
     for (const element of changed.values()) {
       if (!element.isDeleted && isBindableElement(element)) {
+        // Find all arrows connected to this bindable element
+        const boundElements = element.boundElements || [];
+        for (const binding of boundElements) {
+          if (binding.type === "arrow") {
+            arrowsToUpdate.add(binding.id);
+          }
+        }
+
+        // Update bound elements for this bindable element
         updateBoundElements(element, elements, {
           changedElements: changed,
         });
       }
     }
+
+    // Check for arrow elements that were changed
+    for (const element of changed.values()) {
+      if (!element.isDeleted && isArrowElement(element)) {
+        arrowsToUpdate.add(element.id);
+      }
+    }
+
+    // Process all arrows that need updating
+    for (const arrowId of arrowsToUpdate) {
+      const arrowElement = elements.get(arrowId);
+      if (
+        arrowElement &&
+        isArrowElement(arrowElement) &&
+        !arrowElement.isDeleted
+      ) {
+        // Cast to ExcalidrawLinearElement to access binding properties
+        const arrow = arrowElement as NonDeleted<ExcalidrawLinearElement>;
+
+        // Make sure startBinding and endBinding are consistent
+        if (arrow.startBinding) {
+          const bindTarget = elements.get(arrow.startBinding.elementId);
+          if (!bindTarget || bindTarget.isDeleted) {
+            // If the target was deleted, remove the binding
+            mutateElement(arrow, { startBinding: null });
+          } else {
+            // Ensure the bound element has this arrow in its boundElements
+            updateBoundElements(bindTarget, elements, {
+              simultaneouslyUpdated: [arrow],
+            });
+          }
+        }
+
+        if (arrow.endBinding) {
+          const bindTarget = elements.get(arrow.endBinding.elementId);
+          if (!bindTarget || bindTarget.isDeleted) {
+            // If the target was deleted, remove the binding
+            mutateElement(arrow, { endBinding: null });
+          } else {
+            // Ensure the bound element has this arrow in its boundElements
+            updateBoundElements(bindTarget, elements, {
+              simultaneouslyUpdated: [arrow],
+            });
+          }
+        }
+      }
+    }
   }
 
   private static reorderElements(

packages/excalidraw/tests/__snapshots__/history.test.tsx.snap

@@ -226,7 +226,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 40,
+  "version": 56,
   "width": "101.77517",
   "x": "0.70711",
   "y": 0,
@@ -817,7 +817,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 30,
+  "version": 44,
   "width": 0,
   "x": "149.29289",
   "y": 0,
@@ -1275,7 +1275,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 11,
+  "version": 15,
   "width": "98.58579",
   "x": "0.70711",
   "y": 0,
@@ -1646,7 +1646,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 11,
+  "version": 15,
   "width": "98.58579",
   "x": "0.70711",
   "y": 0,
@@ -2353,7 +2353,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 10,
+  "version": 12,
   "width": "502.78936",
   "x": "-0.83465",
   "y": "-36.58211",
@@ -15149,7 +15149,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 10,
+  "version": 12,
   "width": "98.58579",
   "x": "0.70711",
   "y": 0,
@@ -15846,7 +15846,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 10,
+  "version": 12,
   "width": "98.58579",
   "x": "0.70711",
   "y": 0,
@@ -16465,7 +16465,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 10,
+  "version": 12,
   "width": "98.58579",
   "x": "0.70711",
   "y": 0,
@@ -17082,7 +17082,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 10,
+  "version": 12,
   "width": "98.58579",
   "x": "0.70711",
   "y": 0,
@@ -17795,7 +17795,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 11,
+  "version": 13,
   "width": "98.58579",
   "x": "0.70711",
   "y": 0,

packages/excalidraw/tests/arrow-bindings.test.tsx

@@ -0,0 +1,164 @@
+import { vi } from "vitest";
+
+import { Excalidraw } from "../index";
+
+import { API } from "./helpers/api";
+import { Keyboard, Pointer, UI } from "./helpers/ui";
+import { GlobalTestState, render, unmountComponent } from "./test-utils";
+
+import type {
+  ExcalidrawLinearElement,
+  ExcalidrawRectangleElement,
+} from "../element/types";
+import type { NormalizedZoomValue } from "../types";
+
+const { h } = window;
+const mouse = new Pointer("mouse");
+
+describe("arrow bindings", () => {
+  beforeEach(async () => {
+    unmountComponent();
+
+    mouse.reset();
+    localStorage.clear();
+    sessionStorage.clear();
+    vi.clearAllMocks();
+
+    Object.assign(document, {
+      elementFromPoint: () => GlobalTestState.canvas,
+    });
+    await render(<Excalidraw autoFocus={true} handleKeyboardGlobally={true} />);
+    API.setAppState({
+      zoom: {
+        value: 1 as NormalizedZoomValue,
+      },
+    });
+  });
+
+  it("should preserve arrow bindings after undo/redo", async () => {
+    const rect = UI.createElement("rectangle", {
+      x: 100,
+      y: 100,
+      width: 100,
+      height: 100,
+    });
+
+    UI.clickTool("arrow");
+    mouse.downAt(50, 150);
+    mouse.moveTo(100, 150);
+    mouse.up();
+
+    const arrow = h.elements[1] as ExcalidrawLinearElement;
+
+    expect(arrow.endBinding).toEqual(
+      expect.objectContaining({
+        elementId: rect.id,
+        focus: expect.any(Number),
+        gap: expect.any(Number),
+      }),
+    );
+
+    expect(rect.boundElements).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          id: arrow.id,
+          type: "arrow",
+        }),
+      ]),
+    );
+
+    Keyboard.undo();
+
+    expect(h.elements).toEqual([
+      expect.objectContaining({ id: rect.id, boundElements: [] }),
+      expect.objectContaining({ id: arrow.id, isDeleted: true }),
+    ]);
+
+    Keyboard.redo();
+
+    expect(h.elements).toEqual([
+      expect.objectContaining({
+        id: rect.id,
+        boundElements: expect.arrayContaining([
+          expect.objectContaining({
+            id: arrow.id,
+            type: "arrow",
+          }),
+        ]),
+      }),
+      expect.objectContaining({
+        id: arrow.id,
+        isDeleted: false,
+        endBinding: expect.objectContaining({
+          elementId: rect.id,
+          focus: expect.any(Number),
+          gap: expect.any(Number),
+        }),
+      }),
+    ]);
+  });
+
+  it("should preserve arrow bindings after moving rectangle and undo/redo", async () => {
+    const rect = UI.createElement("rectangle", {
+      x: 100,
+      y: 100,
+      width: 100,
+      height: 100,
+    });
+
+    UI.clickTool("arrow");
+    mouse.downAt(50, 150);
+    mouse.moveTo(100, 150);
+    mouse.up();
+
+    mouse.select(rect);
+    mouse.downAt(150, 150);
+    mouse.moveTo(250, 150);
+    mouse.up();
+
+    const movedArrow = h.elements[1] as ExcalidrawLinearElement;
+    const movedRect = h.elements[0] as ExcalidrawRectangleElement;
+
+    expect(movedRect.x).toBe(200);
+    expect(movedArrow.endBinding).toEqual(
+      expect.objectContaining({
+        elementId: rect.id,
+        focus: expect.any(Number),
+        gap: expect.any(Number),
+      }),
+    );
+
+    Keyboard.undo();
+
+    const undoRect = h.elements[0] as ExcalidrawRectangleElement;
+    const undoArrow = h.elements[1] as ExcalidrawLinearElement;
+
+    expect(undoRect.x).toBe(100);
+
+    expect(undoArrow.endBinding).toEqual(
+      expect.objectContaining({
+        elementId: rect.id,
+        focus: expect.any(Number),
+        gap: expect.any(Number),
+      }),
+    );
+
+    Keyboard.redo();
+
+    const redoRect = h.elements[0] as ExcalidrawRectangleElement;
+    const redoArrow = h.elements[1] as ExcalidrawLinearElement;
+
+    expect(redoRect.x).toBe(200);
+
+    expect(redoArrow.endBinding).toEqual(
+      expect.objectContaining({
+        elementId: rect.id,
+        focus: expect.any(Number),
+        gap: expect.any(Number),
+      }),
+    );
+
+    expect(redoRect.x).not.toEqual(undoRect.x);
+    expect(redoArrow.endBinding?.elementId).toEqual(rect.id);
+  });
+});