fix(editor): Tab convert bound arrow update (#12143)

---------

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>

packages/element/src/binding.ts

@@ -1202,6 +1202,104 @@ export const unbindBindingElement = (
   return binding.elementId;
 };
 
+export const reanchorBindingsToOutline = (
+  changedElement: NonDeleted<ExcalidrawBindableElement>,
+  scene: Scene,
+) => {
+  const elementsMap = scene.getNonDeletedElementsMap();
+  const center = elementCenterPoint(changedElement, elementsMap);
+  const reach = Math.max(changedElement.width, changedElement.height) * 2;
+
+  boundElementsVisitor(elementsMap, changedElement, (element) => {
+    if (!isArrowElement(element) || !isNonDeletedElement(element)) {
+      return;
+    }
+
+    const updates: {
+      startBinding?: FixedPointBinding;
+      endBinding?: FixedPointBinding;
+    } = {};
+
+    for (const startOrEnd of ["startBinding", "endBinding"] as const) {
+      const binding = element[startOrEnd];
+      if (binding?.elementId !== changedElement.id) {
+        continue;
+      }
+
+      const startOrEndName = startOrEnd === "startBinding" ? "start" : "end";
+
+      if (isElbowArrow(element)) {
+        updates[startOrEnd] = {
+          ...binding,
+          ...calculateFixedPointForElbowArrowBinding(
+            element,
+            changedElement,
+            startOrEndName,
+            elementsMap,
+          ),
+        };
+        continue;
+      }
+
+      const focusPoint = getGlobalFixedPointForBindableElement(
+        binding.fixedPoint,
+        changedElement,
+        elementsMap,
+      );
+
+      if (
+        hitElementItself({
+          element: changedElement,
+          point: focusPoint,
+          elementsMap,
+          threshold: getBindingGap(changedElement, element),
+          overrideShouldTestInside: true,
+        })
+      ) {
+        continue;
+      }
+
+      const direction = vectorFromPoint(focusPoint, center);
+      if (direction[0] === 0 && direction[1] === 0) {
+        continue;
+      }
+      const outlinePoint = intersectElementWithLineSegment(
+        changedElement,
+        elementsMap,
+        lineSegment(
+          center,
+          pointFromVector(
+            vectorScale(vectorNormalize(direction), reach),
+            center,
+          ),
+        ),
+      ).sort(
+        (a, b) =>
+          pointDistanceSq(a, focusPoint) - pointDistanceSq(b, focusPoint),
+      )[0];
+
+      if (!outlinePoint) {
+        continue;
+      }
+
+      updates[startOrEnd] = {
+        ...binding,
+        ...calculateFixedPointForNonElbowArrowBinding(
+          element,
+          changedElement,
+          startOrEndName,
+          elementsMap,
+          outlinePoint,
+        ),
+      };
+    }
+
+    if (updates.startBinding || updates.endBinding) {
+      mutateElement(element, elementsMap, updates);
+    }
+  });
+};
+
 // Supports translating, rotating and scaling `changedElement` with bound
 // linear elements.
 export const updateBoundElements = (

packages/excalidraw/components/ConvertElementTypePopup.tsx

@@ -56,7 +56,11 @@ import {
 
 import { ShapeCache } from "@excalidraw/element";
 
-import { updateBindings } from "@excalidraw/element";
+import {
+  isBindableElement,
+  reanchorBindingsToOutline,
+  updateBindings,
+} from "@excalidraw/element";
 
 import type {
   ConvertibleGenericTypes,
@@ -475,6 +479,11 @@ export const convertElementTypes = (
       app.scene.replaceAllElements(nextElements);
 
       for (const element of Object.values(convertedElements)) {
+        if (isBindableElement(element)) {
+          reanchorBindingsToOutline(element, app.scene);
+          updateBindings(element, app.scene, app.state);
+        }
+
         const boundText = getBoundTextElement(
           element,
           app.scene.getNonDeletedElementsMap(),
@@ -859,8 +868,6 @@ const convertElementType = <
       }),
     ) as typeof element;
 
-    updateBindings(nextElement, app.scene, app.state);
-
     return nextElement;
   }
 

packages/excalidraw/tests/convertElementType.binding.test.tsx

@@ -0,0 +1,92 @@
+import { pointFrom, type GlobalPoint } from "@excalidraw/math";
+
+import { getBindingGap, distanceToElement } from "@excalidraw/element";
+
+import type {
+  ExcalidrawArrowElement,
+  ExcalidrawBindableElement,
+  FixedPointBinding,
+  NonDeletedExcalidrawElement,
+} from "@excalidraw/element/types";
+
+import { convertElementTypes } from "../components/ConvertElementTypePopup";
+import { Excalidraw } from "../index";
+
+import { API } from "./helpers/api";
+import { act, render } from "./test-utils";
+
+const { h } = window;
+
+describe("convert element type keeps bound arrows attached", () => {
+  beforeEach(async () => {
+    await render(<Excalidraw handleKeyboardGlobally={true} />);
+  });
+
+  const cases = [
+    ["rectangle", "diamond", [1, 0.25]],
+    ["rectangle", "ellipse", [1, 0.1]],
+    ["ellipse", "rectangle", [0.9, 0.2]],
+    ["diamond", "rectangle", [0.75, 0.25]],
+  ] as const;
+
+  for (const elbowed of [false, true]) {
+    it.each(cases)(
+      `elbowed=${elbowed}: %s -> %s`,
+      (fromType, toType, fixedPoint) => {
+        const shape = API.createElement({
+          type: fromType,
+          x: 0,
+          y: 0,
+          width: 200,
+          height: 200,
+        });
+        const arrow = API.createElement({
+          type: "arrow",
+          elbowed,
+          x: 400,
+          y: 50,
+          width: 200,
+          height: 0,
+          points: [pointFrom(0, 0), pointFrom(-200, 0)],
+          endBinding: {
+            elementId: shape.id,
+            fixedPoint,
+            mode: "orbit",
+          } as FixedPointBinding,
+          startBinding: null,
+        });
+        API.setElements([
+          { ...shape, boundElements: [{ id: arrow.id, type: "arrow" }] },
+          arrow,
+        ]);
+        API.setSelectedElements([h.elements[0] as NonDeletedExcalidrawElement]);
+
+        act(() => {
+          convertElementTypes(h.app, {
+            conversionType: "generic",
+            nextType: toType,
+          });
+        });
+        expect(h.elements[0].type).toBe(toType);
+
+        const converted = h.elements[0] as ExcalidrawBindableElement;
+        const arrowAfter = h.elements[1] as ExcalidrawArrowElement;
+        const [endX, endY] = arrowAfter.points[arrowAfter.points.length - 1];
+        const globalEnd = pointFrom<GlobalPoint>(
+          arrowAfter.x + endX,
+          arrowAfter.y + endY,
+        );
+
+        // the arrow end must sit on the new outline (within the binding gap)
+        const distance = distanceToElement(
+          converted,
+          h.app.scene.getNonDeletedElementsMap(),
+          globalEnd,
+        );
+        expect(distance).toBeLessThanOrEqual(
+          getBindingGap(converted, arrowAfter) + 1,
+        );
+      },
+    );
+  }
+});