fix: Bound elbow arrow on duplication does not route correctly (#9236)

packages/excalidraw/element/elbowArrow.test.tsx

@@ -3,6 +3,7 @@ import Scene from "../scene/Scene";
 import { API } from "../tests/helpers/api";
 import { Pointer, UI } from "../tests/helpers/ui";
 import {
+  act,
   fireEvent,
   GlobalTestState,
   queryByTestId,
@@ -19,6 +20,8 @@ import { ARROW_TYPE } from "../constants";
 import "../../utils/test-utils";
 import type { LocalPoint } from "@excalidraw/math";
 import { pointFrom } from "@excalidraw/math";
+import { actionDuplicateSelection } from "../actions/actionDuplicateSelection";
+import { actionSelectAll } from "../actions";
 
 const { h } = window;
 
@@ -292,4 +295,114 @@ describe("elbow arrow ui", () => {
       [103, 165],
     ]);
   });
+
+  it("keeps arrow shape when the whole set of arrow and bindables are duplicated", async () => {
+    UI.createElement("rectangle", {
+      x: -150,
+      y: -150,
+      width: 100,
+      height: 100,
+    });
+    UI.createElement("rectangle", {
+      x: 50,
+      y: 50,
+      width: 100,
+      height: 100,
+    });
+
+    UI.clickTool("arrow");
+    UI.clickOnTestId("elbow-arrow");
+
+    mouse.reset();
+    mouse.moveTo(-43, -99);
+    mouse.click();
+    mouse.moveTo(43, 99);
+    mouse.click();
+
+    const arrow = h.scene.getSelectedElements(
+      h.state,
+    )[0] as ExcalidrawArrowElement;
+    const originalArrowId = arrow.id;
+
+    expect(arrow.startBinding).not.toBe(null);
+    expect(arrow.endBinding).not.toBe(null);
+
+    act(() => {
+      h.app.actionManager.executeAction(actionSelectAll);
+    });
+
+    act(() => {
+      h.app.actionManager.executeAction(actionDuplicateSelection);
+    });
+
+    expect(h.elements.length).toEqual(6);
+
+    const duplicatedArrow = h.scene.getSelectedElements(
+      h.state,
+    )[2] as ExcalidrawArrowElement;
+
+    expect(duplicatedArrow.id).not.toBe(originalArrowId);
+    expect(duplicatedArrow.type).toBe("arrow");
+    expect(duplicatedArrow.elbowed).toBe(true);
+    expect(duplicatedArrow.points).toEqual([
+      [0, 0],
+      [45, 0],
+      [45, 200],
+      [90, 200],
+    ]);
+    expect(arrow.startBinding).not.toBe(null);
+    expect(arrow.endBinding).not.toBe(null);
+  });
+
+  it("keeps arrow shape when only the bound arrow is duplicated", async () => {
+    UI.createElement("rectangle", {
+      x: -150,
+      y: -150,
+      width: 100,
+      height: 100,
+    });
+    UI.createElement("rectangle", {
+      x: 50,
+      y: 50,
+      width: 100,
+      height: 100,
+    });
+
+    UI.clickTool("arrow");
+    UI.clickOnTestId("elbow-arrow");
+
+    mouse.reset();
+    mouse.moveTo(-43, -99);
+    mouse.click();
+    mouse.moveTo(43, 99);
+    mouse.click();
+
+    const arrow = h.scene.getSelectedElements(
+      h.state,
+    )[0] as ExcalidrawArrowElement;
+    const originalArrowId = arrow.id;
+
+    expect(arrow.startBinding).not.toBe(null);
+    expect(arrow.endBinding).not.toBe(null);
+
+    act(() => {
+      h.app.actionManager.executeAction(actionDuplicateSelection);
+    });
+
+    expect(h.elements.length).toEqual(4);
+
+    const duplicatedArrow = h.scene.getSelectedElements(
+      h.state,
+    )[0] as ExcalidrawArrowElement;
+
+    expect(duplicatedArrow.id).not.toBe(originalArrowId);
+    expect(duplicatedArrow.type).toBe("arrow");
+    expect(duplicatedArrow.elbowed).toBe(true);
+    expect(duplicatedArrow.points).toEqual([
+      [0, 0],
+      [45, 0],
+      [45, 200],
+      [90, 200],
+    ]);
+  });
 });

packages/excalidraw/element/elbowArrow.ts

@@ -963,24 +963,6 @@ export const updateElbowArrowPoints = (
     );
   }
 
-  // 0. During all element replacement in the scene, we just need to renormalize
-  // the arrow
-  // TODO (dwelle,mtolmacs): Remove this once Scene.getScene() is removed
-  if (
-    elementsMap.size === 0 &&
-    updates.points &&
-    validateElbowPoints(updates.points)
-  ) {
-    return normalizeArrowElementUpdate(
-      updates.points.map((p) =>
-        pointFrom<GlobalPoint>(arrow.x + p[0], arrow.y + p[1]),
-      ),
-      arrow.fixedSegments,
-      arrow.startIsSpecial,
-      arrow.endIsSpecial,
-    );
-  }
-
   const updatedPoints: readonly LocalPoint[] = updates.points
     ? updates.points && updates.points.length === 2
       ? arrow.points.map((p, idx) =>
@@ -993,6 +975,34 @@ export const updateElbowArrowPoints = (
       : updates.points.slice()
     : arrow.points.slice();
 
+  // 0. During all element replacement in the scene, we just need to renormalize
+  // the arrow
+  // TODO (dwelle,mtolmacs): Remove this once Scene.getScene() is removed
+  const startBinding =
+    typeof updates.startBinding !== "undefined"
+      ? updates.startBinding
+      : arrow.startBinding;
+  const endBinding =
+    typeof updates.endBinding !== "undefined"
+      ? updates.endBinding
+      : arrow.endBinding;
+  const startElement = startBinding && elementsMap.get(startBinding.elementId);
+  const endElement = endBinding && elementsMap.get(endBinding.elementId);
+  if (
+    (elementsMap.size === 0 && validateElbowPoints(updatedPoints)) ||
+    startElement?.id !== startBinding?.elementId ||
+    endElement?.id !== endBinding?.elementId
+  ) {
+    return normalizeArrowElementUpdate(
+      updatedPoints.map((p) =>
+        pointFrom<GlobalPoint>(arrow.x + p[0], arrow.y + p[1]),
+      ),
+      arrow.fixedSegments,
+      arrow.startIsSpecial,
+      arrow.endIsSpecial,
+    );
+  }
+
   const {
     startHeading,
     endHeading,
@@ -1005,14 +1015,8 @@ export const updateElbowArrowPoints = (
     {
       x: arrow.x,
       y: arrow.y,
-      startBinding:
+      startBinding,
-        typeof updates.startBinding !== "undefined"
+      endBinding,
-          ? updates.startBinding
-          : arrow.startBinding,
-      endBinding:
-        typeof updates.endBinding !== "undefined"
-          ? updates.endBinding
-          : arrow.endBinding,
       startArrowhead: arrow.startArrowhead,
       endArrowhead: arrow.endArrowhead,
     },