fix: Elbow arrow z-index binding (#9067)

packages/excalidraw/actions/actionProperties.tsx

@@ -1605,6 +1605,8 @@ export const actionChangeArrowType = register({
             elements,
             elementsMap,
             appState.zoom,
+            false,
+            true,
           );
         const endHoveredElement =
           !newElement.endBinding &&
@@ -1613,6 +1615,8 @@ export const actionChangeArrowType = register({
             elements,
             elementsMap,
             appState.zoom,
+            false,
+            true,
           );
         const startElement = startHoveredElement
           ? startHoveredElement

packages/excalidraw/components/App.tsx

@@ -5770,7 +5770,10 @@ class App extends React.Component<AppProps, AppState> {
         });
       }
       if (editingLinearElement?.lastUncommittedPoint != null) {
-        this.maybeSuggestBindingAtCursor(scenePointer);
+        this.maybeSuggestBindingAtCursor(
+          scenePointer,
+          editingLinearElement.elbowed,
+        );
       } else {
         // causes stack overflow if not sync
         flushSync(() => {
@@ -5790,7 +5793,7 @@ class App extends React.Component<AppProps, AppState> {
           this.state.startBoundElement,
         );
       } else {
-        this.maybeSuggestBindingAtCursor(scenePointer);
+        this.maybeSuggestBindingAtCursor(scenePointer, false);
       }
     }
 
@@ -7728,6 +7731,7 @@ class App extends React.Component<AppProps, AppState> {
         this.scene.getNonDeletedElementsMap(),
         this.state.zoom,
         isElbowArrow(element),
+        isElbowArrow(element),
       );
 
       this.scene.insertElement(element);
@@ -10005,15 +10009,20 @@ class App extends React.Component<AppProps, AppState> {
     }
   };
 
-  private maybeSuggestBindingAtCursor = (pointerCoords: {
-    x: number;
-    y: number;
-  }): void => {
+  private maybeSuggestBindingAtCursor = (
+    pointerCoords: {
+      x: number;
+      y: number;
+    },
+    considerAll: boolean,
+  ): void => {
     const hoveredBindableElement = getHoveredElementForBinding(
       pointerCoords,
       this.scene.getNonDeletedElements(),
       this.scene.getNonDeletedElementsMap(),
       this.state.zoom,
+      false,
+      considerAll,
     );
     this.setState({
       suggestedBindings:
@@ -10043,7 +10052,8 @@ class App extends React.Component<AppProps, AppState> {
           this.scene.getNonDeletedElements(),
           this.scene.getNonDeletedElementsMap(),
           this.state.zoom,
-          isArrowElement(linearElement) && isElbowArrow(linearElement),
+          isElbowArrow(linearElement),
+          isElbowArrow(linearElement),
         );
         if (
           hoveredBindableElement != null &&

packages/excalidraw/element/binding.ts

@@ -49,7 +49,11 @@ import type { ElementUpdate } from "./mutateElement";
 import { mutateElement } from "./mutateElement";
 import type Scene from "../scene/Scene";
 import { LinearElementEditor } from "./linearElementEditor";
-import { arrayToMap, tupleToCoors } from "../utils";
+import {
+  arrayToMap,
+  isBindingFallthroughEnabled,
+  tupleToCoors,
+} from "../utils";
 import { KEYS } from "../keys";
 import { getBoundTextElement, handleBindTextResize } from "./textElement";
 import { aabbForElement, getElementShape, pointInsideBounds } from "../shapes";
@@ -75,6 +79,7 @@ import {
   clamp,
 } from "../../math";
 import { segmentIntersectRectangleElement } from "../../utils/geometry/shape";
+import { getElementsAtPosition } from "../scene/comparisons";
 
 export type SuggestedBinding =
   | NonDeleted<ExcalidrawBindableElement>
@@ -425,7 +430,8 @@ export const maybeBindLinearElement = (
     elements,
     elementsMap,
     appState.zoom,
-    isElbowArrow(linearElement) && isElbowArrow(linearElement),
+    isElbowArrow(linearElement),
+    isElbowArrow(linearElement),
   );
 
   if (hoveredElement !== null) {
@@ -558,7 +564,64 @@ export const getHoveredElementForBinding = (
   elementsMap: NonDeletedSceneElementsMap,
   zoom?: AppState["zoom"],
   fullShape?: boolean,
+  considerAllElements?: boolean,
 ): NonDeleted<ExcalidrawBindableElement> | null => {
+  if (considerAllElements) {
+    let cullRest = false;
+    const candidateElements = getElementsAtPosition(
+      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;
+      }
+
+      return true;
+    }) as NonDeleted<ExcalidrawBindableElement>[] | null;
+
+    // Return early if there are no candidates or just one candidate
+    if (!candidateElements || candidateElements.length === 0) {
+      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>;
+  }
+
   const hoveredElement = getElementAtPosition(
     elements,
     (element) =>
@@ -570,7 +633,8 @@ export const getHoveredElementForBinding = (
         zoom,
         // disable fullshape snapping for frame elements so we
         // can bind to frame children
-        fullShape && !isFrameLikeElement(element),
+        (fullShape || !isBindingFallthroughEnabled(element)) &&
+          !isFrameLikeElement(element),
       ),
   );
 
@@ -1220,6 +1284,8 @@ const getElligibleElementForBindingElement = (
     elements,
     elementsMap,
     zoom,
+    isElbowArrow(linearElement),
+    isElbowArrow(linearElement),
   );
 };
 

packages/excalidraw/element/elbowArrow.ts

@@ -20,11 +20,11 @@ import {
   bindPointToSnapToElementOutline,
   distanceToBindableElement,
   avoidRectangularCorner,
-  getHoveredElementForBinding,
   FIXED_BINDING_DISTANCE,
   getHeadingForElbowArrowSnap,
   getGlobalFixedPointForBindableElement,
   snapToMid,
+  getHoveredElementForBinding,
 } from "./binding";
 import type { Bounds } from "./bounds";
 import type { Heading } from "./heading";
@@ -872,6 +872,8 @@ export const updateElbowArrowPoints = (
   updates: {
     points?: readonly LocalPoint[];
     fixedSegments?: FixedSegment[] | null;
+    startBinding?: FixedPointBinding | null;
+    endBinding?: FixedPointBinding | null;
   },
   options?: {
     isDragging?: boolean;
@@ -953,17 +955,48 @@ export const updateElbowArrowPoints = (
     hoveredStartElement,
     hoveredEndElement,
     ...rest
-  } = getElbowArrowData(arrow, elementsMap, updatedPoints, options);
+  } = getElbowArrowData(
+    {
+      x: arrow.x,
+      y: arrow.y,
+      startBinding:
+        typeof updates.startBinding !== "undefined"
+          ? updates.startBinding
+          : arrow.startBinding,
+      endBinding:
+        typeof updates.endBinding !== "undefined"
+          ? updates.endBinding
+          : arrow.endBinding,
+      startArrowhead: arrow.startArrowhead,
+      endArrowhead: arrow.endArrowhead,
+    },
+    elementsMap,
+    updatedPoints,
+    options,
+  );
 
   const fixedSegments = updates.fixedSegments ?? arrow.fixedSegments ?? [];
 
   ////
   // 1. Renormalize the arrow
   ////
-  if (!updates.points && !updates.fixedSegments) {
+  if (
+    !updates.points &&
+    !updates.fixedSegments &&
+    !updates.startBinding &&
+    !updates.endBinding
+  ) {
     return handleSegmentRenormalization(arrow, elementsMap);
   }
 
+  // Short circuit on no-op to avoid huge performance hit
+  if (
+    updates.startBinding === arrow.startBinding &&
+    updates.endBinding === arrow.endBinding
+  ) {
+    return {};
+  }
+
   ////
   // 2. Just normal elbow arrow things
   ////
@@ -1010,6 +1043,7 @@ export const updateElbowArrowPoints = (
 
   ////
   // 5. Handle resize
+  ////
   if (updates.points && updates.fixedSegments) {
     return updates;
   }
@@ -2120,6 +2154,7 @@ const getHoveredElements = (
       nonDeletedSceneElementsMap,
       zoom,
       true,
+      true,
     ),
     getHoveredElementForBinding(
       tupleToCoors(origEndGlobalPoint),
@@ -2127,6 +2162,7 @@ const getHoveredElements = (
       nonDeletedSceneElementsMap,
       zoom,
       true,
+      true,
     ),
   ];
 };

packages/excalidraw/element/linearElementEditor.ts

@@ -444,6 +444,8 @@ export class LinearElementEditor {
                 elements,
                 elementsMap,
                 appState.zoom,
+                isElbowArrow(element),
+                isElbowArrow(element),
               )
             : null;
 
@@ -796,6 +798,7 @@ export class LinearElementEditor {
           elements,
           elementsMap,
           app.state.zoom,
+          linearElementEditor.elbowed,
         ),
       };
 

packages/excalidraw/element/mutateElement.ts

@@ -33,13 +33,16 @@ export const mutateElement = <TElement extends Mutable<ExcalidrawElement>>(
 
   // casting to any because can't use `in` operator
   // (see https://github.com/microsoft/TypeScript/issues/21732)
-  const { points, fixedSegments, fileId } = updates as any;
+  const { points, fixedSegments, fileId, startBinding, endBinding } =
+    updates as any;
 
   if (
     isElbowArrow(element) &&
     (Object.keys(updates).length === 0 || // normalization case
       typeof points !== "undefined" || // repositioning
-      typeof fixedSegments !== "undefined") // segment fixing
+      typeof fixedSegments !== "undefined" || // segment fixing
+      typeof startBinding !== "undefined" ||
+      typeof endBinding !== "undefined") // manual binding to element
   ) {
     const elementsMap = toBrandedType<SceneElementsMap>(
       Scene.getScene(element)?.getNonDeletedElementsMap() ?? new Map(),
@@ -58,6 +61,8 @@ export const mutateElement = <TElement extends Mutable<ExcalidrawElement>>(
         {
           fixedSegments,
           points,
+          startBinding,
+          endBinding,
         },
         {
           isDragging: options?.isDragging,

packages/excalidraw/scene/comparisons.ts

@@ -75,20 +75,23 @@ export const getElementsAtPosition = (
   isAtPositionFn: (element: NonDeletedExcalidrawElement) => boolean,
 ) => {
   const iframeLikes: ExcalidrawIframeElement[] = [];
-  // The parameter elements comes ordered from lower z-index to higher.
-  // We want to preserve that order on the returned array.
-  // Exception being embeddables which should be on top of everything else in
-  // terms of hit testing.
-  const elsAtPos = elements.filter((element) => {
-    const hit = !element.isDeleted && isAtPositionFn(element);
-    if (hit) {
-      if (isIframeElement(element)) {
-        iframeLikes.push(element);
-        return false;
-      }
-      return true;
+  const elementsAtPosition: NonDeletedExcalidrawElement[] = [];
+  // We need to to hit testing from front (end of the array) to back (beginning of the array)
+  // because array is ordered from lower z-index to highest and we want element z-index
+  // with higher z-index
+  for (let index = elements.length - 1; index >= 0; --index) {
+    const element = elements[index];
+    if (element.isDeleted) {
+      continue;
     }
-    return false;
-  });
-  return elsAtPos.concat(iframeLikes);
+    if (isIframeElement(element)) {
+      iframeLikes.push(element);
+      continue;
+    }
+    if (isAtPositionFn(element)) {
+      elementsAtPosition.push(element);
+    }
+  }
+
+  return elementsAtPosition.concat(iframeLikes);
 };

packages/excalidraw/utils.ts

@@ -9,7 +9,11 @@ import {
   isDarwin,
   WINDOWS_EMOJI_FALLBACK_FONT,
 } from "./constants";
-import type { FontFamilyValues, FontString } from "./element/types";
+import type {
+  ExcalidrawBindableElement,
+  FontFamilyValues,
+  FontString,
+} from "./element/types";
 import type {
   ActiveTool,
   AppState,
@@ -543,6 +547,9 @@ 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