feat: adjust line-confirm-threshold based on zoom (#2884)

Co-authored-by: Lipis <lipiridis@gmail.com>

src/actions/actionFinalize.tsx

@@ -83,7 +83,7 @@ export const actionFinalize = register({
       // 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(multiPointElement.points);
+      const isLoop = isPathALoop(multiPointElement.points, appState.zoom.value);
       if (
         multiPointElement.type === "line" ||
         multiPointElement.type === "draw"

src/components/App.tsx

@@ -1963,7 +1963,7 @@ class App extends React.Component<ExcalidrawProps, AppState> {
           points: points.slice(0, -1),
         });
       } else {
-        if (isPathALoop(points)) {
+        if (isPathALoop(points, this.state.zoom.value)) {
           document.documentElement.style.cursor = CURSOR_TYPE.POINTER;
         }
         // update last uncommitted point
@@ -2635,7 +2635,10 @@ class App extends React.Component<ExcalidrawProps, AppState> {
       const { multiElement } = this.state;
 
       // finalize if completing a loop
-      if (multiElement.type === "line" && isPathALoop(multiElement.points)) {
+      if (
+        multiElement.type === "line" &&
+        isPathALoop(multiElement.points, this.state.zoom.value)
+      ) {
         mutateElement(multiElement, {
           lastCommittedPoint:
             multiElement.points[multiElement.points.length - 1],

src/constants.ts

@@ -2,8 +2,8 @@ import { FontFamily } from "./element/types";
 
 export const APP_NAME = "Excalidraw";
 
-export const DRAGGING_THRESHOLD = 10; // 10px
+export const DRAGGING_THRESHOLD = 10; // px
-export const LINE_CONFIRM_THRESHOLD = 10; // 10px
+export const LINE_CONFIRM_THRESHOLD = 8; // px
 export const ELEMENT_SHIFT_TRANSLATE_AMOUNT = 5;
 export const ELEMENT_TRANSLATE_AMOUNT = 1;
 export const TEXT_TO_CENTER_SNAP_THRESHOLD = 30;

src/element/linearElementEditor.ts

@@ -129,7 +129,7 @@ export class LinearElementEditor {
       isDragging &&
       (activePointIndex === 0 || activePointIndex === element.points.length - 1)
     ) {
-      if (isPathALoop(element.points)) {
+      if (isPathALoop(element.points, appState.zoom.value)) {
         LinearElementEditor.movePoint(
           element,
           activePointIndex,

src/math.ts

@@ -1,4 +1,4 @@
-import { Point } from "./types";
+import { NormalizedZoomValue, Point, Zoom } from "./types";
 import { LINE_CONFIRM_THRESHOLD } from "./constants";
 import { ExcalidrawLinearElement } from "./element/types";
 
@@ -147,13 +147,16 @@ export const centerPoint = (a: Point, b: Point): Point => {
 // to be considered a loop
 export const isPathALoop = (
   points: ExcalidrawLinearElement["points"],
+  /** supply if you want the loop detection to account for current zoom */
+  zoomValue: Zoom["value"] = 1 as NormalizedZoomValue,
 ): boolean => {
   if (points.length >= 3) {
-    const [firstPoint, lastPoint] = [points[0], points[points.length - 1]];
+    const [first, last] = [points[0], points[points.length - 1]];
-    return (
+    const distance = distance2d(first[0], first[1], last[0], last[1]);
-      distance2d(firstPoint[0], firstPoint[1], lastPoint[0], lastPoint[1]) <=
+
-      LINE_CONFIRM_THRESHOLD
+    // Adjusting LINE_CONFIRM_THRESHOLD to current zoom so that when zoomed in
-    );
+    // really close we make the threshold smaller, and vice versa.
+    return distance <= LINE_CONFIRM_THRESHOLD / zoomValue;
   }
   return false;
 };