fix: Bindings at partially overlapping binding areas (#9536)

packages/element/src/binding.ts

@@ -384,6 +384,48 @@ export const getSuggestedBindingsForArrows = (
   );
 };
 
+export const maybeSuggestBindingsForLinearElementAtCoords = (
+  linearElement: NonDeleted<ExcalidrawLinearElement>,
+  /** scene coords */
+  pointerCoords: {
+    x: number;
+    y: number;
+  }[],
+  scene: Scene,
+  zoom: AppState["zoom"],
+  // During line creation the start binding hasn't been written yet
+  // into `linearElement`
+  oppositeBindingBoundElement?: ExcalidrawBindableElement | null,
+): ExcalidrawBindableElement[] =>
+  Array.from(
+    pointerCoords.reduce(
+      (acc: Set<NonDeleted<ExcalidrawBindableElement>>, coords) => {
+        const hoveredBindableElement = getHoveredElementForBinding(
+          coords,
+          scene.getNonDeletedElements(),
+          scene.getNonDeletedElementsMap(),
+          zoom,
+          isElbowArrow(linearElement),
+          isElbowArrow(linearElement),
+        );
+
+        if (
+          hoveredBindableElement != null &&
+          !isLinearElementSimpleAndAlreadyBound(
+            linearElement,
+            oppositeBindingBoundElement?.id,
+            hoveredBindableElement,
+          )
+        ) {
+          acc.add(hoveredBindableElement);
+        }
+
+        return acc;
+      },
+      new Set() as Set<NonDeleted<ExcalidrawBindableElement>>,
+    ),
+  );
+
 export const maybeBindLinearElement = (
   linearElement: NonDeleted<ExcalidrawLinearElement>,
   appState: AppState,
@@ -513,7 +555,7 @@ export const isLinearElementSimpleAndAlreadyBound = (
 
 const isLinearElementSimple = (
   linearElement: NonDeleted<ExcalidrawLinearElement>,
-): boolean => linearElement.points.length < 3;
+): boolean => linearElement.points.length < 3 && !isElbowArrow(linearElement);
 
 const unbindLinearElement = (
   linearElement: NonDeleted<ExcalidrawLinearElement>,

packages/element/src/linearElementEditor.ts

@@ -20,6 +20,7 @@ import {
   getGridPoint,
   invariant,
   tupleToCoors,
+  viewportCoordsToSceneCoords,
 } from "@excalidraw/common";
 
 import {
@@ -45,6 +46,7 @@ import {
   bindOrUnbindLinearElement,
   getHoveredElementForBinding,
   isBindingEnabled,
+  maybeSuggestBindingsForLinearElementAtCoords,
 } from "./binding";
 import {
   getElementAbsoluteCoords,
@@ -275,18 +277,13 @@ export class LinearElementEditor {
     app: AppClassProperties,
     scenePointerX: number,
     scenePointerY: number,
-    maybeSuggestBinding: (
-      element: NonDeleted<ExcalidrawLinearElement>,
-      pointSceneCoords: { x: number; y: number }[],
-    ) => void,
     linearElementEditor: LinearElementEditor,
-    scene: Scene,
-  ): LinearElementEditor | null {
+  ): Pick<AppState, keyof AppState> | null {
     if (!linearElementEditor) {
       return null;
     }
     const { elementId } = linearElementEditor;
-    const elementsMap = scene.getNonDeletedElementsMap();
+    const elementsMap = app.scene.getNonDeletedElementsMap();
     const element = LinearElementEditor.getElement(elementId, elementsMap);
     let customLineAngle = linearElementEditor.customLineAngle;
     if (!element) {
@@ -347,7 +344,7 @@ export class LinearElementEditor {
 
         LinearElementEditor.movePoints(
           element,
-          scene,
+          app.scene,
           new Map([
             [
               selectedIndex,
@@ -375,7 +372,7 @@ export class LinearElementEditor {
 
         LinearElementEditor.movePoints(
           element,
-          scene,
+          app.scene,
           new Map(
             selectedPointsIndices.map((pointIndex) => {
               const newPointPosition: LocalPoint =
@@ -407,46 +404,59 @@ export class LinearElementEditor {
 
       const boundTextElement = getBoundTextElement(element, elementsMap);
       if (boundTextElement) {
-        handleBindTextResize(element, scene, false);
+        handleBindTextResize(element, app.scene, false);
       }
 
       // suggest bindings for first and last point if selected
+      let suggestedBindings: ExcalidrawBindableElement[] = [];
       if (isBindingElement(element, false)) {
+        const firstSelectedIndex = selectedPointsIndices[0] === 0;
+        const lastSelectedIndex =
+          selectedPointsIndices[selectedPointsIndices.length - 1] ===
+          element.points.length - 1;
         const coords: { x: number; y: number }[] = [];
 
-        const firstSelectedIndex = selectedPointsIndices[0];
-        if (firstSelectedIndex === 0) {
-          coords.push(
-            tupleToCoors(
-              LinearElementEditor.getPointGlobalCoordinates(
-                element,
-                element.points[0],
-                elementsMap,
+        if (!firstSelectedIndex !== !lastSelectedIndex) {
+          coords.push({ x: scenePointerX, y: scenePointerY });
+        } else {
+          if (firstSelectedIndex) {
+            coords.push(
+              tupleToCoors(
+                LinearElementEditor.getPointGlobalCoordinates(
+                  element,
+                  element.points[0],
+                  elementsMap,
+                ),
               ),
-            ),
-          );
-        }
+            );
+          }
 
-        const lastSelectedIndex =
-          selectedPointsIndices[selectedPointsIndices.length - 1];
-        if (lastSelectedIndex === element.points.length - 1) {
-          coords.push(
-            tupleToCoors(
-              LinearElementEditor.getPointGlobalCoordinates(
-                element,
-                element.points[lastSelectedIndex],
-                elementsMap,
+          if (lastSelectedIndex) {
+            coords.push(
+              tupleToCoors(
+                LinearElementEditor.getPointGlobalCoordinates(
+                  element,
+                  element.points[
+                    selectedPointsIndices[selectedPointsIndices.length - 1]
+                  ],
+                  elementsMap,
+                ),
               ),
-            ),
-          );
+            );
+          }
         }
 
         if (coords.length) {
-          maybeSuggestBinding(element, coords);
+          suggestedBindings = maybeSuggestBindingsForLinearElementAtCoords(
+            element,
+            coords,
+            app.scene,
+            app.state.zoom,
+          );
         }
       }
 
-      return {
+      const newLinearElementEditor = {
         ...linearElementEditor,
         selectedPointsIndices,
         segmentMidPointHoveredCoords:
@@ -466,6 +476,15 @@ export class LinearElementEditor {
         isDragging: true,
         customLineAngle,
       };
+
+      return {
+        ...app.state,
+        editingLinearElement: app.state.editingLinearElement
+          ? newLinearElementEditor
+          : null,
+        selectedLinearElement: newLinearElementEditor,
+        suggestedBindings,
+      };
     }
 
     return null;
@@ -479,6 +498,7 @@ export class LinearElementEditor {
   ): LinearElementEditor {
     const elementsMap = scene.getNonDeletedElementsMap();
     const elements = scene.getNonDeletedElements();
+    const pointerCoords = viewportCoordsToSceneCoords(event, appState);
 
     const { elementId, selectedPointsIndices, isDragging, pointerDownState } =
       editingLinearElement;
@@ -534,13 +554,15 @@ export class LinearElementEditor {
 
           const bindingElement = isBindingEnabled(appState)
             ? getHoveredElementForBinding(
-                tupleToCoors(
-                  LinearElementEditor.getPointAtIndexGlobalCoordinates(
-                    element,
-                    selectedPoint!,
-                    elementsMap,
-                  ),
-                ),
+                (selectedPointsIndices?.length ?? 0) > 1
+                  ? tupleToCoors(
+                      LinearElementEditor.getPointAtIndexGlobalCoordinates(
+                        element,
+                        selectedPoint!,
+                        elementsMap,
+                      ),
+                    )
+                  : pointerCoords,
                 elements,
                 elementsMap,
                 appState.zoom,

packages/excalidraw/actions/actionFinalize.tsx

@@ -14,7 +14,12 @@ import {
   isLineElement,
 } from "@excalidraw/element";
 
-import { KEYS, arrayToMap, updateActiveTool } from "@excalidraw/common";
+import {
+  KEYS,
+  arrayToMap,
+  tupleToCoors,
+  updateActiveTool,
+} from "@excalidraw/common";
 import { isPathALoop } from "@excalidraw/element";
 
 import { isInvisiblySmallElement } from "@excalidraw/element";
@@ -43,12 +48,16 @@ export const actionFinalize = register({
   trackEvent: false,
   perform: (elements, appState, data, app) => {
     const { interactiveCanvas, focusContainer, scene } = app;
-
+    const { event, sceneCoords } =
+      (data as {
+        event?: PointerEvent;
+        sceneCoords?: { x: number; y: number };
+      }) ?? {};
     const elementsMap = scene.getNonDeletedElementsMap();
 
-    if (data?.event && appState.selectedLinearElement) {
+    if (event && appState.selectedLinearElement) {
       const linearElementEditor = LinearElementEditor.handlePointerUp(
-        data.event,
+        event,
         appState.selectedLinearElement,
         appState,
         app.scene,
@@ -204,12 +213,17 @@ export const actionFinalize = register({
           element.points.length > 1 &&
           isBindingEnabled(appState)
         ) {
-          const [x, y] = LinearElementEditor.getPointAtIndexGlobalCoordinates(
-            element,
-            -1,
-            arrayToMap(elements),
-          );
-          maybeBindLinearElement(element, appState, { x, y }, scene);
+          const coords =
+            sceneCoords ??
+            tupleToCoors(
+              LinearElementEditor.getPointAtIndexGlobalCoordinates(
+                element,
+                -1,
+                arrayToMap(elements),
+              ),
+            );
+
+          maybeBindLinearElement(element, appState, coords, scene);
         }
       }
     }

packages/excalidraw/components/App.tsx

@@ -105,12 +105,12 @@ import {
 import {
   getObservedAppState,
   getCommonBounds,
+  maybeSuggestBindingsForLinearElementAtCoords,
   getElementAbsoluteCoords,
   bindOrUnbindLinearElements,
   fixBindingsAfterDeletion,
   getHoveredElementForBinding,
   isBindingEnabled,
-  isLinearElementSimpleAndAlreadyBound,
   shouldEnableBindingForPointerEvent,
   updateBoundElements,
   getSuggestedBindingsForArrows,
@@ -237,7 +237,6 @@ import {
 import type { LocalPoint, Radians } from "@excalidraw/math";
 
 import type {
-  ExcalidrawBindableElement,
   ExcalidrawElement,
   ExcalidrawFreeDrawElement,
   ExcalidrawGenericElement,
@@ -5883,11 +5882,15 @@ class App extends React.Component<AppProps, AppState> {
       // and point
       const { newElement } = this.state;
       if (isBindingElement(newElement, false)) {
-        this.maybeSuggestBindingsForLinearElementAtCoords(
-          newElement,
-          [scenePointer],
-          this.state.startBoundElement,
-        );
+        this.setState({
+          suggestedBindings: maybeSuggestBindingsForLinearElementAtCoords(
+            newElement,
+            [scenePointer],
+            this.scene,
+            this.state.zoom,
+            this.state.startBoundElement,
+          ),
+        });
       } else {
         this.maybeSuggestBindingAtCursor(scenePointer, false);
       }
@@ -8217,31 +8220,19 @@ class App extends React.Component<AppProps, AppState> {
           return;
         }
 
-        const newLinearElementEditor = LinearElementEditor.handlePointDragging(
+        const newState = LinearElementEditor.handlePointDragging(
           event,
           this,
           pointerCoords.x,
           pointerCoords.y,
-          (element, pointsSceneCoords) => {
-            this.maybeSuggestBindingsForLinearElementAtCoords(
-              element,
-              pointsSceneCoords,
-            );
-          },
           linearElementEditor,
-          this.scene,
         );
-        if (newLinearElementEditor) {
+        if (newState) {
           pointerDownState.lastCoords.x = pointerCoords.x;
           pointerDownState.lastCoords.y = pointerCoords.y;
           pointerDownState.drag.hasOccurred = true;
 
-          this.setState({
-            editingLinearElement: this.state.editingLinearElement
-              ? newLinearElementEditor
-              : null,
-            selectedLinearElement: newLinearElementEditor,
-          });
+          this.setState(newState);
 
           return;
         }
@@ -8720,11 +8711,15 @@ class App extends React.Component<AppProps, AppState> {
 
           if (isBindingElement(newElement, false)) {
             // When creating a linear element by dragging
-            this.maybeSuggestBindingsForLinearElementAtCoords(
-              newElement,
-              [pointerCoords],
-              this.state.startBoundElement,
-            );
+            this.setState({
+              suggestedBindings: maybeSuggestBindingsForLinearElementAtCoords(
+                newElement,
+                [pointerCoords],
+                this.scene,
+                this.state.zoom,
+                this.state.startBoundElement,
+              ),
+            });
           }
         } else {
           pointerDownState.lastCoords.x = pointerCoords.x;
@@ -8919,16 +8914,17 @@ class App extends React.Component<AppProps, AppState> {
 
       const hitElements = pointerDownState.hit.allHitElements;
 
+      const sceneCoords = viewportCoordsToSceneCoords(
+        { clientX: childEvent.clientX, clientY: childEvent.clientY },
+        this.state,
+      );
+
       if (
         this.state.activeTool.type === "selection" &&
         !pointerDownState.boxSelection.hasOccurred &&
         !pointerDownState.resize.isResizing &&
         !hitElements.some((el) => this.state.selectedElementIds[el.id])
       ) {
-        const sceneCoords = viewportCoordsToSceneCoords(
-          { clientX: childEvent.clientX, clientY: childEvent.clientY },
-          this.state,
-        );
         const hitLockedElement = this.getElementAtPosition(
           sceneCoords.x,
           sceneCoords.y,
@@ -9029,6 +9025,7 @@ class App extends React.Component<AppProps, AppState> {
         } else if (this.state.selectedLinearElement.isDragging) {
           this.actionManager.executeAction(actionFinalize, "ui", {
             event: childEvent,
+            sceneCoords,
           });
         }
       }
@@ -9123,7 +9120,10 @@ class App extends React.Component<AppProps, AppState> {
             isBindingEnabled(this.state) &&
             isBindingElement(newElement, false)
           ) {
-            this.actionManager.executeAction(actionFinalize);
+            this.actionManager.executeAction(actionFinalize, "ui", {
+              event: childEvent,
+              sceneCoords,
+            });
           }
           this.setState({ suggestedBindings: [], startBoundElement: null });
           if (!activeTool.locked) {
@@ -9706,7 +9706,8 @@ class App extends React.Component<AppProps, AppState> {
       }
 
       if (
-        pointerDownState.drag.hasOccurred ||
+        (pointerDownState.drag.hasOccurred &&
+          !this.state.selectedLinearElement) ||
         isResizing ||
         isRotating ||
         isCropping
@@ -10172,49 +10173,6 @@ class App extends React.Component<AppProps, AppState> {
     });
   };
 
-  private maybeSuggestBindingsForLinearElementAtCoords = (
-    linearElement: NonDeleted<ExcalidrawLinearElement>,
-    /** scene coords */
-    pointerCoords: {
-      x: number;
-      y: number;
-    }[],
-    // During line creation the start binding hasn't been written yet
-    // into `linearElement`
-    oppositeBindingBoundElement?: ExcalidrawBindableElement | null,
-  ): void => {
-    if (!pointerCoords.length) {
-      return;
-    }
-
-    const suggestedBindings = pointerCoords.reduce(
-      (acc: NonDeleted<ExcalidrawBindableElement>[], coords) => {
-        const hoveredBindableElement = getHoveredElementForBinding(
-          coords,
-          this.scene.getNonDeletedElements(),
-          this.scene.getNonDeletedElementsMap(),
-          this.state.zoom,
-          isElbowArrow(linearElement),
-          isElbowArrow(linearElement),
-        );
-        if (
-          hoveredBindableElement != null &&
-          !isLinearElementSimpleAndAlreadyBound(
-            linearElement,
-            oppositeBindingBoundElement?.id,
-            hoveredBindableElement,
-          )
-        ) {
-          acc.push(hoveredBindableElement);
-        }
-        return acc;
-      },
-      [],
-    );
-
-    this.setState({ suggestedBindings });
-  };
-
   private clearSelection(hitElement: ExcalidrawElement | null): void {
     this.setState((prevState) => ({
       selectedElementIds: makeNextSelectedElementIds({}, prevState),

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

@@ -224,7 +224,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 37,
+  "version": 35,
   "width": "98.40611",
   "x": 1,
   "y": 0,
@@ -348,7 +348,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
               "focus": "0.02970",
               "gap": 1,
             },
-            "version": 35,
+            "version": 33,
           },
           "inserted": {
             "endBinding": {
@@ -372,7 +372,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
               "focus": "0.02000",
               "gap": 1,
             },
-            "version": 32,
+            "version": 30,
           },
         },
       },
@@ -427,7 +427,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
               ],
             ],
             "startBinding": null,
-            "version": 37,
+            "version": 35,
             "y": 0,
           },
           "inserted": {
@@ -447,7 +447,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
               "focus": "0.02970",
               "gap": 1,
             },
-            "version": 35,
+            "version": 33,
             "y": "35.82151",
           },
         },
@@ -828,7 +828,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "strokeWidth": 2,
   "type": "arrow",
   "updated": 1,
-  "version": 33,
+  "version": 31,
   "width": 0,
   "x": 149,
   "y": 0,
@@ -878,7 +878,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
         "id4": {
           "deleted": {
             "endBinding": null,
-            "version": 32,
+            "version": 30,
           },
           "inserted": {
             "endBinding": {
@@ -886,7 +886,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
               "focus": -0,
               "gap": 1,
             },
-            "version": 30,
+            "version": 28,
           },
         },
       },
@@ -922,7 +922,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
         "id4": {
           "deleted": {
             "startBinding": null,
-            "version": 33,
+            "version": 31,
           },
           "inserted": {
             "startBinding": {
@@ -930,7 +930,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
               "focus": 0,
               "gap": 1,
             },
-            "version": 32,
+            "version": 30,
           },
         },
       },