From 4efa6f69e53ccc29b85ac9b067d00809fecc308f Mon Sep 17 00:00:00 2001 From: Mark Tolmacs Date: Sat, 29 Mar 2025 19:06:52 +0100 Subject: [PATCH] Inline binding logic fix --- packages/element/src/binding.ts | 50 +++++++++++++++---- packages/element/src/elbowArrow.ts | 12 +++-- packages/element/tests/binding.test.tsx | 28 +++++++---- .../excalidraw/actions/actionFinalize.tsx | 8 +-- packages/excalidraw/components/App.tsx | 9 ---- 5 files changed, 65 insertions(+), 42 deletions(-) diff --git a/packages/element/src/binding.ts b/packages/element/src/binding.ts index 02edb0db35..954518837d 100644 --- a/packages/element/src/binding.ts +++ b/packages/element/src/binding.ts @@ -223,7 +223,7 @@ const bindOrUnbindLinearElementEdge = ( } }; -const getOriginalBindingsIfStillCloseToArrowEnds = ( +export const getOriginalBindingsIfStillCloseToArrowEnds = ( linearElement: NonDeleted, elementsMap: NonDeletedSceneElementsMap, zoom?: AppState["zoom"], @@ -418,10 +418,47 @@ export const getSuggestedBindingsForArrows = ( export const maybeBindLinearElement = ( linearElement: NonDeleted, appState: AppState, - pointerCoords: { x: number; y: number }, elementsMap: NonDeletedSceneElementsMap, elements: readonly NonDeletedExcalidrawElement[], ): void => { + const start = tupleToCoors( + LinearElementEditor.getPointAtIndexGlobalCoordinates( + linearElement, + 0, + elementsMap, + ), + ); + const end = tupleToCoors( + LinearElementEditor.getPointAtIndexGlobalCoordinates( + linearElement, + -1, + elementsMap, + ), + ); + + const otherHoveredElement = getHoveredElementForBinding( + start, + elements, + elementsMap, + appState.zoom, + isElbowArrow(linearElement), + isElbowArrow(linearElement), + ); + + const hoveredElement = getHoveredElementForBinding( + end, + elements, + elementsMap, + appState.zoom, + isElbowArrow(linearElement), + isElbowArrow(linearElement), + ); + + // Inside the same element there is no binding to the shape + if (hoveredElement === otherHoveredElement) { + return; + } + if (appState.startBoundElement != null) { bindLinearElement( linearElement, @@ -431,15 +468,6 @@ export const maybeBindLinearElement = ( ); } - const hoveredElement = getHoveredElementForBinding( - pointerCoords, - elements, - elementsMap, - appState.zoom, - isElbowArrow(linearElement), - isElbowArrow(linearElement), - ); - if (hoveredElement !== null) { if ( !isLinearElementSimpleAndAlreadyBoundOnOppositeEdge( diff --git a/packages/element/src/elbowArrow.ts b/packages/element/src/elbowArrow.ts index d77bf8280d..a028403b3d 100644 --- a/packages/element/src/elbowArrow.ts +++ b/packages/element/src/elbowArrow.ts @@ -50,7 +50,6 @@ import { isBindableElement } from "./typeChecks"; import { type ExcalidrawElbowArrowElement, type NonDeletedSceneElementsMap, - type SceneElementsMap, } from "./types"; import { aabbForElement, pointInsideBounds } from "./shapes"; @@ -1237,6 +1236,14 @@ const getElbowArrowData = ( true, true, ) || null; + + // Inside the same element there is no binding to the shape + if (hoveredStartElement === hoveredEndElement) { + hoveredStartElement = null; + hoveredEndElement = null; + arrow.startBinding = null; + arrow.endBinding = null; + } } else { hoveredStartElement = arrow.startBinding ? getBindableElementForId(arrow.startBinding.elementId, elementsMap) || @@ -1278,14 +1285,12 @@ const getElbowArrowData = ( const startHeading = getBindPointHeading( startGlobalPoint, endGlobalPoint, - elementsMap, hoveredStartElement, origStartGlobalPoint, ); const endHeading = getBindPointHeading( endGlobalPoint, startGlobalPoint, - elementsMap, hoveredEndElement, origEndGlobalPoint, ); @@ -2257,7 +2262,6 @@ const getGlobalPoint = ( const getBindPointHeading = ( p: GlobalPoint, otherPoint: GlobalPoint, - elementsMap: NonDeletedSceneElementsMap | SceneElementsMap, hoveredElement: ExcalidrawBindableElement | null | undefined, origPoint: GlobalPoint, ): Heading => diff --git a/packages/element/tests/binding.test.tsx b/packages/element/tests/binding.test.tsx index a667597b6c..8a518777c0 100644 --- a/packages/element/tests/binding.test.tsx +++ b/packages/element/tests/binding.test.tsx @@ -10,6 +10,8 @@ import { API } from "@excalidraw/excalidraw/tests/helpers/api"; import { UI, Pointer, Keyboard } from "@excalidraw/excalidraw/tests/helpers/ui"; import { fireEvent, render } from "@excalidraw/excalidraw/tests/test-utils"; +import "@excalidraw/utils/test-utils"; + import { getTransformHandles } from "../src/transformHandles"; const { h } = window; @@ -510,7 +512,7 @@ describe("element binding", () => { "allow non-binding simple (complex) arrow creation while start and end" + " points are in the same shape", () => { - UI.createElement("rectangle", { + const rect = UI.createElement("rectangle", { x: 0, y: 0, width: 100, @@ -526,9 +528,10 @@ describe("element binding", () => { expect(arrow.startBinding).toBe(null); expect(arrow.endBinding).toBe(null); + expect(rect.boundElements).toEqual(null); expect(arrow.points).toCloselyEqualPoints([ [0, 0], - [95, 95], + [92.2855, 92.2855], ]); const rect2 = API.createElement({ @@ -552,32 +555,35 @@ describe("element binding", () => { expect(arrow2.startBinding).toBe(null); expect(arrow2.endBinding).toBe(null); + expect(rect2.boundElements).toEqual(null); expect(arrow2.points).toCloselyEqualPoints([ [0, 0], - [95, 95], + [92.2855, 92.2855], ]); - UI.createElement("rectangle", { + const rect3 = UI.createElement("rectangle", { x: 0, - y: 0, + y: 300, width: 100, height: 100, }); const arrow3 = UI.createElement("arrow", { - x: 5, - y: 5, - height: 95, - width: 95, + x: 10, + y: 310, + height: 85, + width: 84, elbowed: true, }); expect(arrow3.startBinding).toBe(null); expect(arrow3.endBinding).toBe(null); + expect(rect3.boundElements).toEqual(null); expect(arrow3.points).toCloselyEqualPoints([ [0, 0], - [45, 45], - [95, 95], + [0, 42.5], + [84, 42.5], + [84, 85], ]); }, ); diff --git a/packages/excalidraw/actions/actionFinalize.tsx b/packages/excalidraw/actions/actionFinalize.tsx index b29898e7c4..6c881c01e9 100644 --- a/packages/excalidraw/actions/actionFinalize.tsx +++ b/packages/excalidraw/actions/actionFinalize.tsx @@ -13,7 +13,7 @@ import { isLinearElement, } from "@excalidraw/element/typeChecks"; -import { KEYS, arrayToMap, updateActiveTool } from "@excalidraw/common"; +import { KEYS, updateActiveTool } from "@excalidraw/common"; import { isPathALoop } from "@excalidraw/element/shapes"; import { isInvisiblySmallElement } from "@excalidraw/element/sizeHelpers"; @@ -153,15 +153,9 @@ export const actionFinalize = register({ !isLoop && multiPointElement.points.length > 1 ) { - const [x, y] = LinearElementEditor.getPointAtIndexGlobalCoordinates( - multiPointElement, - -1, - arrayToMap(elements), - ); maybeBindLinearElement( multiPointElement, appState, - { x, y }, elementsMap, elements, ); diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 2bf9664043..968f39d220 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -2770,7 +2770,6 @@ class App extends React.Component { this.updateEmbeddables(); const elements = this.scene.getElementsIncludingDeleted(); const elementsMap = this.scene.getElementsMapIncludingDeleted(); - const nonDeletedElementsMap = this.scene.getNonDeletedElementsMap(); if (!this.state.showWelcomeScreen && !elements.length) { this.setState({ showWelcomeScreen: true }); @@ -2927,13 +2926,6 @@ class App extends React.Component { maybeBindLinearElement( multiElement, this.state, - tupleToCoors( - LinearElementEditor.getPointAtIndexGlobalCoordinates( - multiElement, - -1, - nonDeletedElementsMap, - ), - ), this.scene.getNonDeletedElementsMap(), this.scene.getNonDeletedElements(), ); @@ -9174,7 +9166,6 @@ class App extends React.Component { maybeBindLinearElement( newElement, this.state, - pointerCoords, this.scene.getNonDeletedElementsMap(), this.scene.getNonDeletedElements(), );