From b3bc40ef01d3d847491f1e2a22763f933933ce97 Mon Sep 17 00:00:00 2001 From: Mark Tolmacs Date: Fri, 25 Jul 2025 16:13:45 +0200 Subject: [PATCH] Move point click arrow creation over to common strategy Signed-off-by: Mark Tolmacs --- packages/element/src/binding.ts | 107 ++++++++++++++- packages/excalidraw/components/App.tsx | 173 +++++++------------------ 2 files changed, 150 insertions(+), 130 deletions(-) diff --git a/packages/element/src/binding.ts b/packages/element/src/binding.ts index 18bc166a69..beb096c094 100644 --- a/packages/element/src/binding.ts +++ b/packages/element/src/binding.ts @@ -135,7 +135,10 @@ export const bindOrUnbindBindingElement = ( scene.getNonDeletedElementsMap(), scene.getNonDeletedElements(), appState, - opts, + { + ...opts, + appState, + }, ); bindOrUnbindBindingElementEdge(arrow, start, "start", scene); bindOrUnbindBindingElementEdge(arrow, end, "end", scene); @@ -225,6 +228,71 @@ const getOriginalBindingsIfStillCloseToBindingEnds = ( return null; }); +export const getStartGlobalEndLocalPointsForBinding = ( + arrow: NonDeleted, + start: BindingStrategy, + end: BindingStrategy, + startPoint: GlobalPoint, + endPoint: LocalPoint, + elementsMap: ElementsMap, +): [GlobalPoint, LocalPoint] => { + let startGlobalPoint = startPoint; + let endLocalPoint = endPoint; + if (start.mode) { + const newStartLocalPoint = updateBoundPoint( + arrow, + "startBinding", + start.mode + ? { + ...calculateFixedPointForNonElbowArrowBinding( + arrow, + start.element, + "start", + elementsMap, + start.focusPoint, + ), + elementId: start.element.id, + mode: start.mode, + } + : null, + start.element, + elementsMap, + ); + startGlobalPoint = newStartLocalPoint + ? LinearElementEditor.getPointGlobalCoordinates( + arrow, + newStartLocalPoint, + elementsMap, + ) + : startGlobalPoint; + } + + if (end.mode) { + const newEndLocalPoint = updateBoundPoint( + arrow, + "endBinding", + end.mode + ? { + ...calculateFixedPointForNonElbowArrowBinding( + arrow, + end.element, + "end", + elementsMap, + end.focusPoint, + ), + elementId: end.element.id, + mode: end.mode, + } + : null, + end.element, + elementsMap, + ); + endLocalPoint = newEndLocalPoint ?? endLocalPoint; + } + + return [startGlobalPoint, endLocalPoint]; +}; + const bindingStrategyForEndpointDragging = ( point: GlobalPoint, oppositeBinding: FixedPointBinding | null, @@ -233,7 +301,8 @@ const bindingStrategyForEndpointDragging = ( zoom: AppState["zoom"], globalBindMode?: AppState["bindMode"], opts?: { - newArrow: boolean; + newArrow?: boolean; + appState?: AppState; }, ): { current: BindingStrategy; other: BindingStrategy } => { let current: BindingStrategy = { mode: undefined }; @@ -256,7 +325,34 @@ const bindingStrategyForEndpointDragging = ( return { current, other }; } - // Dragged start point is outside of any bindable element + // Update the start point on new arrows + if ( + opts?.newArrow && + opts?.appState?.selectedLinearElement && + oppositeBinding + ) { + const oppositeBindingElement = elementsMap.get( + oppositeBinding?.elementId, + ) as ExcalidrawBindableElement; + + if (oppositeBinding.elementId !== hovered?.id) { + other = { + element: oppositeBindingElement, + mode: "orbit", + focusPoint: elementCenterPoint(oppositeBindingElement, elementsMap), + }; + } else { + other = { + element: oppositeBindingElement, + mode: "inside", + focusPoint: + opts.appState.selectedLinearElement.pointerDownState + .arrowOriginalStartPoint, + }; + } + } + + // Dragged point is outside of any bindable element // so we break any existing binding if (!hovered) { return { current: { mode: null }, other }; @@ -342,14 +438,15 @@ const bindingStrategyForEndpointDragging = ( return { current, other }; }; -const getBindingStrategyForDraggingBindingElementEndpoints = ( +export const getBindingStrategyForDraggingBindingElementEndpoints = ( arrow: NonDeleted, draggingPoints: PointsPositionUpdates, elementsMap: NonDeletedSceneElementsMap, elements: readonly Ordered[], appState: AppState, opts?: { - newArrow: boolean; + newArrow?: boolean; + appState?: AppState; }, ): { start: BindingStrategy; end: BindingStrategy } => { const globalBindMode = appState.bindMode || "focus"; diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 979a0257b9..617d86bda2 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -16,7 +16,6 @@ import { vectorSubtract, vectorDot, vectorNormalize, - pointsEqual, lineSegment, } from "@excalidraw/math"; @@ -240,7 +239,8 @@ import { calculateFixedPointForNonElbowArrowBinding, normalizeFixedPoint, bindOrUnbindBindingElement, - updateBoundPoint, + getBindingStrategyForDraggingBindingElementEndpoints, + getStartGlobalEndLocalPointsForBinding, } from "@excalidraw/element"; import type { GlobalPoint, LocalPoint, Radians } from "@excalidraw/math"; @@ -6155,81 +6155,60 @@ class App extends React.Component { )); } - if ( - isBindingElement(multiElement) && - this.state.bindMode === "orbit" && - isBindingEnabled(this.state) - ) { - const elementsMap = this.scene.getNonDeletedElementsMap(); - const hoveredElement = getHoveredElementForBinding( - pointFrom(scenePointerX, scenePointerY), - this.scene.getNonDeletedElements(), - this.scene.getNonDeletedElementsMap(), - this.state.zoom, - ); - const otherPoint = - LinearElementEditor.getPointAtIndexGlobalCoordinates( - multiElement, - 0, - this.scene.getNonDeletedElementsMap(), - ); - const otherHoveredElement = getHoveredElementForBinding( - otherPoint, - this.scene.getNonDeletedElements(), - this.scene.getNonDeletedElementsMap(), - this.state.zoom, - ); - - if (hoveredElement?.id !== otherHoveredElement?.id) { - const avoidancePoint = - multiElement && - hoveredElement && - getOutlineAvoidingPoint( - multiElement, - hoveredElement, - pointFrom(scenePointerX, scenePointerY), - multiElement.points.length - 1, - elementsMap, - shouldRotateWithDiscreteAngle(event) - ? lineSegment( - otherPoint, - pointFrom( - multiElement.x + lastCommittedX + dxFromLastCommitted, - multiElement.y + lastCommittedY + dyFromLastCommitted, - ), - ) - : undefined, - ); - const x = avoidancePoint - ? avoidancePoint[0] - : hoveredElement - ? scenePointerX - : gridX; - const y = avoidancePoint - ? avoidancePoint[1] - : hoveredElement - ? scenePointerY - : gridY; - dxFromLastCommitted = x - rx - lastCommittedX; - dyFromLastCommitted = y - ry - lastCommittedY; - } - } - if (isPathALoop(points, this.state.zoom.value)) { setCursor(this.interactiveCanvas, CURSOR_TYPE.POINTER); } + // Update arrow points + const elementsMap = this.scene.getNonDeletedElementsMap(); + let startGlobalPoint = + this.state.selectedLinearElement?.pointerDownState + ?.arrowOriginalStartPoint ?? + LinearElementEditor.getPointAtIndexGlobalCoordinates( + multiElement, + 0, + elementsMap, + ); + let endLocalPoint = pointFrom( + lastCommittedX + dxFromLastCommitted, + lastCommittedY + dyFromLastCommitted, + ); + + if (isBindingElement(multiElement)) { + const point = pointFrom( + scenePointerX - rx, + scenePointerY - ry, + ); + const { start, end } = + getBindingStrategyForDraggingBindingElementEndpoints( + multiElement, + new Map([ + [multiElement.points.length - 1, { point, isDragging: true }], + ]), + elementsMap, + this.scene.getNonDeletedElements(), + this.state, + { newArrow: !!this.state.newElement, appState: this.state }, + ); + + [startGlobalPoint, endLocalPoint] = + getStartGlobalEndLocalPointsForBinding( + multiElement, + start, + end, + startGlobalPoint, + endLocalPoint, + elementsMap, + ); + } + // update last uncommitted point this.scene.mutateElement( multiElement, { - points: [ - ...points.slice(0, -1), - pointFrom( - lastCommittedX + dxFromLastCommitted, - lastCommittedY + dyFromLastCommitted, - ), - ], + x: startGlobalPoint[0], + y: startGlobalPoint[1], + points: [...points.slice(0, -1), endLocalPoint], }, { isDragging: true, @@ -6237,62 +6216,6 @@ class App extends React.Component { }, ); - // If start is bound then snap the fixed binding point if needed - if ( - isArrowElement(multiElement) && - multiElement.startBinding && - multiElement.startBinding.mode === "orbit" - ) { - const elementsMap = this.scene.getNonDeletedElementsMap(); - const hoveredElement = getHoveredElementForBinding( - pointFrom(scenePointerX, scenePointerY), - this.scene.getNonDeletedElements(), - this.scene.getNonDeletedElementsMap(), - this.state.zoom, - ); - if ( - !hoveredElement || - hoveredElement.id !== multiElement.startBinding.elementId - ) { - const startPoint = - LinearElementEditor.getPointAtIndexGlobalCoordinates( - multiElement, - 0, - elementsMap, - ); - const startElement = this.scene.getElement( - multiElement.startBinding.elementId, - ) as ExcalidrawBindableElement; - const localPoint = updateBoundPoint( - multiElement, - "startBinding", - multiElement.startBinding, - startElement, - elementsMap, - ); - const avoidancePoint = localPoint - ? LinearElementEditor.getPointGlobalCoordinates( - multiElement, - localPoint, - elementsMap, - ) - : null; - if (avoidancePoint && !pointsEqual(startPoint, avoidancePoint)) { - const point = LinearElementEditor.pointFromAbsoluteCoords( - multiElement, - avoidancePoint, - elementsMap, - ); - - LinearElementEditor.movePoints( - multiElement, - this.scene, - new Map([[0, { point }]]), - ); - } - } - } - // in this path, we're mutating multiElement to reflect // how it will be after adding pointer position as the next point // trigger update here so that new element canvas renders again to reflect this