fix(editor): Double history (#11445)

--------- Signed-off-by: Mark Tolmacs <mark@lazycat.hu> Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com>
2026-06-15 18:19:08 +02:00
parent 069982606d
commit 1cb9fff569
4 changed files with 183 additions and 102 deletions
@@ -643,10 +643,13 @@ const getBindingStrategyForDraggingBindingElementEndpoints_simple = (
  let start: BindingStrategy = { mode: undefined };
  let end: BindingStrategy = { mode: undefined };

-  invariant(
-    arrow.points.length > 1,
-    "Do not attempt to bind linear elements with a single point",
-  );
+  if (arrow.points.length < 2) {
+    console.error(
+      "Attempting to bind a linear element with less than 2 points",
+    );
+    // a single-point can't be bound -> cancel
+    return { start: { mode: undefined }, end: { mode: undefined } };
+  }

  // If none of the ends are dragged, we don't change anything
  if (!startDragged && !endDragged) {
@@ -890,10 +893,13 @@ const getBindingStrategyForDraggingBindingElementEndpoints_complex = (
  let start: BindingStrategy = { mode: undefined };
  let end: BindingStrategy = { mode: undefined };

-  invariant(
-    arrow.points.length > 1,
-    "Do not attempt to bind linear elements with a single point",
-  );
+  if (arrow.points.length < 2) {
+    console.error(
+      "Attempting to bind a linear element with less than 2 points",
+    );
+    // a single-point can't be bound -> cancel
+    return { start: { mode: undefined }, end: { mode: undefined } };
+  }

  // If none of the ends are dragged, we don't change anything
  if (!startDragged && !endDragged) {
@@ -178,6 +178,64 @@ describe("binding for simple arrows", () => {
    });
  });

+  describe("self-binding (both ends to the same element) single-click finalize", () => {
+    // rect spans x:200..400, y:200..400; orbit ring is ~15px outside the outline
+    const INSIDE: [number, number] = [250, 250];
+    const ORBIT_LEFT: [number, number] = [187, 300];
+    const ORBIT_RIGHT: [number, number] = [413, 300];
+    const MIDDLE: [number, number] = [550, 100];
+
+    beforeEach(async () => {
+      mouse.reset();
+      await act(() => setLanguage(defaultLang));
+      await render(<Excalidraw handleKeyboardGlobally={true} />);
+      UI.createElement("rectangle", {
+        x: 200,
+        y: 200,
+        width: 200,
+        height: 200,
+      });
+    });
+
+    const drawSelfArrow = (start: [number, number], end: [number, number]) => {
+      UI.clickTool("arrow");
+      mouse.reset();
+      mouse.clickAt(...start);
+      mouse.moveTo(...MIDDLE);
+      mouse.clickAt(...MIDDLE); // commit a middle point so it's a multi-point arrow
+      mouse.moveTo(...end);
+      mouse.clickAt(...end); // single click at the end
+    };
+
+    it("orbit -> orbit finalizes on a single click", () => {
+      drawSelfArrow(ORBIT_LEFT, ORBIT_RIGHT);
+
+      const arrow = h.elements[h.elements.length - 1] as ExcalidrawArrowElement;
+      expect(h.state.multiElement).toBe(null);
+      expect(h.state.activeTool.type).toBe("selection");
+      expect(arrow.startBinding?.elementId).toBe(arrow.endBinding?.elementId);
+      expect(arrow.endBinding?.elementId).not.toBe(undefined);
+    });
+
+    it("inside -> orbit finalizes on a single click", () => {
+      drawSelfArrow(INSIDE, ORBIT_RIGHT);
+
+      const arrow = h.elements[h.elements.length - 1] as ExcalidrawArrowElement;
+      expect(h.state.multiElement).toBe(null);
+      expect(h.state.activeTool.type).toBe("selection");
+      expect(arrow.startBinding?.elementId).toBe(arrow.endBinding?.elementId);
+      expect(arrow.endBinding?.elementId).not.toBe(undefined);
+    });
+
+    it("inside -> inside keep in multi-point mode (no single-click finalize)", () => {
+      drawSelfArrow(INSIDE, [INSIDE[0] + 50, INSIDE[1] + 50]); // end dropped inside the rect
+
+      // ambiguous → must be confirmed with a second click, so still in progress
+      expect(h.state.multiElement).not.toBe(null);
+      expect(h.state.activeTool.type).toBe("arrow");
+    });
+  });
+
  describe("when arrow is outside of shape", () => {
    beforeEach(async () => {
      mouse.reset();
@@ -403,6 +461,7 @@ describe("binding for simple arrows", () => {
      mouse.moveTo(340, 251);
      mouse.moveTo(410, 251);
      mouse.clickAt(410, 251);
+      mouse.clickAt(410, 251);
      const arrow = h.elements[h.elements.length - 1] as any;

      expect(arrow.startBinding?.elementId).toBe(rectLeft.id);
@@ -447,6 +506,7 @@ describe("binding for simple arrows", () => {
      mouse.moveTo(350, 251);
      mouse.moveTo(410, 251);
      mouse.clickAt(410, 251);
+      mouse.clickAt(410, 251);

      const arrow = API.getSelectedElement() as ExcalidrawArrowElement;

@@ -54,6 +54,7 @@ export const actionFinalize = register<FormData>({
  label: "",
  trackEvent: false,
  perform: (elements, appState, data, app) => {
+    let shouldCommit = true;
    let newElements = elements;
    const { interactiveCanvas, focusContainer, scene } = app;
    const elementsMap = scene.getNonDeletedElementsMap();
@@ -222,9 +223,44 @@ export const actionFinalize = register<FormData>({
          !lastCommittedPoint ||
          points[points.length - 1] !== lastCommittedPoint
        ) {
+          shouldCommit = false;
          scene.mutateElement(element, {
            points: element.points.slice(0, -1),
          });
+          if (
+            isBindingElement(element) &&
+            element.endBinding &&
+            // after slicing the trailing point a <2-point arrow may be left
+            element.points.length > 1
+          ) {
+            const newArrow = !!appState.newElement;
+            const draggedPoints: PointsPositionUpdates = new Map([
+              [
+                element.points.length - 1,
+                {
+                  point: element.points[element.points.length - 1],
+                  isDragging: false,
+                },
+              ],
+            ]);
+            const globalPoint =
+              LinearElementEditor.getPointAtIndexGlobalCoordinates(
+                element,
+                -1,
+                elementsMap,
+              );
+            bindOrUnbindBindingElement(
+              element,
+              draggedPoints,
+              globalPoint[0],
+              globalPoint[1],
+              scene,
+              appState,
+              {
+                newArrow,
+              },
+            );
+          }
        }
      }

@@ -344,7 +380,9 @@ export const actionFinalize = register<FormData>({
        selectedLinearElement,
      },
      // TODO: #7348 we should not capture everything, but if we don't, it leads to incosistencies -> revisit
-      captureUpdate: CaptureUpdateAction.IMMEDIATELY,
+      captureUpdate: shouldCommit
+        ? CaptureUpdateAction.IMMEDIATELY
+        : CaptureUpdateAction.NEVER,
    };
  },
  keyTest: (event, appState) =>
@@ -260,6 +260,7 @@ import {
  getUncroppedWidthAndHeight,
  getActiveTextElement,
  isEligibleFrameChildType,
+  getBindingStrategyForDraggingBindingElementEndpoints,
 } from "@excalidraw/element";

 import type { GlobalPoint, LocalPoint, Radians } from "@excalidraw/math";
@@ -7112,45 +7113,7 @@ class App extends React.Component<AppProps, AppState> {
      setCursorForShape(this.interactiveCanvas, this.state);

      if (lastPoint === lastCommittedPoint) {
-        const hoveredElement =
-          isArrowElement(this.state.newElement) &&
-          isBindingEnabled(this.state) &&
-          getHoveredElementForBinding(
-            pointFrom<GlobalPoint>(scenePointerX, scenePointerY),
-            this.scene.getNonDeletedElements(),
-            this.scene.getNonDeletedElementsMap(),
-            maxBindingDistance_simple(this.state.zoom),
-          );
-        if (hoveredElement) {
-          this.actionManager.executeAction(actionFinalize, "ui", {
-            event: event.nativeEvent,
-            sceneCoords: {
-              x: scenePointerX,
-              y: scenePointerY,
-            },
-          });
-          this.setState({ suggestedBinding: null });
-          if (!this.state.activeTool.locked) {
-            resetCursor(this.interactiveCanvas);
-            this.setState((prevState) => ({
-              newElement: null,
-              activeTool: updateActiveTool(this.state, {
-                type: this.state.preferredSelectionTool.type,
-              }),
-              selectedElementIds: makeNextSelectedElementIds(
-                {
-                  ...prevState.selectedElementIds,
-                  [multiElement.id]: true,
-                },
-                prevState,
-              ),
-              selectedLinearElement: new LinearElementEditor(
-                multiElement,
-                this.scene.getNonDeletedElementsMap(),
-              ),
-            }));
-          }
-        } else if (
+        if (
          // if we haven't yet created a temp point and we're beyond commit-zone
          // threshold, add a point
          pointDistance(
@@ -7158,6 +7121,24 @@ class App extends React.Component<AppProps, AppState> {
            lastPoint,
          ) >= LINE_CONFIRM_THRESHOLD
        ) {
+          this.store.scheduleCapture();
+          flushSync(() => {
+            invariant(
+              this.state.selectedLinearElement?.initialState,
+              "initialState must be set",
+            );
+            this.setState({
+              selectedLinearElement: {
+                ...this.state.selectedLinearElement,
+                lastCommittedPoint: points[points.length - 1],
+                selectedPointsIndices: [multiElement.points.length],
+                initialState: {
+                  ...this.state.selectedLinearElement.initialState,
+                  lastClickedPoint: multiElement.points.length,
+                },
+              },
+            });
+          });
          this.scene.mutateElement(
            multiElement,
            {
@@ -7168,21 +7149,6 @@ class App extends React.Component<AppProps, AppState> {
            },
            { informMutation: false, isDragging: false },
          );
-          invariant(
-            this.state.selectedLinearElement?.initialState,
-            "initialState must be set",
-          );
-          this.setState({
-            selectedLinearElement: {
-              ...this.state.selectedLinearElement,
-              lastCommittedPoint: points[points.length - 1],
-              selectedPointsIndices: [multiElement.points.length - 1],
-              initialState: {
-                ...this.state.selectedLinearElement.initialState,
-                lastClickedPoint: multiElement.points.length - 1,
-              },
-            },
-          });
        } else {
          setCursor(this.interactiveCanvas, CURSOR_TYPE.POINTER);
          // in this branch, we're inside the commit zone, and no uncommitted
@@ -9264,32 +9230,58 @@ class App extends React.Component<AppProps, AppState> {

      const { x: rx, y: ry } = multiElement;
      const { lastCommittedPoint } = selectedLinearElement;
+      const sceneCoords = viewportCoordsToSceneCoords(event, this.state);
+      const { start, end } =
+        isBindingElement(multiElement) && isBindingEnabled(this.state)
+          ? getBindingStrategyForDraggingBindingElementEndpoints(
+              multiElement,
+              new Map([
+                [
+                  multiElement.points.length - 1,
+                  {
+                    point: multiElement.points[multiElement.points.length - 1],
+                    isDragging: false,
+                  },
+                ],
+              ]),
+              sceneCoords.x,
+              sceneCoords.y,
+              this.scene.getNonDeletedElementsMap(),
+              this.scene.getNonDeletedElements(),
+              this.state,
+              {
+                newArrow: Boolean(this.state.newElement),
+                zoom: this.state.zoom,
+              },
+            )
+          : { end: { mode: undefined } };

-      const hoveredElementForBinding =
-        isBindingEnabled(this.state) &&
-        getHoveredElementForBinding(
-          pointFrom<GlobalPoint>(
-            this.lastPointerMoveCoords?.x ??
-              rx + multiElement.points[multiElement.points.length - 1][0],
-            this.lastPointerMoveCoords?.y ??
-              ry + multiElement.points[multiElement.points.length - 1][1],
+      const elementsMap = this.scene.getNonDeletedElementsMap();
+      // Auto-confirm when both ends bind to the SAME element and the end point
+      // lands on the outline rather than inside it
+      const endOutsideSameElement =
+        start?.mode != null &&
+        end.mode != null &&
+        start.element.id === end.element.id &&
+        !isPointInElement(end.focusPoint, end.element, elementsMap);
+      const boundOutsideFromElsewhere =
+        end.mode === "orbit" &&
+        multiElement.startBinding?.elementId !== end.element?.id;
+      const lastCommittedPointIsInsideCommitZone =
+        lastCommittedPoint &&
+        pointDistance(
+          pointFrom(
+            pointerDownState.origin.x - rx,
+            pointerDownState.origin.y - ry,
          ),
-          this.scene.getNonDeletedElements(),
-          this.scene.getNonDeletedElementsMap(),
-        );
+          lastCommittedPoint,
+        ) < LINE_CONFIRM_THRESHOLD;

      // clicking inside commit zone → finalize arrow
      if (
-        (isBindingElement(multiElement) && hoveredElementForBinding) ||
-        (multiElement.points.length > 1 &&
-          lastCommittedPoint &&
-          pointDistance(
-            pointFrom(
-              pointerDownState.origin.x - rx,
-              pointerDownState.origin.y - ry,
-            ),
-            lastCommittedPoint,
-          ) < LINE_CONFIRM_THRESHOLD)
+        boundOutsideFromElsewhere || // Outside -> orbit: Bind immediately
+        endOutsideSameElement || // End outside the start's element: Bind immediately
+        (multiElement.points.length > 1 && lastCommittedPointIsInsideCommitZone)
      ) {
        this.actionManager.executeAction(actionFinalize, "ui", {
          event: event.nativeEvent,
@@ -10864,13 +10856,6 @@ class App extends React.Component<AppProps, AppState> {
      }

      if (isLinearElement(newElement)) {
-        if (
-          newElement!.points.length > 1 &&
-          newElement.points[1][0] !== 0 &&
-          newElement.points[1][1] !== 0
-        ) {
-          this.store.scheduleCapture();
-        }
        const pointerCoords = viewportCoordsToSceneCoords(
          childEvent,
          this.state,
@@ -10908,23 +10893,15 @@ class App extends React.Component<AppProps, AppState> {

            this.actionManager.executeAction(actionFinalize);
          } else {
-            const dx = pointerCoords.x - newElement.x;
-            const dy = pointerCoords.y - newElement.y;
-
-            this.scene.mutateElement(
-              newElement,
-              {
-                points: [newElement.points[0], pointFrom<LocalPoint>(dx, dy)],
-              },
-              { informMutation: false, isDragging: false },
-            );
-
+            // Movement out of commit area will create the point
            this.setState({
              multiElement: newElement,
              newElement,
            });
          }
        } else if (pointerDownState.drag.hasOccurred && !multiElement) {
+          this.store.scheduleCapture();
+
          if (isLinearElement(newElement)) {
            this.actionManager.executeAction(actionFinalize, "ui", {
              event: childEvent,