Half done

packages/common/src/utils.ts

@@ -7,7 +7,6 @@ import {
 } from "@excalidraw/math";
 
 import type {
-  ExcalidrawBindableElement,
   ExcalidrawElement,
   FontFamilyValues,
   FontString,
@@ -559,9 +558,6 @@ export const isTransparent = (color: string) => {
   );
 };
 
-export const isBindingFallthroughEnabled = (el: ExcalidrawBindableElement) =>
-  el.fillStyle !== "solid" || isTransparent(el.backgroundColor);
-
 export type ResolvablePromise<T> = Promise<T> & {
   resolve: [T] extends [undefined]
     ? (value?: MaybePromise<Awaited<T>>) => void

packages/element/src/binding.ts

@@ -1,7 +1,6 @@
 import {
   KEYS,
   arrayToMap,
-  isBindingFallthroughEnabled,
   tupleToCoors,
   invariant,
   isDevEnv,
@@ -427,7 +426,7 @@ export const getSuggestedBindingsForArrows = (
 export const maybeBindLinearElement = (
   linearElement: NonDeleted<ExcalidrawLinearElement>,
   appState: AppState,
-  pointerCoords: { x: number; y: number },
+  startOrEnd: "start" | "end",
   elementsMap: NonDeletedSceneElementsMap,
   elements: readonly NonDeletedExcalidrawElement[],
 ): void => {
@@ -441,7 +440,20 @@ export const maybeBindLinearElement = (
   }
 
   const hoveredElement = getHoveredElementForBinding(
-    pointerCoords,
+    tupleToCoors(
+      LinearElementEditor.getPointAtIndexGlobalCoordinates(
+        linearElement,
+        startOrEnd ? 0 : -1,
+        elementsMap,
+      ),
+    ),
+    tupleToCoors(
+      LinearElementEditor.getPointAtIndexGlobalCoordinates(
+        linearElement,
+        startOrEnd ? -1 : 0,
+        elementsMap,
+      ),
+    ),
     elements,
     elementsMap,
     appState.zoom,
@@ -568,6 +580,10 @@ export const getHoveredElementForBinding = (
     x: number;
     y: number;
   },
+  otherPointerCoords: {
+    x: number;
+    y: number;
+  },
   elements: readonly NonDeletedExcalidrawElement[],
   elementsMap: NonDeletedSceneElementsMap,
   zoom?: AppState["zoom"],
@@ -575,59 +591,89 @@ export const getHoveredElementForBinding = (
   considerAllElements?: boolean,
 ): NonDeleted<ExcalidrawBindableElement> | null => {
   if (considerAllElements) {
-    let cullRest = false;
-    const candidateElements = getAllElementsAtPositionForBinding(
-      elements,
-      (element) =>
-        isBindableElement(element, false) &&
-        bindingBorderTest(
-          element,
-          pointerCoords,
-          elementsMap,
-          zoom,
-          (fullShape ||
-            !isBindingFallthroughEnabled(
-              element as ExcalidrawBindableElement,
-            )) &&
-            // disable fullshape snapping for frame elements so we
-            // can bind to frame children
-            !isFrameLikeElement(element),
-        ),
-    ).filter((element) => {
-      if (cullRest) {
-        return false;
-      }
-
-      if (!isBindingFallthroughEnabled(element as ExcalidrawBindableElement)) {
-        cullRest = true;
-      }
+    const otherCandidateElement =
+      getAllElementsAtPositionForBinding(
+        elements,
+        (element) =>
+          isBindableElement(element, false) &&
+          bindingBorderTest(
+            element,
+            otherPointerCoords,
+            elementsMap,
+            zoom,
+            fullShape ||
+              // disable fullshape snapping for frame elements so we
+              // can bind to frame children
+              !isFrameLikeElement(element),
+          ),
+      )
+        .filter(
+          (element): element is NonDeleted<ExcalidrawBindableElement> =>
+            element != null,
+        )
+        // Prefer the shape with the border being tested (if any)
+        .filter(
+          (element, _, arr) =>
+            arr.length <= 1 ||
+            bindingBorderTest(
+              element as NonDeleted<ExcalidrawBindableElement>,
+              otherPointerCoords,
+              elementsMap,
+              zoom,
+              false,
+            ),
+        )
+        // Prefer smaller bindables to be consisent with the check for the other
+        // point
+        .sort(
+          (a, b) =>
+            b.width ** 2 + b.height ** 2 - (a.width ** 2 + a.height ** 2),
+        )
+        .pop() ?? null;
 
-      return true;
-    }) as NonDeleted<ExcalidrawBindableElement>[] | null;
+    const candidateElement =
+      getAllElementsAtPositionForBinding(
+        elements,
+        (element) =>
+          isBindableElement(element, false) &&
+          bindingBorderTest(
+            element,
+            pointerCoords,
+            elementsMap,
+            zoom,
+            fullShape ||
+              // disable fullshape snapping for frame elements so we
+              // can bind to frame children
+              !isFrameLikeElement(element),
+          ),
+      )
+        .filter(
+          (element): element is NonDeleted<ExcalidrawBindableElement> =>
+            element != null,
+        ) // Prefer the shape with the border being tested (if any)
+        .filter(
+          (element, _, arr) =>
+            arr.length <= 1 ||
+            bindingBorderTest(
+              element as NonDeleted<ExcalidrawBindableElement>,
+              pointerCoords,
+              elementsMap,
+              zoom,
+              false,
+            ),
+        )
+        // Prefer smaller bindables
+        .sort(
+          (a, b) =>
+            b.width ** 2 + b.height ** 2 - (a.width ** 2 + a.height ** 2),
+        )
+        .pop() ?? null;
 
-    // Return early if there are no candidates or just one candidate
-    if (!candidateElements || candidateElements.length === 0) {
+    if (otherCandidateElement === candidateElement) {
       return null;
     }
 
-    if (candidateElements.length === 1) {
-      return candidateElements[0] as NonDeleted<ExcalidrawBindableElement>;
-    }
-
-    // Prefer the shape with the border being tested (if any)
-    const borderTestElements = candidateElements.filter((element) =>
-      bindingBorderTest(element, pointerCoords, elementsMap, zoom, false),
-    );
-    if (borderTestElements.length === 1) {
-      return borderTestElements[0];
-    }
-
-    // Prefer smaller shapes
-    return candidateElements
-      .sort(
-        (a, b) => b.width ** 2 + b.height ** 2 - (a.width ** 2 + a.height ** 2),
-      )
-      .pop() as NonDeleted<ExcalidrawBindableElement>;
+    return candidateElement;
   }
 
   const hoveredElement = getElementAtPositionForBinding(
@@ -641,8 +687,7 @@ export const getHoveredElementForBinding = (
         zoom,
         // disable fullshape snapping for frame elements so we
         // can bind to frame children
-        (fullShape || !isBindingFallthroughEnabled(element)) &&
-          !isFrameLikeElement(element),
+        fullShape || !isFrameLikeElement(element),
       ),
   );
 
@@ -1179,34 +1224,39 @@ export const snapToMid = (
 
 export const getOutlineAvoidingPoint = (
   element: NonDeleted<ExcalidrawLinearElement>,
-  coords: GlobalPoint,
-  pointIndex: number,
+  startOrEnd: "start" | "end",
   scene: Scene,
   zoom: AppState["zoom"],
   fallback?: GlobalPoint,
 ): GlobalPoint => {
   const elementsMap = scene.getNonDeletedElementsMap();
+  const elements = scene.getNonDeletedElements();
+  const coords = LinearElementEditor.getPointAtIndexGlobalCoordinates(
+    element,
+    startOrEnd ? 0 : -1,
+    elementsMap,
+  );
   const hoveredElement = getHoveredElementForBinding(
-    { x: coords[0], y: coords[1] },
-    scene.getNonDeletedElements(),
+    tupleToCoors(coords),
+    tupleToCoors(
+      LinearElementEditor.getPointAtIndexGlobalCoordinates(
+        element,
+        startOrEnd ? -1 : 0,
+        elementsMap,
+      ),
+    ),
+    elements,
     elementsMap,
     zoom,
     true,
     isElbowArrow(element),
   );
 
-  if (hoveredElement) {
-    const newPoints = Array.from(element.points);
-    newPoints[pointIndex] = pointFrom<LocalPoint>(
-      coords[0] - element.x,
-      coords[1] - element.y,
-    );
+  const pointIndex = startOrEnd === "start" ? 0 : element.points.length - 1;
 
+  if (hoveredElement) {
     return bindPointToSnapToElementOutline(
-      {
-        ...element,
-        points: newPoints,
-      },
+      element,
       hoveredElement,
       pointIndex === 0 ? "start" : "end",
       elementsMap,

packages/element/tests/binding.test.tsx

@@ -503,4 +503,82 @@ describe("element binding", () => {
       });
     });
   });
+
+  // UX RATIONALE: The arrow might be outside of the shape at high zoom and you
+  // won't see what's going on.
+  it(
+    "allow non-binding simple (complex) arrow creation while start and end" +
+      " points are in the same shape",
+    () => {
+      UI.createElement("rectangle", {
+        x: 0,
+        y: 0,
+        width: 100,
+        height: 100,
+      });
+
+      const arrow = UI.createElement("arrow", {
+        x: 5,
+        y: 5,
+        height: 95,
+        width: 95,
+      });
+
+      expect(arrow.startBinding).toBe(null);
+      expect(arrow.endBinding).toBe(null);
+      expect(arrow.points).toCloselyEqualPoints([
+        [0, 0],
+        [95, 95],
+      ]);
+
+      const rect2 = API.createElement({
+        type: "rectangle",
+        x: 300,
+        y: 300,
+        width: 100,
+        height: 100,
+        backgroundColor: "red",
+        fillStyle: "solid",
+      });
+
+      API.setElements([rect2]);
+
+      const arrow2 = UI.createElement("arrow", {
+        x: 305,
+        y: 305,
+        height: 95,
+        width: 95,
+      });
+
+      expect(arrow2.startBinding).toBe(null);
+      expect(arrow2.endBinding).toBe(null);
+      expect(arrow2.points).toCloselyEqualPoints([
+        [0, 0],
+        [95, 95],
+      ]);
+
+      UI.createElement("rectangle", {
+        x: 0,
+        y: 0,
+        width: 100,
+        height: 100,
+      });
+
+      const arrow3 = UI.createElement("arrow", {
+        x: 5,
+        y: 5,
+        height: 95,
+        width: 95,
+        elbowed: true,
+      });
+
+      expect(arrow3.startBinding).toBe(null);
+      expect(arrow3.endBinding).toBe(null);
+      expect(arrow3.points).toCloselyEqualPoints([
+        [0, 0],
+        [45, 45],
+        [95, 95],
+      ]);
+    },
+  );
 });

packages/excalidraw/components/App.tsx

@@ -2919,14 +2919,8 @@ class App extends React.Component<AppProps, AppState> {
       maybeBindLinearElement(
         multiElement,
         this.state,
-        tupleToCoors(
-          LinearElementEditor.getPointAtIndexGlobalCoordinates(
-            multiElement,
-            -1,
-            nonDeletedElementsMap,
-          ),
-        ),
-        this.scene.getNonDeletedElementsMap(),
+        "end",
+        nonDeletedElementsMap,
         this.scene.getNonDeletedElements(),
       );
     }
@@ -5976,8 +5970,7 @@ class App extends React.Component<AppProps, AppState> {
               toLocalPoint(
                 getOutlineAvoidingPoint(
                   multiElement,
-                  pointFrom<GlobalPoint>(scenePointerX, scenePointerY),
-                  multiElement.points.length - 1,
+                  "end",
                   this.scene,
                   this.state.zoom,
                   pointFrom<GlobalPoint>(
@@ -8667,7 +8660,16 @@ class App extends React.Component<AppProps, AppState> {
                   ...points.slice(0, -1),
                   toLocalPoint(
                     getOutlineAvoidingPoint(
-                      newElement,
+                      {
+                        ...newElement,
+                        points: [
+                          ...points.slice(0, -1),
+                          pointFrom<LocalPoint>(
+                            pointerCoords.x - newElement.x,
+                            pointerCoords.y - newElement.y,
+                          ),
+                        ],
+                      },
                       pointFrom<GlobalPoint>(pointerCoords.x, pointerCoords.y),
                       newElement.points.length - 1,
                       this.scene,
@@ -9091,7 +9093,7 @@ class App extends React.Component<AppProps, AppState> {
             maybeBindLinearElement(
               newElement,
               this.state,
-              pointerCoords,
+              "end",
               this.scene.getNonDeletedElementsMap(),
               this.scene.getNonDeletedElements(),
             );

packages/excalidraw/tests/helpers/ui.ts

@@ -464,6 +464,7 @@ export class UI {
       height: initialHeight = initialWidth,
       angle = 0,
       points: initialPoints,
+      elbowed = false,
     }: {
       position?: number;
       x?: number;
@@ -473,6 +474,7 @@ export class UI {
       height?: number;
       angle?: number;
       points?: T extends "line" | "arrow" | "freedraw" ? LocalPoint[] : never;
+      elbowed?: boolean;
     } = {},
   ): Element<T> & {
     /** Returns the actual, current element from the elements array, instead
@@ -491,6 +493,17 @@ export class UI {
     if (type === "text") {
       mouse.reset();
       mouse.click(x, y);
+    } else if (type === "arrow" && points.length === 2 && elbowed) {
+      UI.clickOnTestId("elbow-arrow");
+      mouse.reset();
+      mouse.moveTo(x + points[0][0], y + points[0][1]);
+      mouse.click();
+      mouse.moveTo(
+        x + points[points.length - 1][0],
+        y + points[points.length - 1][1],
+      );
+      mouse.click();
+      Keyboard.keyPress(KEYS.ESCAPE);
     } else if ((type === "line" || type === "arrow") && points.length > 2) {
       points.forEach((point) => {
         mouse.reset();