fix: Arrow conversion regression (#9241)

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

packages/excalidraw/actions/actionProperties.tsx

@@ -111,6 +111,8 @@ import {
   tupleToCoors,
 } from "../utils";
 
+import { updateElbowArrowPoints } from "../element/elbowArrow";
+
 import { register } from "./register";
 
 import type {
@@ -1572,7 +1574,7 @@ export const actionChangeArrowType = register({
       if (!isArrowElement(el)) {
         return el;
       }
-      const newElement = newElementWith(el, {
+      let newElement = newElementWith(el, {
         roundness:
           value === ARROW_TYPE.round
             ? {
@@ -1587,6 +1589,8 @@ export const actionChangeArrowType = register({
       });
 
       if (isElbowArrow(newElement)) {
+        newElement.fixedSegments = null;
+
         const elementsMap = app.scene.getNonDeletedElementsMap();
 
         app.dismissLinearEditor();
@@ -1661,46 +1665,71 @@ export const actionChangeArrowType = register({
         endHoveredElement &&
           bindLinearElement(newElement, endHoveredElement, "end", elementsMap);
 
-        mutateElement(newElement, {
-          points: [finalStartPoint, finalEndPoint].map(
-            (p): LocalPoint =>
-              pointFrom(p[0] - newElement.x, p[1] - newElement.y),
-          ),
-          ...(startElement && newElement.startBinding
+        const startBinding =
+          startElement && newElement.startBinding
             ? {
-                startBinding: {
-                  // @ts-ignore TS cannot discern check above
-                  ...newElement.startBinding!,
-                  ...calculateFixedPointForElbowArrowBinding(
-                    newElement,
-                    startElement,
-                    "start",
-                    elementsMap,
-                  ),
-                },
+                // @ts-ignore TS cannot discern check above
+                ...newElement.startBinding!,
+                ...calculateFixedPointForElbowArrowBinding(
+                  newElement,
+                  startElement,
+                  "start",
+                  elementsMap,
+                ),
               }
-            : {}),
-          ...(endElement && newElement.endBinding
+            : null;
+        const endBinding =
+          endElement && newElement.endBinding
             ? {
-                endBinding: {
-                  // @ts-ignore TS cannot discern check above
-                  ...newElement.endBinding,
-                  ...calculateFixedPointForElbowArrowBinding(
-                    newElement,
-                    endElement,
-                    "end",
-                    elementsMap,
-                  ),
-                },
+                // @ts-ignore TS cannot discern check above
+                ...newElement.endBinding,
+                ...calculateFixedPointForElbowArrowBinding(
+                  newElement,
+                  endElement,
+                  "end",
+                  elementsMap,
+                ),
               }
-            : {}),
-        });
+            : null;
+
+        newElement = {
+          ...newElement,
+          startBinding,
+          endBinding,
+          ...updateElbowArrowPoints(newElement, elementsMap, {
+            points: [finalStartPoint, finalEndPoint].map(
+              (p): LocalPoint =>
+                pointFrom(p[0] - newElement.x, p[1] - newElement.y),
+            ),
+            startBinding,
+            endBinding,
+            fixedSegments: null,
+          }),
+        };
 
         LinearElementEditor.updateEditorMidPointsCache(
           newElement,
           elementsMap,
           app.state,
         );
+      } else {
+        const elementsMap = app.scene.getNonDeletedElementsMap();
+        if (newElement.startBinding) {
+          const startElement = elementsMap.get(
+            newElement.startBinding.elementId,
+          ) as ExcalidrawBindableElement;
+          if (startElement) {
+            bindLinearElement(newElement, startElement, "start", elementsMap);
+          }
+        }
+        if (newElement.endBinding) {
+          const endElement = elementsMap.get(
+            newElement.endBinding.elementId,
+          ) as ExcalidrawBindableElement;
+          if (endElement) {
+            bindLinearElement(newElement, endElement, "end", elementsMap);
+          }
+        }
       }
 
       return newElement;

packages/excalidraw/element/binding.ts

@@ -487,32 +487,31 @@ export const bindLinearElement = (
     return;
   }
 
-  const binding: PointBinding | FixedPointBinding = {
+  let binding: PointBinding | FixedPointBinding = {
     elementId: hoveredElement.id,
-    ...(isElbowArrow(linearElement)
-      ? {
-          ...calculateFixedPointForElbowArrowBinding(
-            linearElement,
-            hoveredElement,
-            startOrEnd,
-            elementsMap,
-          ),
-          focus: 0,
-          gap: 0,
-        }
-      : {
-          ...normalizePointBinding(
-            calculateFocusAndGap(
-              linearElement,
-              hoveredElement,
-              startOrEnd,
-              elementsMap,
-            ),
-            hoveredElement,
-          ),
-        }),
+    ...normalizePointBinding(
+      calculateFocusAndGap(
+        linearElement,
+        hoveredElement,
+        startOrEnd,
+        elementsMap,
+      ),
+      hoveredElement,
+    ),
   };
 
+  if (isElbowArrow(linearElement)) {
+    binding = {
+      ...binding,
+      ...calculateFixedPointForElbowArrowBinding(
+        linearElement,
+        hoveredElement,
+        startOrEnd,
+        elementsMap,
+      ),
+    };
+  }
+
   mutateElement(linearElement, {
     [startOrEnd === "start" ? "startBinding" : "endBinding"]: binding,
   });
@@ -1272,39 +1271,35 @@ 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,
-          ),
+    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),
       ),
-      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" },
-    // );
+      // Fallback when arrow doesn't point to the shape
+      pointFromVector(
+        vectorScale(
+          vectorNormalize(vectorFromPoint(focusPointAbsolute, adjacentPoint)),
+          pointDistance(adjacentPoint, edgePointAbsolute),
+        ),
+        adjacentPoint,
+      ),
+    ];
 
     if (intersections.length > 1) {
       // The adjacent point is outside the shape (+ gap)
@@ -1727,21 +1722,6 @@ 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/element/elbowArrow.ts

@@ -998,23 +998,32 @@ 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
+  const {
+    startBinding: updatedStartBinding,
+    endBinding: updatedEndBinding,
+    ...restOfTheUpdates
+  } = updates;
   const startBinding =
-    typeof updates.startBinding !== "undefined"
-      ? updates.startBinding
+    typeof updatedStartBinding !== "undefined"
+      ? updatedStartBinding
       : arrow.startBinding;
   const endBinding =
-    typeof updates.endBinding !== "undefined"
-      ? updates.endBinding
+    typeof updatedEndBinding !== "undefined"
+      ? updatedEndBinding
       : arrow.endBinding;
   const startElement =
     startBinding &&
     getBindableElementForId(startBinding.elementId, elementsMap);
   const endElement =
     endBinding && getBindableElementForId(endBinding.elementId, elementsMap);
+
   if (
+    (startBinding && !startElement) ||
+    (endBinding && !endElement) ||
     (elementsMap.size === 0 && validateElbowPoints(updatedPoints)) ||
-    startElement?.id !== startBinding?.elementId ||
-    endElement?.id !== endBinding?.elementId
+    (Object.keys(restOfTheUpdates).length === 0 &&
+      (startElement?.id !== startBinding?.elementId ||
+        endElement?.id !== endBinding?.elementId))
   ) {
     return normalizeArrowElementUpdate(
       updatedPoints.map((p) =>
@@ -1074,7 +1083,8 @@ export const updateElbowArrowPoints = (
         p,
         arrow.points[i] ?? pointFrom<LocalPoint>(Infinity, Infinity),
       ),
-    )
+    ) &&
+    validateElbowPoints(updatedPoints)
   ) {
     return {};
   }

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

@@ -818,8 +818,8 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "type": "arrow",
   "updated": 1,
   "version": 30,
-  "width": 50,
-  "x": 200,
+  "width": 0,
+  "x": "149.29289",
   "y": 0,
 }
 `;
@@ -852,7 +852,7 @@ History {
                   0,
                 ],
                 [
-                  50,
+                  0,
                   0,
                 ],
               ],
@@ -937,7 +937,7 @@ History {
                   0,
                 ],
                 [
-                  50,
+                  0,
                   0,
                 ],
               ],