From 06b3750a2f7ca707774f32d06bf53debe763e4b9 Mon Sep 17 00:00:00 2001 From: Mark Tolmacs Date: Fri, 4 Apr 2025 18:33:36 +0200 Subject: [PATCH] Fix microjump on drag binding, no keyboard move if bound arrow Signed-off-by: Mark Tolmacs --- packages/element/src/binding.ts | 12 ++++ packages/element/src/linearElementEditor.ts | 27 +++++---- packages/element/tests/binding.test.tsx | 14 ++--- packages/element/tests/resize.test.tsx | 2 +- packages/excalidraw/components/App.tsx | 2 +- .../data/__snapshots__/transform.test.ts.snap | 32 +++++------ packages/excalidraw/data/transform.test.ts | 4 +- .../tests/__snapshots__/history.test.tsx.snap | 56 +++++++++---------- 8 files changed, 80 insertions(+), 69 deletions(-) diff --git a/packages/element/src/binding.ts b/packages/element/src/binding.ts index 09faac5935..ad3af4e6d9 100644 --- a/packages/element/src/binding.ts +++ b/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, }); diff --git a/packages/element/src/linearElementEditor.ts b/packages/element/src/linearElementEditor.ts index 229518d79c..837f755ce5 100644 --- a/packages/element/src/linearElementEditor.ts +++ b/packages/element/src/linearElementEditor.ts @@ -356,16 +356,17 @@ export class LinearElementEditor { elementsMap, true, ); + const newGlobalPointPosition = pointRotateRads( + pointFrom( + element.x + newPointPosition[0], + element.y + newPointPosition[1], + ), + pointFrom(cx, cy), + element.angle, + ); const avoidancePoint = getOutlineAvoidingPoint( element, - pointRotateRads( - pointFrom( - element.x + newPointPosition[0], - element.y + newPointPosition[1], - ), - pointFrom(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, ); } diff --git a/packages/element/tests/binding.test.tsx b/packages/element/tests/binding.test.tsx index 8a518777c0..031f80fdd0 100644 --- a/packages/element/tests/binding.test.tsx +++ b/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", () => { diff --git a/packages/element/tests/resize.test.tsx b/packages/element/tests/resize.test.tsx index f2381223e3..8479462573 100644 --- a/packages/element/tests/resize.test.tsx +++ b/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(); diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 968f39d220..4bb23b8a9c 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -4375,7 +4375,7 @@ class App extends React.Component { const arrowIdsToRemove = new Set(); selectedElements - .filter(isElbowArrow) + .filter(isArrowElement) .filter((arrow) => { const startElementNotInSelection = arrow.startBinding && diff --git a/packages/excalidraw/data/__snapshots__/transform.test.ts.snap b/packages/excalidraw/data/__snapshots__/transform.test.ts.snap index 00331bd11f..83f3497b0b 100644 --- a/packages/excalidraw/data/__snapshots__/transform.test.ts.snap +++ b/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, "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, "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, "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, "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, "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, "index": "a4", "isDeleted": false, diff --git a/packages/excalidraw/data/transform.test.ts b/packages/excalidraw/data/transform.test.ts index 46a86c6e63..e79a4fcc7b 100644 --- a/packages/excalidraw/data/transform.test.ts +++ b/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: [ diff --git a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap index 09f6334571..05611e8aa1 100644 --- a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap +++ b/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, },