From c8b832b0e1a91a45af992477c33f243402208ef7 Mon Sep 17 00:00:00 2001 From: dwelle <5153846+dwelle@users.noreply.github.com> Date: Tue, 11 Nov 2025 23:15:38 +0100 Subject: [PATCH] shorten focus point diagonal helpers to fix corner binding cases --- packages/element/src/utils.ts | 126 +-- .../tests/__snapshots__/history.test.tsx.snap | 868 +++++++++--------- 2 files changed, 520 insertions(+), 474 deletions(-) diff --git a/packages/element/src/utils.ts b/packages/element/src/utils.ts index 673dfefd16..59fd49680e 100644 --- a/packages/element/src/utils.ts +++ b/packages/element/src/utils.ts @@ -489,66 +489,82 @@ const getDiagonalsForBindableElement = ( element: ExcalidrawElement, elementsMap: ElementsMap, ) => { + // for rectangles, shrink the diagonals a bit because there's something + // going on with the focus points around the corners. Ask Mark for details. + const OFFSET_PX = element.type === "rectangle" ? 15 : 0; + const shrinkSegment = (seg: LineSegment) => { + const v = vectorNormalize(vectorFromPoint(seg[1], seg[0])); + const offset = vectorScale(v, OFFSET_PX); + return lineSegment( + pointTranslate(seg[0], offset), + pointTranslate(seg[1], vectorScale(offset, -1)), + ); + }; + const center = elementCenterPoint(element, elementsMap); - const diagonalOne = isRectangularElement(element) - ? lineSegment( - pointRotateRads( - pointFrom(element.x, element.y), - center, - element.angle, - ), - pointRotateRads( - pointFrom( - element.x + element.width, - element.y + element.height, + const diagonalOne = shrinkSegment( + isRectangularElement(element) + ? lineSegment( + pointRotateRads( + pointFrom(element.x, element.y), + center, + element.angle, ), - center, - element.angle, - ), - ) - : lineSegment( - pointRotateRads( - pointFrom(element.x + element.width / 2, element.y), - center, - element.angle, - ), - pointRotateRads( - pointFrom( - element.x + element.width / 2, - element.y + element.height, + pointRotateRads( + pointFrom( + element.x + element.width, + element.y + element.height, + ), + center, + element.angle, ), - center, - element.angle, - ), - ); - const diagonalTwo = isRectangularElement(element) - ? lineSegment( - pointRotateRads( - pointFrom(element.x + element.width, element.y), - center, - element.angle, - ), - pointRotateRads( - pointFrom(element.x, element.y + element.height), - center, - element.angle, - ), - ) - : lineSegment( - pointRotateRads( - pointFrom(element.x, element.y + element.height / 2), - center, - element.angle, - ), - pointRotateRads( - pointFrom( - element.x + element.width, - element.y + element.height / 2, + ) + : lineSegment( + pointRotateRads( + pointFrom(element.x + element.width / 2, element.y), + center, + element.angle, + ), + pointRotateRads( + pointFrom( + element.x + element.width / 2, + element.y + element.height, + ), + center, + element.angle, ), - center, - element.angle, ), - ); + ); + const diagonalTwo = shrinkSegment( + isRectangularElement(element) + ? lineSegment( + pointRotateRads( + pointFrom(element.x + element.width, element.y), + center, + element.angle, + ), + pointRotateRads( + pointFrom(element.x, element.y + element.height), + center, + element.angle, + ), + ) + : lineSegment( + pointRotateRads( + pointFrom(element.x, element.y + element.height / 2), + center, + element.angle, + ), + pointRotateRads( + pointFrom( + element.x + element.width, + element.y + element.height / 2, + ), + center, + element.angle, + ), + ), + ); return [diagonalOne, diagonalTwo]; }; diff --git a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap index 2daabc8720..436b0039d6 100644 --- a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap @@ -123,7 +123,12 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl { "angle": 0, "backgroundColor": "transparent", - "boundElements": [], + "boundElements": [ + { + "id": "id4", + "type": "arrow", + }, + ], "customData": undefined, "fillStyle": "solid", "frameId": null, @@ -142,7 +147,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 13, + "version": 7, "width": 100, "x": -100, "y": -50, @@ -153,7 +158,12 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl { "angle": 0, "backgroundColor": "transparent", - "boundElements": [], + "boundElements": [ + { + "id": "id4", + "type": "arrow", + }, + ], "customData": undefined, "fillStyle": "solid", "frameId": null, @@ -172,7 +182,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 9, + "version": 6, "width": 100, "x": 100, "y": -50, @@ -188,17 +198,17 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "elbowed": false, "endArrowhead": "arrow", "endBinding": { - "elementId": "id15", + "elementId": "id1", "fixedPoint": [ - "0.50000", - 1, + "0.41019", + "0.58981", ], "mode": "orbit", }, "fillStyle": "solid", "frameId": null, "groupIds": [], - "height": "106.79573", + "height": "2.03061", "id": "id4", "index": "a2", "isDeleted": false, @@ -212,8 +222,8 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl 0, ], [ - "89.00000", - "106.79573", + "78.00000", + "-2.03061", ], ], "roughness": 1, @@ -221,16 +231,23 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "type": 2, }, "startArrowhead": null, - "startBinding": null, + "startBinding": { + "elementId": "id0", + "fixedPoint": [ + "0.63636", + "0.63636", + ], + "mode": "orbit", + }, "strokeColor": "#1e1e1e", "strokeStyle": "solid", "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 34, - "width": "89.00000", - "x": 0, - "y": 0, + "version": 19, + "width": "78.00000", + "x": 11, + "y": "12.36576", } `; @@ -238,12 +255,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl { "angle": 0, "backgroundColor": "transparent", - "boundElements": [ - { - "id": "id4", - "type": "arrow", - }, - ], + "boundElements": [], "customData": undefined, "fillStyle": "solid", "frameId": null, @@ -262,7 +274,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 10, + "version": 4, "width": 50, "x": 100, "y": 100, @@ -271,214 +283,9 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and the arrow got bound to a different element in the meantime > [end of test] number of elements 1`] = `4`; -exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and the arrow got bound to a different element in the meantime > [end of test] number of renders 1`] = `22`; +exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and the arrow got bound to a different element in the meantime > [end of test] number of renders 1`] = `16`; -exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and the arrow got bound to a different element in the meantime > [end of test] redo stack 1`] = ` -[ - { - "appState": AppStateDelta { - "delta": Delta { - "deleted": {}, - "inserted": {}, - }, - }, - "elements": { - "added": {}, - "removed": {}, - "updated": { - "id0": { - "deleted": { - "version": 12, - }, - "inserted": { - "version": 11, - }, - }, - "id1": { - "deleted": { - "boundElements": [], - "version": 9, - }, - "inserted": { - "boundElements": [ - { - "id": "id4", - "type": "arrow", - }, - ], - "version": 8, - }, - }, - "id15": { - "deleted": { - "boundElements": [ - { - "id": "id4", - "type": "arrow", - }, - ], - "version": 9, - }, - "inserted": { - "boundElements": [], - "version": 8, - }, - }, - "id4": { - "deleted": { - "endBinding": { - "elementId": "id15", - "fixedPoint": [ - "0.50000", - 1, - ], - "mode": "orbit", - }, - "height": "66.30324", - "points": [ - [ - 0, - 0, - ], - [ - "78.47830", - "66.30324", - ], - ], - "startBinding": { - "elementId": "id0", - "fixedPoint": [ - "0.63636", - "0.63636", - ], - "mode": "orbit", - }, - "version": 33, - "width": "78.47830", - "y": "53.20079", - }, - "inserted": { - "endBinding": { - "elementId": "id1", - "fixedPoint": [ - "0.41019", - "0.58981", - ], - "mode": "orbit", - }, - "height": "2.04467", - "points": [ - [ - 0, - 0, - ], - [ - "78.00000", - "-2.04467", - ], - ], - "startBinding": { - "elementId": "id0", - "fixedPoint": [ - "0.63636", - "0.63636", - ], - "mode": "orbit", - }, - "version": 30, - "width": "78.00000", - "y": "12.38727", - }, - }, - }, - }, - "id": "id22", - }, - { - "appState": AppStateDelta { - "delta": Delta { - "deleted": {}, - "inserted": {}, - }, - }, - "elements": { - "added": {}, - "removed": {}, - "updated": { - "id0": { - "deleted": { - "boundElements": [], - "version": 13, - }, - "inserted": { - "boundElements": [ - { - "id": "id4", - "type": "arrow", - }, - ], - "version": 12, - }, - }, - "id15": { - "deleted": { - "version": 10, - }, - "inserted": { - "version": 9, - }, - }, - "id4": { - "deleted": { - "height": "106.79573", - "points": [ - [ - 0, - 0, - ], - [ - "89.00000", - "106.79573", - ], - ], - "startBinding": null, - "version": 34, - "width": "89.00000", - "x": 0, - "y": 0, - }, - "inserted": { - "height": "66.30324", - "points": [ - [ - 0, - 0, - ], - [ - "78.47830", - "66.30324", - ], - ], - "startBinding": { - "elementId": "id0", - "fixedPoint": [ - "0.63636", - "0.63636", - ], - "mode": "orbit", - }, - "version": 33, - "width": "78.47830", - "x": "10.52170", - "y": "53.20079", - }, - }, - }, - }, - "id": "id23", - }, -] -`; +exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and the arrow got bound to a different element in the meantime > [end of test] redo stack 1`] = `[]`; exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and the arrow got bound to a different element in the meantime > [end of test] undo stack 1`] = ` [ @@ -633,6 +440,208 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl }, "id": "id6", }, + { + "appState": AppStateDelta { + "delta": Delta { + "deleted": {}, + "inserted": {}, + }, + }, + "elements": { + "added": {}, + "removed": {}, + "updated": { + "id0": { + "deleted": { + "boundElements": [ + { + "id": "id4", + "type": "arrow", + }, + ], + "version": 6, + }, + "inserted": { + "boundElements": [], + "version": 5, + }, + }, + "id15": { + "deleted": { + "version": 3, + }, + "inserted": { + "version": 2, + }, + }, + "id4": { + "deleted": { + "height": "57.11798", + "points": [ + [ + 0, + 0, + ], + [ + 78, + "57.11798", + ], + ], + "startBinding": { + "elementId": "id0", + "fixedPoint": [ + "0.63636", + "0.63636", + ], + "mode": "orbit", + }, + "version": 16, + "width": 78, + "x": 11, + "y": "48.31989", + }, + "inserted": { + "height": 0, + "points": [ + [ + 0, + 0, + ], + [ + 100, + 0, + ], + ], + "startBinding": null, + "version": 13, + "width": 100, + "x": 0, + "y": 0, + }, + }, + }, + }, + "id": "id16", + }, + { + "appState": AppStateDelta { + "delta": Delta { + "deleted": {}, + "inserted": {}, + }, + }, + "elements": { + "added": {}, + "removed": {}, + "updated": { + "id0": { + "deleted": { + "version": 7, + }, + "inserted": { + "version": 6, + }, + }, + "id1": { + "deleted": { + "boundElements": [ + { + "id": "id4", + "type": "arrow", + }, + ], + "version": 6, + }, + "inserted": { + "boundElements": [], + "version": 5, + }, + }, + "id15": { + "deleted": { + "boundElements": [], + "version": 4, + }, + "inserted": { + "boundElements": [ + { + "id": "id4", + "type": "arrow", + }, + ], + "version": 3, + }, + }, + "id4": { + "deleted": { + "endBinding": { + "elementId": "id1", + "fixedPoint": [ + "0.41019", + "0.58981", + ], + "mode": "orbit", + }, + "height": "2.03061", + "points": [ + [ + 0, + 0, + ], + [ + "78.00000", + "-2.03061", + ], + ], + "startBinding": { + "elementId": "id0", + "fixedPoint": [ + "0.63636", + "0.63636", + ], + "mode": "orbit", + }, + "version": 19, + "width": "78.00000", + "y": "12.36576", + }, + "inserted": { + "endBinding": { + "elementId": "id15", + "fixedPoint": [ + "0.50000", + 1, + ], + "mode": "orbit", + }, + "height": "57.11798", + "points": [ + [ + 0, + 0, + ], + [ + 78, + "57.11798", + ], + ], + "startBinding": { + "elementId": "id0", + "fixedPoint": [ + "0.63636", + "0.63636", + ], + "mode": "orbit", + }, + "version": 16, + "width": 78, + "y": "48.31989", + }, + }, + }, + }, + "id": "id17", + }, ] `; @@ -759,7 +768,12 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl { "angle": 0, "backgroundColor": "transparent", - "boundElements": [], + "boundElements": [ + { + "id": "id4", + "type": "arrow", + }, + ], "customData": undefined, "fillStyle": "solid", "frameId": null, @@ -778,7 +792,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 14, + "version": 8, "width": 100, "x": 150, "y": -50, @@ -789,7 +803,12 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl { "angle": 0, "backgroundColor": "transparent", - "boundElements": [], + "boundElements": [ + { + "id": "id4", + "type": "arrow", + }, + ], "customData": undefined, "fillStyle": "solid", "frameId": null, @@ -808,7 +827,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 9, + "version": 6, "width": 100, "x": 150, "y": -50, @@ -823,11 +842,18 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "customData": undefined, "elbowed": false, "endArrowhead": "arrow", - "endBinding": null, + "endBinding": { + "elementId": "id1", + "fixedPoint": [ + "0.41092", + "0.58908", + ], + "mode": "orbit", + }, "fillStyle": "solid", "frameId": null, "groupIds": [], - "height": 0, + "height": "10.92646", "id": "id4", "index": "a2", "isDeleted": false, @@ -841,8 +867,8 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl 0, ], [ - 100, - 0, + "52.09230", + "10.92646", ], ], "roughness": 1, @@ -850,199 +876,31 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "type": 2, }, "startArrowhead": null, - "startBinding": null, + "startBinding": { + "elementId": "id0", + "fixedPoint": [ + "0.63636", + "0.63636", + ], + "mode": "orbit", + }, "strokeColor": "#1e1e1e", "strokeStyle": "solid", "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 30, - "width": 100, - "x": 150, - "y": 0, + "version": 20, + "width": "52.09230", + "x": 139, + "y": "-2.01876", } `; exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and there are no conflicting updates in the meantime > [end of test] number of elements 1`] = `3`; -exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and there are no conflicting updates in the meantime > [end of test] number of renders 1`] = `24`; +exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and there are no conflicting updates in the meantime > [end of test] number of renders 1`] = `18`; -exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and there are no conflicting updates in the meantime > [end of test] redo stack 1`] = ` -[ - { - "appState": AppStateDelta { - "delta": Delta { - "deleted": {}, - "inserted": {}, - }, - }, - "elements": { - "added": {}, - "removed": {}, - "updated": { - "id0": { - "deleted": { - "version": 13, - }, - "inserted": { - "version": 12, - }, - }, - "id1": { - "deleted": { - "boundElements": [], - "version": 9, - }, - "inserted": { - "boundElements": [ - { - "id": "id4", - "type": "arrow", - }, - ], - "version": 8, - }, - }, - "id4": { - "deleted": { - "endBinding": null, - "height": "4.68000", - "points": [ - [ - 0, - 0, - ], - [ - "-39.00000", - "-4.68000", - ], - ], - "startBinding": { - "elementId": "id0", - "fixedPoint": [ - "0.63636", - "0.63636", - ], - "mode": "orbit", - }, - "version": 29, - "width": "39.00000", - "y": "4.68000", - }, - "inserted": { - "endBinding": { - "elementId": "id1", - "fixedPoint": [ - "0.41092", - "0.58908", - ], - "mode": "orbit", - }, - "height": "10.92646", - "points": [ - [ - 0, - 0, - ], - [ - "52.09230", - "10.92646", - ], - ], - "startBinding": { - "elementId": "id0", - "fixedPoint": [ - "0.63636", - "0.63636", - ], - "mode": "orbit", - }, - "version": 27, - "width": "52.09230", - "y": "-2.01876", - }, - }, - }, - }, - "id": "id21", - }, - { - "appState": AppStateDelta { - "delta": Delta { - "deleted": {}, - "inserted": {}, - }, - }, - "elements": { - "added": {}, - "removed": {}, - "updated": { - "id0": { - "deleted": { - "boundElements": [], - "version": 14, - }, - "inserted": { - "boundElements": [ - { - "id": "id4", - "type": "arrow", - }, - ], - "version": 13, - }, - }, - "id4": { - "deleted": { - "height": 0, - "points": [ - [ - 0, - 0, - ], - [ - 100, - 0, - ], - ], - "startBinding": null, - "version": 30, - "width": 100, - "x": 150, - "y": 0, - }, - "inserted": { - "height": "4.68000", - "points": [ - [ - 0, - 0, - ], - [ - "-39.00000", - "-4.68000", - ], - ], - "startBinding": { - "elementId": "id0", - "fixedPoint": [ - "0.63636", - "0.63636", - ], - "mode": "orbit", - }, - "version": 29, - "width": "39.00000", - "x": 139, - "y": "4.68000", - }, - }, - }, - }, - "id": "id22", - }, -] -`; +exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and there are no conflicting updates in the meantime > [end of test] redo stack 1`] = `[]`; exports[`history > multiplayer undo/redo > conflicts in arrows and their bindable elements > should rebind bindings when both are updated through the history and there are no conflicting updates in the meantime > [end of test] undo stack 1`] = ` [ @@ -1197,6 +1055,178 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl }, "id": "id6", }, + { + "appState": AppStateDelta { + "delta": Delta { + "deleted": {}, + "inserted": {}, + }, + }, + "elements": { + "added": {}, + "removed": {}, + "updated": { + "id0": { + "deleted": { + "boundElements": [ + { + "id": "id4", + "type": "arrow", + }, + ], + "version": 7, + }, + "inserted": { + "boundElements": [], + "version": 6, + }, + }, + "id4": { + "deleted": { + "height": "4.68000", + "points": [ + [ + 0, + 0, + ], + [ + -39, + "-4.68000", + ], + ], + "startBinding": { + "elementId": "id0", + "fixedPoint": [ + "0.63636", + "0.63636", + ], + "mode": "orbit", + }, + "version": 17, + "width": 39, + "x": 139, + "y": "4.68000", + }, + "inserted": { + "height": 0, + "points": [ + [ + 0, + 0, + ], + [ + 100, + 0, + ], + ], + "startBinding": null, + "version": 15, + "width": 100, + "x": 150, + "y": 0, + }, + }, + }, + }, + "id": "id15", + }, + { + "appState": AppStateDelta { + "delta": Delta { + "deleted": {}, + "inserted": {}, + }, + }, + "elements": { + "added": {}, + "removed": {}, + "updated": { + "id0": { + "deleted": { + "version": 8, + }, + "inserted": { + "version": 7, + }, + }, + "id1": { + "deleted": { + "boundElements": [ + { + "id": "id4", + "type": "arrow", + }, + ], + "version": 6, + }, + "inserted": { + "boundElements": [], + "version": 5, + }, + }, + "id4": { + "deleted": { + "endBinding": { + "elementId": "id1", + "fixedPoint": [ + "0.41092", + "0.58908", + ], + "mode": "orbit", + }, + "height": "10.92646", + "points": [ + [ + 0, + 0, + ], + [ + "52.09230", + "10.92646", + ], + ], + "startBinding": { + "elementId": "id0", + "fixedPoint": [ + "0.63636", + "0.63636", + ], + "mode": "orbit", + }, + "version": 20, + "width": "52.09230", + "y": "-2.01876", + }, + "inserted": { + "endBinding": null, + "height": "4.68000", + "points": [ + [ + 0, + 0, + ], + [ + -39, + "-4.68000", + ], + ], + "startBinding": { + "elementId": "id0", + "fixedPoint": [ + "0.63636", + "0.63636", + ], + "mode": "orbit", + }, + "version": 17, + "width": 39, + "y": "4.68000", + }, + }, + }, + }, + "id": "id16", + }, ] `;