From 302664e500c7f2ee44a1f107d8f4680c0254305b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rk=20Tolm=C3=A1cs?= Date: Sat, 1 Feb 2025 19:21:03 +0100 Subject: [PATCH] fix: Elbow arrow z-index binding (#9067) --- .../excalidraw/actions/actionProperties.tsx | 4 ++ packages/excalidraw/components/App.tsx | 24 +++++-- packages/excalidraw/element/binding.ts | 72 ++++++++++++++++++- packages/excalidraw/element/elbowArrow.ts | 42 ++++++++++- .../excalidraw/element/linearElementEditor.ts | 3 + packages/excalidraw/element/mutateElement.ts | 9 ++- packages/excalidraw/scene/comparisons.ts | 33 +++++---- packages/excalidraw/utils.ts | 9 ++- 8 files changed, 165 insertions(+), 31 deletions(-) diff --git a/packages/excalidraw/actions/actionProperties.tsx b/packages/excalidraw/actions/actionProperties.tsx index 3e0768b265..660d7e6794 100644 --- a/packages/excalidraw/actions/actionProperties.tsx +++ b/packages/excalidraw/actions/actionProperties.tsx @@ -1605,6 +1605,8 @@ export const actionChangeArrowType = register({ elements, elementsMap, appState.zoom, + false, + true, ); const endHoveredElement = !newElement.endBinding && @@ -1613,6 +1615,8 @@ export const actionChangeArrowType = register({ elements, elementsMap, appState.zoom, + false, + true, ); const startElement = startHoveredElement ? startHoveredElement diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index f16f886574..dc81f9181e 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -5770,7 +5770,10 @@ class App extends React.Component { }); } if (editingLinearElement?.lastUncommittedPoint != null) { - this.maybeSuggestBindingAtCursor(scenePointer); + this.maybeSuggestBindingAtCursor( + scenePointer, + editingLinearElement.elbowed, + ); } else { // causes stack overflow if not sync flushSync(() => { @@ -5790,7 +5793,7 @@ class App extends React.Component { this.state.startBoundElement, ); } else { - this.maybeSuggestBindingAtCursor(scenePointer); + this.maybeSuggestBindingAtCursor(scenePointer, false); } } @@ -7728,6 +7731,7 @@ class App extends React.Component { this.scene.getNonDeletedElementsMap(), this.state.zoom, isElbowArrow(element), + isElbowArrow(element), ); this.scene.insertElement(element); @@ -10005,15 +10009,20 @@ class App extends React.Component { } }; - private maybeSuggestBindingAtCursor = (pointerCoords: { - x: number; - y: number; - }): void => { + private maybeSuggestBindingAtCursor = ( + pointerCoords: { + x: number; + y: number; + }, + considerAll: boolean, + ): void => { const hoveredBindableElement = getHoveredElementForBinding( pointerCoords, this.scene.getNonDeletedElements(), this.scene.getNonDeletedElementsMap(), this.state.zoom, + false, + considerAll, ); this.setState({ suggestedBindings: @@ -10043,7 +10052,8 @@ class App extends React.Component { this.scene.getNonDeletedElements(), this.scene.getNonDeletedElementsMap(), this.state.zoom, - isArrowElement(linearElement) && isElbowArrow(linearElement), + isElbowArrow(linearElement), + isElbowArrow(linearElement), ); if ( hoveredBindableElement != null && diff --git a/packages/excalidraw/element/binding.ts b/packages/excalidraw/element/binding.ts index 615a484a47..5b45d982b1 100644 --- a/packages/excalidraw/element/binding.ts +++ b/packages/excalidraw/element/binding.ts @@ -49,7 +49,11 @@ import type { ElementUpdate } from "./mutateElement"; import { mutateElement } from "./mutateElement"; import type Scene from "../scene/Scene"; import { LinearElementEditor } from "./linearElementEditor"; -import { arrayToMap, tupleToCoors } from "../utils"; +import { + arrayToMap, + isBindingFallthroughEnabled, + tupleToCoors, +} from "../utils"; import { KEYS } from "../keys"; import { getBoundTextElement, handleBindTextResize } from "./textElement"; import { aabbForElement, getElementShape, pointInsideBounds } from "../shapes"; @@ -75,6 +79,7 @@ import { clamp, } from "../../math"; import { segmentIntersectRectangleElement } from "../../utils/geometry/shape"; +import { getElementsAtPosition } from "../scene/comparisons"; export type SuggestedBinding = | NonDeleted @@ -425,7 +430,8 @@ export const maybeBindLinearElement = ( elements, elementsMap, appState.zoom, - isElbowArrow(linearElement) && isElbowArrow(linearElement), + isElbowArrow(linearElement), + isElbowArrow(linearElement), ); if (hoveredElement !== null) { @@ -558,7 +564,64 @@ export const getHoveredElementForBinding = ( elementsMap: NonDeletedSceneElementsMap, zoom?: AppState["zoom"], fullShape?: boolean, + considerAllElements?: boolean, ): NonDeleted | null => { + if (considerAllElements) { + let cullRest = false; + const candidateElements = getElementsAtPosition( + elements, + (element) => + isBindableElement(element, false) && + bindingBorderTest( + element, + pointerCoords, + elementsMap, + zoom, + (fullShape || + !isBindingFallthroughEnabled( + element as ExcalidrawBindableElement, + )) && + // disable fullshape snapping for frame elements so we + // can bind to frame children + !isFrameLikeElement(element), + ), + ).filter((element) => { + if (cullRest) { + return false; + } + + if (!isBindingFallthroughEnabled(element as ExcalidrawBindableElement)) { + cullRest = true; + } + + return true; + }) as NonDeleted[] | null; + + // Return early if there are no candidates or just one candidate + if (!candidateElements || candidateElements.length === 0) { + return null; + } + + if (candidateElements.length === 1) { + return candidateElements[0] as NonDeleted; + } + + // Prefer the shape with the border being tested (if any) + const borderTestElements = candidateElements.filter((element) => + bindingBorderTest(element, pointerCoords, elementsMap, zoom, false), + ); + if (borderTestElements.length === 1) { + return borderTestElements[0]; + } + + // Prefer smaller shapes + return candidateElements + .sort( + (a, b) => b.width ** 2 + b.height ** 2 - (a.width ** 2 + a.height ** 2), + ) + .pop() as NonDeleted; + } + const hoveredElement = getElementAtPosition( elements, (element) => @@ -570,7 +633,8 @@ export const getHoveredElementForBinding = ( zoom, // disable fullshape snapping for frame elements so we // can bind to frame children - fullShape && !isFrameLikeElement(element), + (fullShape || !isBindingFallthroughEnabled(element)) && + !isFrameLikeElement(element), ), ); @@ -1220,6 +1284,8 @@ const getElligibleElementForBindingElement = ( elements, elementsMap, zoom, + isElbowArrow(linearElement), + isElbowArrow(linearElement), ); }; diff --git a/packages/excalidraw/element/elbowArrow.ts b/packages/excalidraw/element/elbowArrow.ts index 921e735185..498e59d71a 100644 --- a/packages/excalidraw/element/elbowArrow.ts +++ b/packages/excalidraw/element/elbowArrow.ts @@ -20,11 +20,11 @@ import { bindPointToSnapToElementOutline, distanceToBindableElement, avoidRectangularCorner, - getHoveredElementForBinding, FIXED_BINDING_DISTANCE, getHeadingForElbowArrowSnap, getGlobalFixedPointForBindableElement, snapToMid, + getHoveredElementForBinding, } from "./binding"; import type { Bounds } from "./bounds"; import type { Heading } from "./heading"; @@ -872,6 +872,8 @@ export const updateElbowArrowPoints = ( updates: { points?: readonly LocalPoint[]; fixedSegments?: FixedSegment[] | null; + startBinding?: FixedPointBinding | null; + endBinding?: FixedPointBinding | null; }, options?: { isDragging?: boolean; @@ -953,17 +955,48 @@ export const updateElbowArrowPoints = ( hoveredStartElement, hoveredEndElement, ...rest - } = getElbowArrowData(arrow, elementsMap, updatedPoints, options); + } = getElbowArrowData( + { + x: arrow.x, + y: arrow.y, + startBinding: + typeof updates.startBinding !== "undefined" + ? updates.startBinding + : arrow.startBinding, + endBinding: + typeof updates.endBinding !== "undefined" + ? updates.endBinding + : arrow.endBinding, + startArrowhead: arrow.startArrowhead, + endArrowhead: arrow.endArrowhead, + }, + elementsMap, + updatedPoints, + options, + ); const fixedSegments = updates.fixedSegments ?? arrow.fixedSegments ?? []; //// // 1. Renormalize the arrow //// - if (!updates.points && !updates.fixedSegments) { + if ( + !updates.points && + !updates.fixedSegments && + !updates.startBinding && + !updates.endBinding + ) { return handleSegmentRenormalization(arrow, elementsMap); } + // Short circuit on no-op to avoid huge performance hit + if ( + updates.startBinding === arrow.startBinding && + updates.endBinding === arrow.endBinding + ) { + return {}; + } + //// // 2. Just normal elbow arrow things //// @@ -1010,6 +1043,7 @@ export const updateElbowArrowPoints = ( //// // 5. Handle resize + //// if (updates.points && updates.fixedSegments) { return updates; } @@ -2120,6 +2154,7 @@ const getHoveredElements = ( nonDeletedSceneElementsMap, zoom, true, + true, ), getHoveredElementForBinding( tupleToCoors(origEndGlobalPoint), @@ -2127,6 +2162,7 @@ const getHoveredElements = ( nonDeletedSceneElementsMap, zoom, true, + true, ), ]; }; diff --git a/packages/excalidraw/element/linearElementEditor.ts b/packages/excalidraw/element/linearElementEditor.ts index 99b369a766..380313067e 100644 --- a/packages/excalidraw/element/linearElementEditor.ts +++ b/packages/excalidraw/element/linearElementEditor.ts @@ -444,6 +444,8 @@ export class LinearElementEditor { elements, elementsMap, appState.zoom, + isElbowArrow(element), + isElbowArrow(element), ) : null; @@ -796,6 +798,7 @@ export class LinearElementEditor { elements, elementsMap, app.state.zoom, + linearElementEditor.elbowed, ), }; diff --git a/packages/excalidraw/element/mutateElement.ts b/packages/excalidraw/element/mutateElement.ts index d5fcc1ff35..2f2a5d47f9 100644 --- a/packages/excalidraw/element/mutateElement.ts +++ b/packages/excalidraw/element/mutateElement.ts @@ -33,13 +33,16 @@ export const mutateElement = >( // casting to any because can't use `in` operator // (see https://github.com/microsoft/TypeScript/issues/21732) - const { points, fixedSegments, fileId } = updates as any; + const { points, fixedSegments, fileId, startBinding, endBinding } = + updates as any; if ( isElbowArrow(element) && (Object.keys(updates).length === 0 || // normalization case typeof points !== "undefined" || // repositioning - typeof fixedSegments !== "undefined") // segment fixing + typeof fixedSegments !== "undefined" || // segment fixing + typeof startBinding !== "undefined" || + typeof endBinding !== "undefined") // manual binding to element ) { const elementsMap = toBrandedType( Scene.getScene(element)?.getNonDeletedElementsMap() ?? new Map(), @@ -58,6 +61,8 @@ export const mutateElement = >( { fixedSegments, points, + startBinding, + endBinding, }, { isDragging: options?.isDragging, diff --git a/packages/excalidraw/scene/comparisons.ts b/packages/excalidraw/scene/comparisons.ts index 629fb371eb..47561fdf9f 100644 --- a/packages/excalidraw/scene/comparisons.ts +++ b/packages/excalidraw/scene/comparisons.ts @@ -75,20 +75,23 @@ export const getElementsAtPosition = ( isAtPositionFn: (element: NonDeletedExcalidrawElement) => boolean, ) => { const iframeLikes: ExcalidrawIframeElement[] = []; - // The parameter elements comes ordered from lower z-index to higher. - // We want to preserve that order on the returned array. - // Exception being embeddables which should be on top of everything else in - // terms of hit testing. - const elsAtPos = elements.filter((element) => { - const hit = !element.isDeleted && isAtPositionFn(element); - if (hit) { - if (isIframeElement(element)) { - iframeLikes.push(element); - return false; - } - return true; + const elementsAtPosition: NonDeletedExcalidrawElement[] = []; + // We need to to hit testing from front (end of the array) to back (beginning of the array) + // because array is ordered from lower z-index to highest and we want element z-index + // with higher z-index + for (let index = elements.length - 1; index >= 0; --index) { + const element = elements[index]; + if (element.isDeleted) { + continue; } - return false; - }); - return elsAtPos.concat(iframeLikes); + if (isIframeElement(element)) { + iframeLikes.push(element); + continue; + } + if (isAtPositionFn(element)) { + elementsAtPosition.push(element); + } + } + + return elementsAtPosition.concat(iframeLikes); }; diff --git a/packages/excalidraw/utils.ts b/packages/excalidraw/utils.ts index e30c67ff68..e4431ddd1f 100644 --- a/packages/excalidraw/utils.ts +++ b/packages/excalidraw/utils.ts @@ -9,7 +9,11 @@ import { isDarwin, WINDOWS_EMOJI_FALLBACK_FONT, } from "./constants"; -import type { FontFamilyValues, FontString } from "./element/types"; +import type { + ExcalidrawBindableElement, + FontFamilyValues, + FontString, +} from "./element/types"; import type { ActiveTool, AppState, @@ -543,6 +547,9 @@ export const isTransparent = (color: string) => { ); }; +export const isBindingFallthroughEnabled = (el: ExcalidrawBindableElement) => + el.fillStyle !== "solid" || isTransparent(el.backgroundColor); + export type ResolvablePromise = Promise & { resolve: [T] extends [undefined] ? (value?: MaybePromise>) => void