From 04668d8263b35bf76f1390b25abeeed4181820f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?M=C3=A1rk=20Tolm=C3=A1cs?= <mark@lazycat.hu>
Date: Fri, 28 Jun 2024 15:28:48 +0200
Subject: [PATCH] fix: Binding after duplicating is now applied for both the
 old and duplicate shapes (#8185)

Using ALT/OPT + drag to clone does not transfer the bindings (or leaves the duplicates in place of the old one , which are also not bound).

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
---
 packages/excalidraw/element/binding.ts     |  9 ++++-
 packages/excalidraw/tests/binding.test.tsx | 47 +++++++++++++++++++++-
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/packages/excalidraw/element/binding.ts b/packages/excalidraw/element/binding.ts
index 8d1a728eaa..ba0a110c64 100644
--- a/packages/excalidraw/element/binding.ts
+++ b/packages/excalidraw/element/binding.ts
@@ -707,6 +707,9 @@ export const fixBindingsAfterDuplication = (
   const allBoundElementIds: Set<ExcalidrawElement["id"]> = new Set();
   const allBindableElementIds: Set<ExcalidrawElement["id"]> = new Set();
   const shouldReverseRoles = duplicatesServeAsOld === "duplicatesServeAsOld";
+  const duplicateIdToOldId = new Map(
+    [...oldIdToDuplicatedId].map(([key, value]) => [value, key]),
+  );
   oldElements.forEach((oldElement) => {
     const { boundElements } = oldElement;
     if (boundElements != null && boundElements.length > 0) {
@@ -756,7 +759,11 @@ export const fixBindingsAfterDuplication = (
   sceneElements
     .filter(({ id }) => allBindableElementIds.has(id))
     .forEach((bindableElement) => {
-      const { boundElements } = bindableElement;
+      const oldElementId = duplicateIdToOldId.get(bindableElement.id);
+      const { boundElements } = sceneElements.find(
+        ({ id }) => id === oldElementId,
+      )!;
+
       if (boundElements != null && boundElements.length > 0) {
         mutateElement(bindableElement, {
           boundElements: boundElements.map((boundElement) =>
diff --git a/packages/excalidraw/tests/binding.test.tsx b/packages/excalidraw/tests/binding.test.tsx
index ece2d2fe3e..9bc9947c7f 100644
--- a/packages/excalidraw/tests/binding.test.tsx
+++ b/packages/excalidraw/tests/binding.test.tsx
@@ -1,5 +1,5 @@
 import { fireEvent, render } from "./test-utils";
-import { Excalidraw } from "../index";
+import { Excalidraw, isLinearElement } from "../index";
 import { UI, Pointer, Keyboard } from "./helpers/ui";
 import { getTransformHandles } from "../element/transformHandles";
 import { API } from "./helpers/api";
@@ -433,4 +433,49 @@ describe("element binding", () => {
     expect(arrow.startBinding).not.toBe(null);
     expect(arrow.endBinding).toBe(null);
   });
+
+  it("should not unbind when duplicating via selection group", () => {
+    const rectLeft = UI.createElement("rectangle", {
+      x: 0,
+      width: 200,
+      height: 500,
+    });
+    const rectRight = UI.createElement("rectangle", {
+      x: 400,
+      y: 200,
+      width: 200,
+      height: 500,
+    });
+    const arrow = UI.createElement("arrow", {
+      x: 210,
+      y: 250,
+      width: 177,
+      height: 1,
+    });
+    expect(arrow.startBinding?.elementId).toBe(rectLeft.id);
+    expect(arrow.endBinding?.elementId).toBe(rectRight.id);
+
+    mouse.downAt(-100, -100);
+    mouse.moveTo(650, 750);
+    mouse.up(0, 0);
+
+    expect(API.getSelectedElements().length).toBe(3);
+
+    mouse.moveTo(5, 5);
+    Keyboard.withModifierKeys({ alt: true }, () => {
+      mouse.downAt(5, 5);
+      mouse.moveTo(1000, 1000);
+      mouse.up(0, 0);
+
+      expect(window.h.elements.length).toBe(6);
+      window.h.elements.forEach((element) => {
+        if (isLinearElement(element)) {
+          expect(element.startBinding).not.toBe(null);
+          expect(element.endBinding).not.toBe(null);
+        } else {
+          expect(element.boundElements).not.toBe(null);
+        }
+      });
+    });
+  });
 });