Fix microjump on drag binding, no keyboard move if bound arrow

Signed-off-by: Mark Tolmacs <[email protected]>

packages/element/src/binding.ts

@@ -6,6 +6,7 @@ import {
   invariant,
   isDevEnv,
   isTestEnv,
+  toLocalPoint,
 } from "@excalidraw/common";
 
 import {
@@ -523,8 +524,19 @@ export const bindLinearElement = (
       ),
     };
   }
+  const points = Array.from(linearElement.points);
+  points[edgePointIndex] = toLocalPoint(
+    bindPointToSnapToElementOutline(
+      linearElement,
+      hoveredElement,
+      startOrEnd,
+      elementsMap,
+    ),
+    linearElement,
+  );
 
   mutateElement(linearElement, {
+    points,
     [startOrEnd === "start" ? "startBinding" : "endBinding"]: binding,
   });
 

packages/element/src/linearElementEditor.ts

@@ -356,16 +356,17 @@ export class LinearElementEditor {
                 elementsMap,
                 true,
               );
+              const newGlobalPointPosition = pointRotateRads(
+                pointFrom<GlobalPoint>(
+                  element.x + newPointPosition[0],
+                  element.y + newPointPosition[1],
+                ),
+                pointFrom<GlobalPoint>(cx, cy),
+                element.angle,
+              );
               const avoidancePoint = getOutlineAvoidingPoint(
                 element,
-                pointRotateRads(
-                  pointFrom<GlobalPoint>(
-                    element.x + newPointPosition[0],
-                    element.y + newPointPosition[1],
-                  ),
-                  pointFrom<GlobalPoint>(cx, cy),
-                  element.angle,
-                ),
+                newGlobalPointPosition,
                 pointIndex,
                 app.scene,
                 app.state.zoom,
@@ -373,8 +374,14 @@ export class LinearElementEditor {
               newPointPosition = LinearElementEditor.createPointAt(
                 element,
                 elementsMap,
-                avoidancePoint[0] - linearElementEditor.pointerOffset.x,
-                avoidancePoint[1] - linearElementEditor.pointerOffset.y,
+                avoidancePoint[0] === newGlobalPointPosition[0]
+                  ? newGlobalPointPosition[0] -
+                      linearElementEditor.pointerOffset.x
+                  : avoidancePoint[0],
+                avoidancePoint[1] === newGlobalPointPosition[1]
+                  ? newGlobalPointPosition[1] -
+                      linearElementEditor.pointerOffset.y
+                  : avoidancePoint[1],
                 null,
               );
             }

packages/element/tests/binding.test.tsx

@@ -173,7 +173,7 @@ describe("element binding", () => {
     },
   );
 
-  it("should unbind arrow when moving it with keyboard", () => {
+  it("should not move bound arrows when moving it with keyboard", () => {
     const rectangle = UI.createElement("rectangle", {
       x: 75,
       y: 0,
@@ -208,16 +208,10 @@ describe("element binding", () => {
         Keyboard.keyPress(KEYS.ARROW_LEFT);
       });
     });
-    expect(arrow.endBinding).toBe(null);
 
-    Keyboard.withModifierKeys({ shift: true }, () => {
-      // We have to move a significant distance to return to the binding
-      Array.from({ length: 10 }).forEach(() => {
-        Keyboard.keyPress(KEYS.ARROW_RIGHT);
-      });
-    });
-    // We are back in the binding zone but we shouldn't rebind
-    expect(arrow.endBinding).toBe(null);
+    expect(arrow.endBinding?.elementId).toBe(rectangle.id);
+    expect(arrow.x).toBe(0);
+    expect(arrow.y).toBe(0);
   });
 
   it("should unbind on bound element deletion", () => {

packages/element/tests/resize.test.tsx

@@ -1052,7 +1052,7 @@ describe("multiple selection", () => {
       0,
     );
     //console.log(JSON.stringify(h.elements));
-    expect(rightBoundArrow.width).toBeCloseTo(100 * scale + 1, 0);
+    expect(rightBoundArrow.width).toBeCloseTo(100 * scale, 0);
     expect(rightBoundArrow.height).toBeCloseTo(0);
     expect(rightBoundArrow.angle).toEqual(0);
     expect(rightBoundArrow.startBinding).toBeNull();

packages/excalidraw/components/App.tsx

@@ -4375,7 +4375,7 @@ class App extends React.Component<AppProps, AppState> {
         const arrowIdsToRemove = new Set<string>();
 
         selectedElements
-          .filter(isElbowArrow)
+          .filter(isArrowElement)
           .filter((arrow) => {
             const startElementNotInSelection =
               arrow.startBinding &&

packages/excalidraw/data/__snapshots__/transform.test.ts.snap

@@ -94,7 +94,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s
   "fillStyle": "solid",
   "frameId": null,
   "groupIds": [],
-  "height": 35,
+  "height": 33.53813187180941,
   "id": Any<String>,
   "index": "a2",
   "isDeleted": false,
@@ -427,7 +427,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id40",
+      "id": "id1",
       "type": "text",
     },
   ],
@@ -435,7 +435,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "elbowed": false,
   "endArrowhead": "arrow",
   "endBinding": {
-    "elementId": "id42",
+    "elementId": "id3",
     "focus": -0,
     "gap": 5,
   },
@@ -465,7 +465,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "seed": Any<Number>,
   "startArrowhead": null,
   "startBinding": {
-    "elementId": "id41",
+    "elementId": "id2",
     "focus": 0,
     "gap": 5,
   },
@@ -488,7 +488,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "autoResize": true,
   "backgroundColor": "transparent",
   "boundElements": null,
-  "containerId": "id39",
+  "containerId": "id0",
   "customData": undefined,
   "fillStyle": "solid",
   "fontFamily": 5,
@@ -529,7 +529,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id39",
+      "id": "id0",
       "type": "arrow",
     },
   ],
@@ -566,7 +566,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id39",
+      "id": "id0",
       "type": "arrow",
     },
   ],
@@ -592,7 +592,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe
   "version": 3,
   "versionNonce": Any<Number>,
   "width": 100,
-  "x": 355,
+  "x": 350,
   "y": 189,
 }
 `;
@@ -603,7 +603,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id44",
+      "id": "id1",
       "type": "text",
     },
   ],
@@ -611,7 +611,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "elbowed": false,
   "endArrowhead": "arrow",
   "endBinding": {
-    "elementId": "id46",
+    "elementId": "id3",
     "focus": -0,
     "gap": 5,
   },
@@ -641,7 +641,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "seed": Any<Number>,
   "startArrowhead": null,
   "startBinding": {
-    "elementId": "id45",
+    "elementId": "id2",
     "focus": 0,
     "gap": 5,
   },
@@ -664,7 +664,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "autoResize": true,
   "backgroundColor": "transparent",
   "boundElements": null,
-  "containerId": "id43",
+  "containerId": "id0",
   "customData": undefined,
   "fillStyle": "solid",
   "fontFamily": 5,
@@ -706,7 +706,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id43",
+      "id": "id0",
       "type": "arrow",
     },
   ],
@@ -752,7 +752,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "backgroundColor": "transparent",
   "boundElements": [
     {
-      "id": "id43",
+      "id": "id0",
       "type": "arrow",
     },
   ],
@@ -786,7 +786,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when
   "versionNonce": Any<Number>,
   "verticalAlign": "top",
   "width": 100,
-  "x": 355,
+  "x": 350,
   "y": 226.5,
 }
 `;
@@ -1480,7 +1480,7 @@ exports[`Test Transform > should transform the elements correctly when linear el
   "fillStyle": "solid",
   "frameId": null,
   "groupIds": [],
-  "height": 0,
+  "height": 7.105427357601002e-15,
   "id": Any<String>,
   "index": "a4",
   "isDeleted": false,

packages/excalidraw/data/transform.test.ts

@@ -464,7 +464,7 @@ describe("Test Transform", () => {
       });
 
       expect(ellipse).toMatchObject({
-        x: 355,
+        x: 350,
         y: 189,
         type: "ellipse",
         boundElements: [
@@ -549,7 +549,7 @@ describe("Test Transform", () => {
       });
 
       expect(text3).toMatchObject({
-        x: 355,
+        x: 350,
         y: 226.5,
         type: "text",
         boundElements: [

packages/excalidraw/tests/__snapshots__/history.test.tsx.snap

@@ -314,15 +314,14 @@ History {
                 "focus": "0.03194",
                 "gap": 5,
               },
-              "width": "92.92893",
             },
             "inserted": {
               "endBinding": {
                 "elementId": "id171",
-                "focus": "-0.02000",
+                "focus": "-0.02251",
                 "gap": 5,
               },
-              "height": "0.00002",
+              "height": "0.08238",
               "points": [
                 [
                   0,
@@ -330,15 +329,14 @@ History {
                 ],
                 [
                   "92.92893",
-                  "0.00002",
+                  "0.08238",
                 ],
               ],
               "startBinding": {
                 "elementId": "id170",
-                "focus": "0.02000",
+                "focus": "0.01897",
                 "gap": 5,
               },
-              "width": "92.92893",
             },
           },
         },
@@ -432,7 +430,7 @@ History {
               },
               "width": "92.92893",
               "x": "3.53553",
-              "y": "1.03376",
+              "y": "1.03339",
             },
           },
           "id175" => Delta {
@@ -860,11 +858,11 @@ History {
                   0,
                 ],
                 [
-                  0,
+                  "0.00000",
                   0,
                 ],
               ],
-              "width": 0,
+              "width": "0.00000",
             },
             "inserted": {
               "points": [
@@ -949,7 +947,7 @@ History {
                   0,
                 ],
                 [
-                  0,
+                  "0.00000",
                   0,
                 ],
               ],
@@ -958,7 +956,7 @@ History {
                 "focus": 0,
                 "gap": 5,
               },
-              "width": 0,
+              "width": "0.00000",
               "x": "146.46447",
             },
           },
@@ -2511,7 +2509,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -2529,7 +2527,7 @@ History {
               "strokeStyle": "solid",
               "strokeWidth": 2,
               "type": "arrow",
-              "width": "96.46447",
+              "width": "92.92893",
               "x": "3.53553",
               "y": 0,
             },
@@ -15254,7 +15252,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -15267,7 +15265,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -15562,7 +15560,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -15580,7 +15578,7 @@ History {
               "strokeStyle": "solid",
               "strokeWidth": 2,
               "type": "arrow",
-              "width": "96.46447",
+              "width": "92.92893",
               "x": "3.53553",
               "y": 0,
             },
@@ -16182,7 +16180,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -16200,7 +16198,7 @@ History {
               "strokeStyle": "solid",
               "strokeWidth": 2,
               "type": "arrow",
-              "width": "96.46447",
+              "width": "92.92893",
               "x": "3.53553",
               "y": 0,
             },
@@ -16802,7 +16800,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -16820,7 +16818,7 @@ History {
               "strokeStyle": "solid",
               "strokeWidth": 2,
               "type": "arrow",
-              "width": "96.46447",
+              "width": "92.92893",
               "x": "3.53553",
               "y": 0,
             },
@@ -17205,7 +17203,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -17222,7 +17220,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -17490,7 +17488,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -17508,7 +17506,7 @@ History {
               "strokeStyle": "solid",
               "strokeWidth": 2,
               "type": "arrow",
-              "width": "96.46447",
+              "width": "92.92893",
               "x": "3.53553",
               "y": 0,
             },
@@ -17933,7 +17931,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -17951,7 +17949,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -18219,7 +18217,7 @@ History {
                   0,
                 ],
                 [
-                  "96.46447",
+                  "92.92893",
                   0,
                 ],
               ],
@@ -18237,7 +18235,7 @@ History {
               "strokeStyle": "solid",
               "strokeWidth": 2,
               "type": "arrow",
-              "width": "96.46447",
+              "width": "92.92893",
               "x": "3.53553",
               "y": 0,
             },