Restore master

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

packages/element/src/binding.ts

@@ -27,9 +27,7 @@ import {
   PRECISION,
 } from "@excalidraw/math";
 
-import { isPointInShape } from "@excalidraw/utils/collision";
-
-import { getEllipseShape, getPolygonShape } from "@excalidraw/utils/shape";
+import { isPointOnShape } from "@excalidraw/utils/collision";
 
 import type { LocalPoint, Radians } from "@excalidraw/math";
 
@@ -46,6 +44,7 @@ import { intersectElementWithLineSegment } from "./collision";
 import { distanceToBindableElement } from "./distance";
 import {
   headingForPointFromElement,
+  headingIsHorizontal,
   vectorToHeading,
   type Heading,
 } from "./heading";
@@ -64,7 +63,7 @@ import {
   isTextElement,
 } from "./typeChecks";
 
-import { aabbForElement } from "./shapes";
+import { aabbForElement, getElementShape, pointInsideBounds } from "./shapes";
 import { updateElbowArrowPoints } from "./elbowArrow";
 
 import type Scene from "./Scene";
@@ -108,7 +107,7 @@ export const isBindingEnabled = (appState: AppState): boolean => {
 };
 
 export const FIXED_BINDING_DISTANCE = 5;
-const BINDING_HIGHLIGHT_THICKNESS = 10;
+export const BINDING_HIGHLIGHT_THICKNESS = 10;
 export const BINDING_HIGHLIGHT_OFFSET = 4;
 
 const getNonDeletedElements = (
@@ -442,15 +441,19 @@ const normalizePointBinding = (
   binding: { focus: number; gap: number },
   hoveredElement: ExcalidrawBindableElement,
 ) => {
+  let gap = binding.gap;
   const maxGap = maxBindingGap(
     hoveredElement,
     hoveredElement.width,
     hoveredElement.height,
   );
 
+  if (gap > maxGap) {
+    gap = BINDING_HIGHLIGHT_THICKNESS + BINDING_HIGHLIGHT_OFFSET;
+  }
   return {
     ...binding,
-    gap: Math.min(binding.gap, maxGap),
+    gap,
   };
 };
 
@@ -562,8 +565,7 @@ export const getHoveredElementForBinding = (
     let cullRest = false;
     const candidateElements = getAllElementsAtPositionForBinding(
       elements,
-      (element) => {
-        const result =
+      (element) =>
         isBindableElement(element, false) &&
         bindingBorderTest(
           element,
@@ -577,10 +579,7 @@ export const getHoveredElementForBinding = (
             // disable fullshape snapping for frame elements so we
             // can bind to frame children
             !isFrameLikeElement(element),
-          );
-
-        return result;
-      },
+        ),
     ).filter((element) => {
       if (cullRest) {
         return false;
@@ -888,12 +887,7 @@ export const getHeadingForElbowArrowSnap = (
     return otherPointHeading;
   }
 
-  const distance = getDistanceForBinding(
-    origPoint,
-    bindableElement,
-    true,
-    zoom,
-  );
+  const distance = getDistanceForBinding(origPoint, bindableElement, zoom);
 
   if (!distance) {
     return vectorToHeading(
@@ -904,10 +898,9 @@ export const getHeadingForElbowArrowSnap = (
   return headingForPointFromElement(bindableElement, aabb, p);
 };
 
-export const getDistanceForBinding = (
+const getDistanceForBinding = (
   point: Readonly<GlobalPoint>,
   bindableElement: ExcalidrawBindableElement,
-  fullShape: boolean,
   zoom?: AppState["zoom"],
 ) => {
   const distance = distanceToBindableElement(bindableElement, point);
@@ -917,16 +910,8 @@ export const getDistanceForBinding = (
     bindableElement.height,
     zoom,
   );
-  const isInside = fullShape
-    ? isPointInShape(
-        point,
-        bindableElement.type === "ellipse"
-          ? getEllipseShape(bindableElement)
-          : getPolygonShape(bindableElement),
-      )
-    : false;
 
-  return distance > bindDistance && !isInside ? null : distance;
+  return distance > bindDistance ? null : distance;
 };
 
 export const bindPointToSnapToElementOutline = (
@@ -962,16 +947,23 @@ export const bindPointToSnapToElementOutline = (
 
   let intersection: GlobalPoint | null = null;
   if (elbowed) {
+    const isHorizontal = headingIsHorizontal(
+      headingForPointFromElement(bindableElement, aabb, globalP),
+    );
+    const otherPoint = pointFrom<GlobalPoint>(
+      isHorizontal ? center[0] : edgePoint[0],
+      !isHorizontal ? center[1] : edgePoint[1],
+    );
     intersection = intersectElementWithLineSegment(
       bindableElement,
       lineSegment(
-        center,
+        otherPoint,
         pointFromVector(
           vectorScale(
-            vectorNormalize(vectorFromPoint(edgePoint, center)),
+            vectorNormalize(vectorFromPoint(edgePoint, otherPoint)),
             Math.max(bindableElement.width, bindableElement.height) * 2,
           ),
-          center,
+          otherPoint,
         ),
       ),
     )[0];
@@ -1179,48 +1171,6 @@ export const snapToMid = (
       center,
       angle,
     );
-  } else if (element.type === "diamond") {
-    const distance = FIXED_BINDING_DISTANCE - 1;
-    const topLeft = pointFrom<GlobalPoint>(
-      x + width / 4 - distance,
-      y + height / 4 - distance,
-    );
-    const topRight = pointFrom<GlobalPoint>(
-      x + (3 * width) / 4 + distance,
-      y + height / 4 - distance,
-    );
-    const bottomLeft = pointFrom<GlobalPoint>(
-      x + width / 4 - distance,
-      y + (3 * height) / 4 + distance,
-    );
-    const bottomRight = pointFrom<GlobalPoint>(
-      x + (3 * width) / 4 + distance,
-      y + (3 * height) / 4 + distance,
-    );
-    if (
-      pointDistance(topLeft, nonRotated) <
-      Math.max(horizontalThrehsold, verticalThrehsold)
-    ) {
-      return pointRotateRads(topLeft, center, angle);
-    }
-    if (
-      pointDistance(topRight, nonRotated) <
-      Math.max(horizontalThrehsold, verticalThrehsold)
-    ) {
-      return pointRotateRads(topRight, center, angle);
-    }
-    if (
-      pointDistance(bottomLeft, nonRotated) <
-      Math.max(horizontalThrehsold, verticalThrehsold)
-    ) {
-      return pointRotateRads(bottomLeft, center, angle);
-    }
-    if (
-      pointDistance(bottomRight, nonRotated) <
-      Math.max(horizontalThrehsold, verticalThrehsold)
-    ) {
-      return pointRotateRads(bottomRight, center, angle);
-    }
   }
 
   return p;
@@ -1561,14 +1511,14 @@ export const bindingBorderTest = (
   zoom?: AppState["zoom"],
   fullShape?: boolean,
 ): boolean => {
-  const distance = getDistanceForBinding(
-    pointFrom(x, y),
-    element,
-    !!fullShape,
-    zoom,
-  );
+  const threshold = maxBindingGap(element, element.width, element.height, zoom);
 
-  return !!distance;
+  const shape = getElementShape(element, elementsMap);
+  return (
+    isPointOnShape(pointFrom(x, y), shape, threshold) ||
+    (fullShape === true &&
+      pointInsideBounds(pointFrom(x, y), aabbForElement(element)))
+  );
 };
 
 export const maxBindingGap = (

packages/element/src/elbowArrow.ts

@@ -31,7 +31,6 @@ import {
   getGlobalFixedPointForBindableElement,
   snapToMid,
   getHoveredElementForBinding,
-  getDistanceForBinding,
 } from "./binding";
 import { distanceToBindableElement } from "./distance";
 import {
@@ -1256,7 +1255,6 @@ const getElbowArrowData = (
     origStartGlobalPoint,
     hoveredStartElement,
     options?.isDragging,
-    options?.zoom,
   );
   const endGlobalPoint = getGlobalPoint(
     {
@@ -1270,7 +1268,6 @@ const getElbowArrowData = (
     origEndGlobalPoint,
     hoveredEndElement,
     options?.isDragging,
-    options?.zoom,
   );
   const startHeading = getBindPointHeading(
     startGlobalPoint,
@@ -2214,14 +2211,16 @@ const getGlobalPoint = (
   initialPoint: GlobalPoint,
   element?: ExcalidrawBindableElement | null,
   isDragging?: boolean,
-  zoom?: AppState["zoom"],
 ): GlobalPoint => {
   if (isDragging) {
-    if (element && getDistanceForBinding(initialPoint, element, true, zoom)) {
-      return snapToMid(
+    if (element) {
+      const snapPoint = bindPointToSnapToElementOutline(
+        arrow,
         element,
-        bindPointToSnapToElementOutline(arrow, element, startOrEnd),
+        startOrEnd,
       );
+
+      return snapToMid(element, snapPoint);
     }
 
     return initialPoint;

packages/element/src/sizeHelpers.ts

@@ -3,12 +3,10 @@ import {
   viewportCoordsToSceneCoords,
 } from "@excalidraw/common";
 
-import { pointsEqual } from "@excalidraw/math";
-
 import type { AppState, Offsets, Zoom } from "@excalidraw/excalidraw/types";
 
 import { getCommonBounds, getElementBounds } from "./bounds";
-import { isElbowArrow, isFreeDrawElement, isLinearElement } from "./typeChecks";
+import { isFreeDrawElement, isLinearElement } from "./typeChecks";
 
 import type { ElementsMap, ExcalidrawElement } from "./types";
 
@@ -18,12 +16,6 @@ import type { ElementsMap, ExcalidrawElement } from "./types";
 export const isInvisiblySmallElement = (
   element: ExcalidrawElement,
 ): boolean => {
-  if (isElbowArrow(element)) {
-    return (
-      element.points.length < 2 ||
-      pointsEqual(element.points[0], element.points[element.points.length - 1])
-    );
-  }
   if (isLinearElement(element) || isFreeDrawElement(element)) {
     return element.points.length < 2;
   }

packages/element/tests/elbowArrow.test.tsx

@@ -199,7 +199,7 @@ describe("elbow arrow routing", () => {
       points: [pointFrom<LocalPoint>(0, 0), pointFrom<LocalPoint>(90, 200)],
     });
 
-    expect(arrow.points).toCloselyEqualPoints([
+    expect(arrow.points).toEqual([
       [0, 0],
       [45, 0],
       [45, 200],
@@ -253,7 +253,7 @@ describe("elbow arrow ui", () => {
 
     expect(arrow.type).toBe("arrow");
     expect(arrow.elbowed).toBe(true);
-    expect(arrow.points).toCloselyEqualPoints([
+    expect(arrow.points).toEqual([
       [0, 0],
       [45, 0],
       [45, 200],
@@ -351,7 +351,7 @@ describe("elbow arrow ui", () => {
     expect(duplicatedArrow.id).not.toBe(originalArrowId);
     expect(duplicatedArrow.type).toBe("arrow");
     expect(duplicatedArrow.elbowed).toBe(true);
-    expect(duplicatedArrow.points).toCloselyEqualPoints([
+    expect(duplicatedArrow.points).toEqual([
       [0, 0],
       [45, 0],
       [45, 200],
@@ -405,98 +405,11 @@ describe("elbow arrow ui", () => {
     expect(duplicatedArrow.id).not.toBe(originalArrowId);
     expect(duplicatedArrow.type).toBe("arrow");
     expect(duplicatedArrow.elbowed).toBe(true);
-    expect(duplicatedArrow.points).toCloselyEqualPoints([
+    expect(duplicatedArrow.points).toEqual([
       [0, 0],
       [0, 100],
       [90, 100],
       [90, 200],
     ]);
   });
-
-  it("elbow arrow snap at diamond quarter point too", async () => {
-    UI.createElement("diamond", {
-      x: -50,
-      y: -50,
-      width: 100,
-      height: 100,
-    });
-
-    UI.clickTool("arrow");
-    UI.clickOnTestId("elbow-arrow");
-
-    mouse.reset();
-    mouse.moveTo(43, 99);
-    mouse.click();
-    mouse.moveTo(27, 25);
-    mouse.click();
-
-    let arrow = h.scene.getSelectedElements(
-      h.state,
-    )[0] as ExcalidrawArrowElement;
-
-    expect(arrow.endBinding).not.toBe(null);
-    expect(arrow.x + arrow.points[arrow.points.length - 1][0]).toBeCloseTo(
-      29.0355,
-    );
-    expect(arrow.y + arrow.points[arrow.points.length - 1][1]).toBeCloseTo(
-      29.0355,
-    );
-
-    UI.clickTool("arrow");
-    UI.clickOnTestId("elbow-arrow");
-
-    mouse.reset();
-    mouse.moveTo(43, 99);
-    mouse.click();
-    mouse.moveTo(-23, 25);
-    mouse.click();
-
-    arrow = h.scene.getSelectedElements(h.state)[0] as ExcalidrawArrowElement;
-
-    expect(arrow.endBinding).not.toBe(null);
-    expect(arrow.x + arrow.points[arrow.points.length - 1][0]).toBeCloseTo(
-      -28.5559,
-    );
-    expect(arrow.y + arrow.points[arrow.points.length - 1][1]).toBeCloseTo(
-      28.5559,
-    );
-
-    UI.clickTool("arrow");
-    UI.clickOnTestId("elbow-arrow");
-
-    mouse.reset();
-    mouse.moveTo(43, 99);
-    mouse.click();
-    mouse.moveTo(-27, -25);
-    mouse.click();
-
-    arrow = h.scene.getSelectedElements(h.state)[0] as ExcalidrawArrowElement;
-
-    expect(arrow.endBinding).not.toBe(null);
-    expect(arrow.x + arrow.points[arrow.points.length - 1][0]).toBeCloseTo(
-      -28.0355,
-    );
-    expect(arrow.y + arrow.points[arrow.points.length - 1][1]).toBeCloseTo(
-      -28.0355,
-    );
-
-    UI.clickTool("arrow");
-    UI.clickOnTestId("elbow-arrow");
-
-    mouse.reset();
-    mouse.moveTo(43, 99);
-    mouse.click();
-    mouse.moveTo(23, -25);
-    mouse.click();
-
-    arrow = h.scene.getSelectedElements(h.state)[0] as ExcalidrawArrowElement;
-
-    expect(arrow.endBinding).not.toBe(null);
-    expect(arrow.x + arrow.points[arrow.points.length - 1][0]).toBeCloseTo(
-      28.5559,
-    );
-    expect(arrow.y + arrow.points[arrow.points.length - 1][1]).toBeCloseTo(
-      -28.5559,
-    );
-  });
 });

packages/excalidraw/actions/actionFinalize.tsx

@@ -7,7 +7,6 @@ import {
 import { LinearElementEditor } from "@excalidraw/element/linearElementEditor";
 
 import {
-  isArrowElement,
   isBindingElement,
   isLinearElement,
 } from "@excalidraw/element/typeChecks";
@@ -17,12 +16,6 @@ import { isPathALoop } from "@excalidraw/element/shapes";
 
 import { isInvisiblySmallElement } from "@excalidraw/element/sizeHelpers";
 
-import type {
-  ExcalidrawElement,
-  ExcalidrawLinearElement,
-  NonDeleted,
-} from "@excalidraw/element/types";
-
 import { t } from "../i18n";
 import { resetCursor } from "../cursor";
 import { done } from "../components/icons";
@@ -69,7 +62,6 @@ export const actionFinalize = register({
           captureUpdate: CaptureUpdateAction.IMMEDIATELY,
         };
       }
-    } else if (isArrowElement(appState.newElement)) {
     }
 
     let newElements = elements;
@@ -90,55 +82,48 @@ export const actionFinalize = register({
       focusContainer();
     }
 
-    let element: NonDeleted<ExcalidrawElement> | null = null;
-    if (appState.multiElement) {
-      element = appState.multiElement;
-    } else if (
-      appState.newElement?.type === "freedraw" ||
-      isBindingElement(appState.newElement)
-    ) {
-      element = appState.newElement;
-    } else if (Object.keys(appState.selectedElementIds).length === 1) {
-      const candidate = elementsMap.get(
-        Object.keys(appState.selectedElementIds)[0],
-      ) as NonDeleted<ExcalidrawLinearElement> | undefined;
-      if (candidate) {
-        element = candidate;
-      }
-    }
+    const multiPointElement = appState.multiElement
+      ? appState.multiElement
+      : appState.newElement?.type === "freedraw"
+      ? appState.newElement
+      : null;
 
-    if (element) {
+    if (multiPointElement) {
       // pen and mouse have hover
       if (
-        appState.multiElement &&
-        element.type !== "freedraw" &&
+        multiPointElement.type !== "freedraw" &&
         appState.lastPointerDownWith !== "touch"
       ) {
-        const { points, lastCommittedPoint } = element;
+        const { points, lastCommittedPoint } = multiPointElement;
         if (
           !lastCommittedPoint ||
           points[points.length - 1] !== lastCommittedPoint
         ) {
-          scene.mutateElement(element, {
-            points: element.points.slice(0, -1),
+          scene.mutateElement(multiPointElement, {
+            points: multiPointElement.points.slice(0, -1),
           });
         }
       }
 
-      if (isInvisiblySmallElement(element)) {
+      if (isInvisiblySmallElement(multiPointElement)) {
         // TODO: #7348 in theory this gets recorded by the store, so the invisible elements could be restored by the undo/redo, which might be not what we would want
-        newElements = newElements.filter((el) => el.id !== element!.id);
+        newElements = newElements.filter(
+          (el) => el.id !== multiPointElement.id,
+        );
       }
 
       // If the multi point line closes the loop,
       // set the last point to first point.
       // This ensures that loop remains closed at different scales.
-      const isLoop = isPathALoop(element.points, appState.zoom.value);
-      if (element.type === "line" || element.type === "freedraw") {
+      const isLoop = isPathALoop(multiPointElement.points, appState.zoom.value);
+      if (
+        multiPointElement.type === "line" ||
+        multiPointElement.type === "freedraw"
+      ) {
         if (isLoop) {
-          const linePoints = element.points;
+          const linePoints = multiPointElement.points;
           const firstPoint = linePoints[0];
-          scene.mutateElement(element, {
+          scene.mutateElement(multiPointElement, {
             points: linePoints.map((p, index) =>
               index === linePoints.length - 1
                 ? pointFrom(firstPoint[0], firstPoint[1])
@@ -149,24 +134,23 @@ export const actionFinalize = register({
       }
 
       if (
-        isBindingElement(element) &&
+        isBindingElement(multiPointElement) &&
         !isLoop &&
-        element.points.length > 1 &&
-        !appState.selectedElementIds[element.id]
+        multiPointElement.points.length > 1
       ) {
         const [x, y] = LinearElementEditor.getPointAtIndexGlobalCoordinates(
-          element,
+          multiPointElement,
           -1,
           arrayToMap(elements),
         );
-        maybeBindLinearElement(element, appState, { x, y }, scene);
+        maybeBindLinearElement(multiPointElement, appState, { x, y }, scene);
       }
     }
 
     if (
       (!appState.activeTool.locked &&
         appState.activeTool.type !== "freedraw") ||
-      !element
+      !multiPointElement
     ) {
       resetCursor(interactiveCanvas);
     }
@@ -193,7 +177,7 @@ export const actionFinalize = register({
         activeTool:
           (appState.activeTool.locked ||
             appState.activeTool.type === "freedraw") &&
-          element
+          multiPointElement
             ? appState.activeTool
             : activeTool,
         activeEmbeddable: null,
@@ -204,18 +188,21 @@ export const actionFinalize = register({
         startBoundElement: null,
         suggestedBindings: [],
         selectedElementIds:
-          element &&
+          multiPointElement &&
           !appState.activeTool.locked &&
           appState.activeTool.type !== "freedraw"
             ? {
                 ...appState.selectedElementIds,
-                [element.id]: true,
+                [multiPointElement.id]: true,
               }
             : appState.selectedElementIds,
         // To select the linear element when user has finished mutipoint editing
         selectedLinearElement:
-          element && isLinearElement(element)
-            ? new LinearElementEditor(element, arrayToMap(newElements))
+          multiPointElement && isLinearElement(multiPointElement)
+            ? new LinearElementEditor(
+                multiPointElement,
+                arrayToMap(newElements),
+              )
             : appState.selectedLinearElement,
         pendingImageElementId: null,
       },

packages/excalidraw/components/App.tsx

@@ -9022,7 +9022,6 @@ class App extends React.Component<AppProps, AppState> {
             linearElementEditor;
           const element = this.scene.getElement(linearElementEditor.elementId);
           if (isBindingElement(element)) {
-            this.actionManager.executeAction(actionFinalize);
             bindOrUnbindLinearElement(
               element,
               startBindingElement,

packages/excalidraw/renderer/interactiveScene.ts

@@ -17,6 +17,7 @@ import {
 
 import {
   BINDING_HIGHLIGHT_OFFSET,
+  BINDING_HIGHLIGHT_THICKNESS,
   maxBindingGap,
 } from "@excalidraw/element/binding";
 import { LinearElementEditor } from "@excalidraw/element/linearElementEditor";
@@ -260,9 +261,9 @@ const renderBindingHighlightForBindableElement = (
   const height = y2 - y1;
 
   context.strokeStyle = "rgba(0,0,0,.05)";
-  context.lineWidth =
-    maxBindingGap(element, element.width, element.height, zoom) -
-    BINDING_HIGHLIGHT_OFFSET;
+  // When zooming out, make line width greater for visibility
+  const zoomValue = zoom.value < 1 ? zoom.value : 1;
+  context.lineWidth = BINDING_HIGHLIGHT_THICKNESS / zoomValue;
   // To ensure the binding highlight doesn't overlap the element itself
   const padding = context.lineWidth / 2 + BINDING_HIGHLIGHT_OFFSET;