feat: Don't add midpoint until dragged beyond a threshold (#5927)

* Don't add midpoint until dragged beyond a threshold

* remove unnecessary code

* fix tests

* fix

* add spec

* remove isMidpoint

* cleanup

* fix threshold for zoom

* split into shouldAddMidpoint and addMidpoint

* wrap in flushSync for synchronous updates

* remove threshold for line editor and add spec

* [unrelated] fix stack overflow state update

* fix tests

* don't drag arrow when dragging to add mid point

* add specs

Co-authored-by: dwelle <luzar.david@gmail.com>

src/components/App.tsx

@@ -2885,7 +2885,10 @@ class App extends React.Component<AppProps, AppState> {
       if (editingLinearElement?.lastUncommittedPoint != null) {
         this.maybeSuggestBindingAtCursor(scenePointer);
       } else {
+        // causes stack overflow if not sync
+        flushSync(() => {
           this.setState({ suggestedBindings: [] });
+        });
       }
     }
 
@@ -3825,7 +3828,7 @@ class App extends React.Component<AppProps, AppState> {
               this.setState({ editingLinearElement: ret.linearElementEditor });
             }
           }
-          if (ret.didAddPoint && !ret.isMidPoint) {
+          if (ret.didAddPoint) {
             return true;
           }
         }
@@ -4315,7 +4318,6 @@ class App extends React.Component<AppProps, AppState> {
       // to ensure we don't create a 2-point arrow by mistake when
       // user clicks mouse in a way that it moves a tiny bit (thus
       // triggering pointermove)
-
       if (
         !pointerDownState.drag.hasOccurred &&
         (this.state.activeTool.type === "arrow" ||
@@ -4343,6 +4345,56 @@ class App extends React.Component<AppProps, AppState> {
       if (this.state.selectedLinearElement) {
         const linearElementEditor =
           this.state.editingLinearElement || this.state.selectedLinearElement;
+
+        if (
+          LinearElementEditor.shouldAddMidpoint(
+            this.state.selectedLinearElement,
+            pointerCoords,
+            this.state,
+          )
+        ) {
+          const ret = LinearElementEditor.addMidpoint(
+            this.state.selectedLinearElement,
+            pointerCoords,
+            this.state,
+          );
+          if (!ret) {
+            return;
+          }
+
+          // Since we are reading from previous state which is not possible with
+          // automatic batching in React 18 hence using flush sync to synchronously
+          // update the state. Check https://github.com/excalidraw/excalidraw/pull/5508 for more details.
+
+          flushSync(() => {
+            if (this.state.selectedLinearElement) {
+              this.setState({
+                selectedLinearElement: {
+                  ...this.state.selectedLinearElement,
+                  pointerDownState: ret.pointerDownState,
+                  selectedPointsIndices: ret.selectedPointsIndices,
+                },
+              });
+            }
+            if (this.state.editingLinearElement) {
+              this.setState({
+                editingLinearElement: {
+                  ...this.state.editingLinearElement,
+                  pointerDownState: ret.pointerDownState,
+                  selectedPointsIndices: ret.selectedPointsIndices,
+                },
+              });
+            }
+          });
+
+          return;
+        } else if (
+          linearElementEditor.pointerDownState.segmentMidpoint.value !== null &&
+          !linearElementEditor.pointerDownState.segmentMidpoint.added
+        ) {
+          return;
+        }
+
         const didDrag = LinearElementEditor.handlePointDragging(
           event,
           this.state,

src/element/linearElementEditor.ts

@@ -20,7 +20,7 @@ import {
 } from "../math";
 import { getElementAbsoluteCoords, getLockedLinearCursorAlignSize } from ".";
 import { getElementPointsCoords } from "./bounds";
-import { Point, AppState } from "../types";
+import { Point, AppState, PointerCoords } from "../types";
 import { mutateElement } from "./mutateElement";
 import History from "../history";
 
@@ -33,6 +33,7 @@ import {
 import { tupleToCoors } from "../utils";
 import { isBindingElement } from "./typeChecks";
 import { shouldRotateWithDiscreteAngle } from "../keys";
+import { DRAGGING_THRESHOLD } from "../constants";
 
 const editorMidPointsCache: {
   version: number | null;
@@ -51,6 +52,12 @@ export class LinearElementEditor {
     prevSelectedPointsIndices: readonly number[] | null;
     /** index */
     lastClickedPoint: number;
+    origin: Readonly<{ x: number; y: number }> | null;
+    segmentMidpoint: {
+      value: Point | null;
+      index: number | null;
+      added: boolean;
+    };
   }>;
 
   /** whether you're dragging a point */
@@ -81,6 +88,13 @@ export class LinearElementEditor {
     this.pointerDownState = {
       prevSelectedPointsIndices: null,
       lastClickedPoint: -1,
+      origin: null,
+
+      segmentMidpoint: {
+        value: null,
+        index: null,
+        added: false,
+      },
     };
     this.hoverPointIndex = -1;
     this.segmentMidPointHoveredCoords = null;
@@ -180,6 +194,7 @@ export class LinearElementEditor {
     const draggingPoint = element.points[
       linearElementEditor.pointerDownState.lastClickedPoint
     ] as [number, number] | undefined;
+
     if (selectedPointsIndices && draggingPoint) {
       if (
         shouldRotateWithDiscreteAngle(event) &&
@@ -551,7 +566,7 @@ export class LinearElementEditor {
     }
     const midPoints = LinearElementEditor.getEditorMidPoints(element, appState);
     let index = 0;
-    while (index < midPoints.length - 1) {
+    while (index < midPoints.length) {
       if (LinearElementEditor.arePointsEqual(midPoint, midPoints[index])) {
         return index + 1;
       }
@@ -570,13 +585,11 @@ export class LinearElementEditor {
     didAddPoint: boolean;
     hitElement: NonDeleted<ExcalidrawElement> | null;
     linearElementEditor: LinearElementEditor | null;
-    isMidPoint: boolean;
   } {
     const ret: ReturnType<typeof LinearElementEditor["handlePointerDown"]> = {
       didAddPoint: false,
       hitElement: null,
       linearElementEditor: null,
-      isMidPoint: false,
     };
 
     if (!linearElementEditor) {
@@ -589,43 +602,18 @@ export class LinearElementEditor {
     if (!element) {
       return ret;
     }
-    const segmentMidPoint = LinearElementEditor.getSegmentMidpointHitCoords(
+    const segmentMidpoint = LinearElementEditor.getSegmentMidpointHitCoords(
       linearElementEditor,
       scenePointer,
       appState,
     );
-    if (segmentMidPoint) {
+    let segmentMidpointIndex = null;
-      const index = LinearElementEditor.getSegmentMidPointIndex(
+    if (segmentMidpoint) {
+      segmentMidpointIndex = LinearElementEditor.getSegmentMidPointIndex(
         linearElementEditor,
         appState,
-        segmentMidPoint,
+        segmentMidpoint,
       );
-      const newMidPoint = LinearElementEditor.createPointAt(
-        element,
-        segmentMidPoint[0],
-        segmentMidPoint[1],
-        appState.gridSize,
-      );
-      const points = [
-        ...element.points.slice(0, index),
-        newMidPoint,
-        ...element.points.slice(index),
-      ];
-      mutateElement(element, {
-        points,
-      });
-
-      ret.didAddPoint = true;
-      ret.isMidPoint = true;
-      ret.linearElementEditor = {
-        ...linearElementEditor,
-        selectedPointsIndices: element.points[1],
-        pointerDownState: {
-          prevSelectedPointsIndices: linearElementEditor.selectedPointsIndices,
-          lastClickedPoint: -1,
-        },
-        lastUncommittedPoint: null,
-      };
     }
     if (event.altKey && appState.editingLinearElement) {
       if (linearElementEditor.lastUncommittedPoint == null) {
@@ -648,6 +636,12 @@ export class LinearElementEditor {
         pointerDownState: {
           prevSelectedPointsIndices: linearElementEditor.selectedPointsIndices,
           lastClickedPoint: -1,
+          origin: { x: scenePointer.x, y: scenePointer.y },
+          segmentMidpoint: {
+            value: segmentMidpoint,
+            index: segmentMidpointIndex,
+            added: false,
+          },
         },
         selectedPointsIndices: [element.points.length - 1],
         lastUncommittedPoint: null,
@@ -670,7 +664,7 @@ export class LinearElementEditor {
 
     // if we clicked on a point, set the element as hitElement otherwise
     // it would get deselected if the point is outside the hitbox area
-    if (clickedPointIndex >= 0 || segmentMidPoint) {
+    if (clickedPointIndex >= 0 || segmentMidpoint) {
       ret.hitElement = element;
     } else {
       // You might be wandering why we are storing the binding elements on
@@ -716,6 +710,12 @@ export class LinearElementEditor {
       pointerDownState: {
         prevSelectedPointsIndices: linearElementEditor.selectedPointsIndices,
         lastClickedPoint: clickedPointIndex,
+        origin: { x: scenePointer.x, y: scenePointer.y },
+        segmentMidpoint: {
+          value: segmentMidpoint,
+          index: segmentMidpointIndex,
+          added: false,
+        },
       },
       selectedPointsIndices: nextSelectedPointsIndices,
       pointerOffset: targetPoint
@@ -1111,6 +1111,94 @@ export class LinearElementEditor {
     );
   }
 
+  static shouldAddMidpoint(
+    linearElementEditor: LinearElementEditor,
+    pointerCoords: PointerCoords,
+    appState: AppState,
+  ) {
+    const element = LinearElementEditor.getElement(
+      linearElementEditor.elementId,
+    );
+
+    if (!element) {
+      return false;
+    }
+
+    const { segmentMidpoint } = linearElementEditor.pointerDownState;
+
+    if (
+      segmentMidpoint.added ||
+      segmentMidpoint.value === null ||
+      segmentMidpoint.index === null ||
+      linearElementEditor.pointerDownState.origin === null
+    ) {
+      return false;
+    }
+
+    const origin = linearElementEditor.pointerDownState.origin!;
+    const dist = distance2d(
+      origin.x,
+      origin.y,
+      pointerCoords.x,
+      pointerCoords.y,
+    );
+    if (
+      !appState.editingLinearElement &&
+      dist < DRAGGING_THRESHOLD / appState.zoom.value
+    ) {
+      return false;
+    }
+    return true;
+  }
+
+  static addMidpoint(
+    linearElementEditor: LinearElementEditor,
+    pointerCoords: PointerCoords,
+    appState: AppState,
+  ) {
+    const element = LinearElementEditor.getElement(
+      linearElementEditor.elementId,
+    );
+    if (!element) {
+      return;
+    }
+    const { segmentMidpoint } = linearElementEditor.pointerDownState;
+    const ret: {
+      pointerDownState: LinearElementEditor["pointerDownState"];
+      selectedPointsIndices: LinearElementEditor["selectedPointsIndices"];
+    } = {
+      pointerDownState: linearElementEditor.pointerDownState,
+      selectedPointsIndices: linearElementEditor.selectedPointsIndices,
+    };
+
+    const midpoint = LinearElementEditor.createPointAt(
+      element,
+      pointerCoords.x,
+      pointerCoords.y,
+      appState.gridSize,
+    );
+    const points = [
+      ...element.points.slice(0, segmentMidpoint.index!),
+      midpoint,
+      ...element.points.slice(segmentMidpoint.index!),
+    ];
+
+    mutateElement(element, {
+      points,
+    });
+
+    ret.pointerDownState = {
+      ...linearElementEditor.pointerDownState,
+      segmentMidpoint: {
+        ...linearElementEditor.pointerDownState.segmentMidpoint,
+        added: true,
+      },
+      lastClickedPoint: segmentMidpoint.index!,
+    };
+    ret.selectedPointsIndices = [segmentMidpoint.index!];
+    return ret;
+  }
+
   private static _updatePoints(
     element: NonDeleted<ExcalidrawLinearElement>,
     nextPoints: readonly Point[],

src/tests/__snapshots__/regressionTests.test.tsx.snap

@@ -10989,7 +10989,13 @@ Object {
     "lastUncommittedPoint": null,
     "pointerDownState": Object {
       "lastClickedPoint": -1,
+      "origin": null,
       "prevSelectedPointsIndices": null,
+      "segmentMidpoint": Object {
+        "added": false,
+        "index": null,
+        "value": null,
+      },
     },
     "pointerOffset": Object {
       "x": 0,
@@ -11216,7 +11222,13 @@ Object {
     "lastUncommittedPoint": null,
     "pointerDownState": Object {
       "lastClickedPoint": -1,
+      "origin": null,
       "prevSelectedPointsIndices": null,
+      "segmentMidpoint": Object {
+        "added": false,
+        "index": null,
+        "value": null,
+      },
     },
     "pointerOffset": Object {
       "x": 0,
@@ -11671,7 +11683,13 @@ Object {
     "lastUncommittedPoint": null,
     "pointerDownState": Object {
       "lastClickedPoint": -1,
+      "origin": null,
       "prevSelectedPointsIndices": null,
+      "segmentMidpoint": Object {
+        "added": false,
+        "index": null,
+        "value": null,
+      },
     },
     "pointerOffset": Object {
       "x": 0,
@@ -12078,7 +12096,13 @@ Object {
     "lastUncommittedPoint": null,
     "pointerDownState": Object {
       "lastClickedPoint": -1,
+      "origin": null,
       "prevSelectedPointsIndices": null,
+      "segmentMidpoint": Object {
+        "added": false,
+        "index": null,
+        "value": null,
+      },
     },
     "pointerOffset": Object {
       "x": 0,

src/tests/__snapshots__/selection.test.tsx.snap

@@ -21,10 +21,6 @@ Object {
       0,
       0,
     ],
-    Array [
-      15,
-      25,
-    ],
     Array [
       30,
       50,
@@ -40,8 +36,8 @@ Object {
   "strokeWidth": 1,
   "type": "arrow",
   "updated": 1,
-  "version": 4,
+  "version": 3,
-  "versionNonce": 453191,
+  "versionNonce": 449462985,
   "width": 30,
   "x": 10,
   "y": 10,
@@ -69,10 +65,6 @@ Object {
       0,
       0,
     ],
-    Array [
-      15,
-      25,
-    ],
     Array [
       30,
       50,
@@ -88,8 +80,8 @@ Object {
   "strokeWidth": 1,
   "type": "line",
   "updated": 1,
-  "version": 4,
+  "version": 3,
-  "versionNonce": 453191,
+  "versionNonce": 449462985,
   "width": 30,
   "x": 10,
   "y": 10,

src/tests/linearElementEditor.test.tsx

@@ -129,6 +129,28 @@ describe("Test Linear Elements", () => {
     Keyboard.keyPress(KEYS.DELETE);
   };
 
+  it("should not drag line and add midpoint until dragged beyond a threshold", () => {
+    createTwoPointerLinearElement("line");
+    const line = h.elements[0] as ExcalidrawLinearElement;
+    const originalX = line.x;
+    const originalY = line.y;
+    expect(line.points.length).toEqual(2);
+
+    mouse.clickAt(midpoint[0], midpoint[1]);
+    drag(midpoint, [midpoint[0] + 1, midpoint[1] + 1]);
+
+    expect(line.points.length).toEqual(2);
+
+    expect(line.x).toBe(originalX);
+    expect(line.y).toBe(originalY);
+    expect(line.points.length).toEqual(2);
+
+    drag(midpoint, [midpoint[0] + delta, midpoint[1] + delta]);
+    expect(line.x).toBe(originalX);
+    expect(line.y).toBe(originalY);
+    expect(line.points.length).toEqual(3);
+  });
+
   it("should allow dragging line from midpoint in 2 pointer lines outside editor", async () => {
     createTwoPointerLinearElement("line");
     const line = h.elements[0] as ExcalidrawLinearElement;
@@ -138,7 +160,7 @@ describe("Test Linear Elements", () => {
 
     // drag line from midpoint
     drag(midpoint, [midpoint[0] + delta, midpoint[1] + delta]);
-    expect(renderScene).toHaveBeenCalledTimes(10);
+    expect(renderScene).toHaveBeenCalledTimes(11);
     expect(line.points.length).toEqual(3);
     expect(line.points).toMatchInlineSnapshot(`
       Array [
@@ -195,6 +217,24 @@ describe("Test Linear Elements", () => {
   });
 
   describe("Inside editor", () => {
+    it("should not drag line and add midpoint when dragged irrespective of threshold", () => {
+      createTwoPointerLinearElement("line");
+      const line = h.elements[0] as ExcalidrawLinearElement;
+      const originalX = line.x;
+      const originalY = line.y;
+      enterLineEditingMode(line);
+
+      expect(line.points.length).toEqual(2);
+
+      mouse.clickAt(midpoint[0], midpoint[1]);
+      expect(line.points.length).toEqual(2);
+
+      drag(midpoint, [midpoint[0] + 1, midpoint[1] + 1]);
+      expect(line.x).toBe(originalX);
+      expect(line.y).toBe(originalY);
+      expect(line.points.length).toEqual(3);
+    });
+
     it("should allow dragging line from midpoint in 2 pointer lines", async () => {
       createTwoPointerLinearElement("line");
 
@@ -203,7 +243,7 @@ describe("Test Linear Elements", () => {
 
       // drag line from midpoint
       drag(midpoint, [midpoint[0] + delta, midpoint[1] + delta]);
-      expect(renderScene).toHaveBeenCalledTimes(14);
+      expect(renderScene).toHaveBeenCalledTimes(15);
 
       expect(line.points.length).toEqual(3);
       expect(line.points).toMatchInlineSnapshot(`
@@ -282,7 +322,7 @@ describe("Test Linear Elements", () => {
       // Move the element
       drag(startPoint, endPoint);
 
-      expect(renderScene).toHaveBeenCalledTimes(15);
+      expect(renderScene).toHaveBeenCalledTimes(16);
       expect([line.x, line.y]).toEqual([
         points[0][0] + deltaX,
         points[0][1] + deltaY,
@@ -339,7 +379,7 @@ describe("Test Linear Elements", () => {
           lastSegmentMidpoint[1] + delta,
         ]);
 
-        expect(renderScene).toHaveBeenCalledTimes(19);
+        expect(renderScene).toHaveBeenCalledTimes(21);
         expect(line.points.length).toEqual(5);
 
         expect((h.elements[0] as ExcalidrawLinearElement).points)
@@ -359,7 +399,7 @@ describe("Test Linear Elements", () => {
             ],
             Array [
               105,
-              75,
+              70,
             ],
             Array [
               40,
@@ -378,7 +418,7 @@ describe("Test Linear Elements", () => {
         // Drag from first point
         drag(hitCoords, [hitCoords[0] - delta, hitCoords[1] - delta]);
 
-        expect(renderScene).toHaveBeenCalledTimes(15);
+        expect(renderScene).toHaveBeenCalledTimes(16);
 
         const newPoints = LinearElementEditor.getPointsGlobalCoordinates(line);
         expect([newPoints[0][0], newPoints[0][1]]).toEqual([
@@ -404,7 +444,7 @@ describe("Test Linear Elements", () => {
         // Drag from first point
         drag(hitCoords, [hitCoords[0] + delta, hitCoords[1] + delta]);
 
-        expect(renderScene).toHaveBeenCalledTimes(15);
+        expect(renderScene).toHaveBeenCalledTimes(16);
 
         const newPoints = LinearElementEditor.getPointsGlobalCoordinates(line);
         expect([newPoints[0][0], newPoints[0][1]]).toEqual([
@@ -438,7 +478,7 @@ describe("Test Linear Elements", () => {
         // delete 3rd point
         deletePoint(points[2]);
         expect(line.points.length).toEqual(3);
-        expect(renderScene).toHaveBeenCalledTimes(20);
+        expect(renderScene).toHaveBeenCalledTimes(21);
 
         const newMidPoints = LinearElementEditor.getEditorMidPoints(
           line,
@@ -483,7 +523,7 @@ describe("Test Linear Elements", () => {
           lastSegmentMidpoint[0] + delta,
           lastSegmentMidpoint[1] + delta,
         ]);
-        expect(renderScene).toHaveBeenCalledTimes(19);
+        expect(renderScene).toHaveBeenCalledTimes(21);
 
         expect(line.points.length).toEqual(5);
 
@@ -503,8 +543,8 @@ describe("Test Linear Elements", () => {
               50,
             ],
             Array [
-              104.58050066266131,
+              106.08587175006699,
-              74.24758482724201,
+              73.29416593965323,
             ],
             Array [
               40,
@@ -559,7 +599,7 @@ describe("Test Linear Elements", () => {
         // Drag from first point
         drag(hitCoords, [hitCoords[0] + delta, hitCoords[1] + delta]);
 
-        expect(renderScene).toHaveBeenCalledTimes(15);
+        expect(renderScene).toHaveBeenCalledTimes(16);
 
         const newPoints = LinearElementEditor.getPointsGlobalCoordinates(line);
         expect([newPoints[0][0], newPoints[0][1]]).toEqual([
@@ -627,7 +667,6 @@ describe("Test Linear Elements", () => {
           fillStyle: "solid",
         }),
       ];
-
       const origPoints = line.points.map((point) => [...point]);
       const dragEndPositionOffset = [100, 100] as const;
       API.setSelectedElements([line]);