From 1e3399eac865e13a96d615862524f8f5c33d45f2 Mon Sep 17 00:00:00 2001 From: Ryan Di Date: Mon, 23 Dec 2024 05:55:50 +0800 Subject: [PATCH] fix: make arrow binding area adapt to zoom levels (#8927) * make binding area adapt to zoom * revert stroke color * normalize binding gap * reduce normalized gap --- .../actions/actionDeleteSelected.tsx | 1 + packages/excalidraw/actions/actionFlip.ts | 1 + .../excalidraw/actions/actionProperties.tsx | 2 + packages/excalidraw/components/App.tsx | 16 +++ packages/excalidraw/components/Stats/utils.ts | 2 + .../data/__snapshots__/transform.test.ts.snap | 58 ++++---- packages/excalidraw/data/transform.test.ts | 2 +- packages/excalidraw/element/binding.ts | 94 +++++++++++-- .../excalidraw/element/linearElementEditor.ts | 10 ++ packages/excalidraw/element/routing.ts | 13 +- .../excalidraw/renderer/interactiveScene.ts | 21 ++- .../tests/__snapshots__/history.test.tsx.snap | 128 ++++++++++-------- .../tests/__snapshots__/move.test.tsx.snap | 6 +- packages/excalidraw/tests/history.test.tsx | 12 +- 14 files changed, 247 insertions(+), 119 deletions(-) diff --git a/packages/excalidraw/actions/actionDeleteSelected.tsx b/packages/excalidraw/actions/actionDeleteSelected.tsx index b354c4440..49292c10e 100644 --- a/packages/excalidraw/actions/actionDeleteSelected.tsx +++ b/packages/excalidraw/actions/actionDeleteSelected.tsx @@ -161,6 +161,7 @@ export const actionDeleteSelected = register({ element, selectedPointsIndices, elementsMap, + appState.zoom, ); return { diff --git a/packages/excalidraw/actions/actionFlip.ts b/packages/excalidraw/actions/actionFlip.ts index 6b75b8fac..a02d6bcf8 100644 --- a/packages/excalidraw/actions/actionFlip.ts +++ b/packages/excalidraw/actions/actionFlip.ts @@ -153,6 +153,7 @@ const flipElements = ( app.scene, isBindingEnabled(appState), [], + appState.zoom, ); // --------------------------------------------------------------------------- diff --git a/packages/excalidraw/actions/actionProperties.tsx b/packages/excalidraw/actions/actionProperties.tsx index 72ff8896b..7ff078673 100644 --- a/packages/excalidraw/actions/actionProperties.tsx +++ b/packages/excalidraw/actions/actionProperties.tsx @@ -1591,6 +1591,7 @@ export const actionChangeArrowType = register({ tupleToCoors(startGlobalPoint), elements, elementsMap, + appState.zoom, true, ); const endHoveredElement = @@ -1599,6 +1600,7 @@ export const actionChangeArrowType = register({ tupleToCoors(endGlobalPoint), elements, elementsMap, + appState.zoom, true, ); const startElement = startHoveredElement diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 6e09b4a07..1e02f512f 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -3215,6 +3215,10 @@ class App extends React.Component { ), ), [el.points[0], el.points[el.points.length - 1]], + undefined, + { + zoom: this.state.zoom, + }, ), }; } @@ -4372,6 +4376,7 @@ class App extends React.Component { updateBoundElements(element, this.scene.getNonDeletedElementsMap(), { simultaneouslyUpdated: selectedElements, + zoom: this.state.zoom, }); }); @@ -4381,6 +4386,7 @@ class App extends React.Component { (element) => element.id !== elbowArrow?.id || step !== 0, ), this.scene.getNonDeletedElementsMap(), + this.state.zoom, ), }); @@ -4596,6 +4602,7 @@ class App extends React.Component { this.scene, isBindingEnabled(this.state), this.state.selectedLinearElement?.selectedPointsIndices ?? [], + this.state.zoom, ); this.setState({ suggestedBindings: [] }); } @@ -5854,6 +5861,7 @@ class App extends React.Component { { isDragging: true, informMutation: false, + zoom: this.state.zoom, }, ); } else { @@ -7401,6 +7409,7 @@ class App extends React.Component { pointerDownState.origin, this.scene.getNonDeletedElements(), this.scene.getNonDeletedElementsMap(), + this.state.zoom, ); this.setState({ @@ -7698,6 +7707,7 @@ class App extends React.Component { pointerDownState.origin, this.scene.getNonDeletedElements(), this.scene.getNonDeletedElementsMap(), + this.state.zoom, isElbowArrow(element), ); @@ -8276,6 +8286,7 @@ class App extends React.Component { suggestedBindings: getSuggestedBindingsForArrows( selectedElements, this.scene.getNonDeletedElementsMap(), + this.state.zoom, ), }); } @@ -8444,6 +8455,7 @@ class App extends React.Component { { isDragging: true, informMutation: false, + zoom: this.state.zoom, }, ); } else if (points.length === 2) { @@ -9408,6 +9420,7 @@ class App extends React.Component { this.scene, isBindingEnabled(this.state), this.state.selectedLinearElement?.selectedPointsIndices ?? [], + this.state.zoom, ); } @@ -9900,6 +9913,7 @@ class App extends React.Component { pointerCoords, this.scene.getNonDeletedElements(), this.scene.getNonDeletedElementsMap(), + this.state.zoom, ); this.setState({ suggestedBindings: @@ -9928,6 +9942,7 @@ class App extends React.Component { coords, this.scene.getNonDeletedElements(), this.scene.getNonDeletedElementsMap(), + this.state.zoom, isArrowElement(linearElement) && isElbowArrow(linearElement), ); if ( @@ -10569,6 +10584,7 @@ class App extends React.Component { const suggestedBindings = getSuggestedBindingsForArrows( selectedElements, this.scene.getNonDeletedElementsMap(), + this.state.zoom, ); const elementsToHighlight = new Set(); diff --git a/packages/excalidraw/components/Stats/utils.ts b/packages/excalidraw/components/Stats/utils.ts index 3fcbc11c7..d38158d0a 100644 --- a/packages/excalidraw/components/Stats/utils.ts +++ b/packages/excalidraw/components/Stats/utils.ts @@ -300,6 +300,7 @@ export const updateBindings = ( options?: { simultaneouslyUpdated?: readonly ExcalidrawElement[]; newSize?: { width: number; height: number }; + zoom?: AppState["zoom"]; }, ) => { if (isLinearElement(latestElement)) { @@ -310,6 +311,7 @@ export const updateBindings = ( scene, true, [], + options?.zoom, ); } else { updateBoundElements(latestElement, elementsMap, options); diff --git a/packages/excalidraw/data/__snapshots__/transform.test.ts.snap b/packages/excalidraw/data/__snapshots__/transform.test.ts.snap index 2ab3ee1fe..2b56d9b27 100644 --- a/packages/excalidraw/data/__snapshots__/transform.test.ts.snap +++ b/packages/excalidraw/data/__snapshots__/transform.test.ts.snap @@ -95,7 +95,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s "fillStyle": "solid", "frameId": null, "groupIds": [], - "height": 35, + "height": 33.519031369643244, "id": Any, "index": "a2", "isDeleted": false, @@ -109,8 +109,8 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s 0.5, ], [ - 394.5, - 34.5, + 382.47606040672997, + 34.019031369643244, ], ], "roughness": 1, @@ -128,9 +128,9 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 4, + "version": 7, "versionNonce": Any, - "width": 395, + "width": 381.97606040672997, "x": 247, "y": 420, } @@ -167,7 +167,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s 0, ], [ - 399.5, + 389.5, 0, ], ], @@ -186,10 +186,10 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 4, + "version": 6, "versionNonce": Any, - "width": 400, - "x": 227, + "width": 390, + "x": 237, "y": 450, } `; @@ -319,7 +319,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t "verticalAlign": "top", "width": 100, "x": 560, - "y": 226.5, + "y": 236.95454545454544, } `; @@ -339,13 +339,13 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t "endBinding": { "elementId": "text-2", "fixedPoint": null, - "focus": 0, - "gap": 205, + "focus": 1.625925925925924, + "gap": 14, }, "fillStyle": "solid", "frameId": null, "groupIds": [], - "height": 0, + "height": 18.278619528619487, "id": Any, "index": "a2", "isDeleted": false, @@ -356,11 +356,11 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t "points": [ [ 0.5, - 0, + -0.5, ], [ - 99.5, - 0, + 357.2037037037038, + -17.778619528619487, ], ], "roughness": 1, @@ -378,11 +378,11 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 4, + "version": 6, "versionNonce": Any, - "width": 100, - "x": 255, - "y": 239, + "width": 357.7037037037038, + "x": 171, + "y": 249.45454545454544, } `; @@ -482,7 +482,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to shapes whe "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 4, + "version": 6, "versionNonce": Any, "width": 100, "x": 255, @@ -660,7 +660,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to text when "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 4, + "version": 6, "versionNonce": Any, "width": 100, "x": 255, @@ -1505,7 +1505,7 @@ exports[`Test Transform > should transform the elements correctly when linear el 0, ], [ - 272.485, + 270.98528125, 0, ], ], @@ -1526,10 +1526,10 @@ exports[`Test Transform > should transform the elements correctly when linear el "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 4, + "version": 7, "versionNonce": Any, - "width": 272.985, - "x": 111.262, + "width": 270.48528125, + "x": 112.76171875, "y": 57, } `; @@ -1587,11 +1587,11 @@ exports[`Test Transform > should transform the elements correctly when linear el "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 4, + "version": 6, "versionNonce": Any, "width": 0, - "x": 77.017, - "y": 79, + "x": 83.015625, + "y": 81.5, } `; diff --git a/packages/excalidraw/data/transform.test.ts b/packages/excalidraw/data/transform.test.ts index 3fecf957b..ce0cf89f7 100644 --- a/packages/excalidraw/data/transform.test.ts +++ b/packages/excalidraw/data/transform.test.ts @@ -779,7 +779,7 @@ describe("Test Transform", () => { elementId: "rect-1", fixedPoint: null, focus: 0, - gap: 205, + gap: 14, }); expect(rect.boundElements).toStrictEqual([ { diff --git a/packages/excalidraw/element/binding.ts b/packages/excalidraw/element/binding.ts index 3c4869fe4..aa6cff249 100644 --- a/packages/excalidraw/element/binding.ts +++ b/packages/excalidraw/element/binding.ts @@ -40,7 +40,6 @@ import { isBoundToContainer, isElbowArrow, isFixedPointBinding, - isFrameLikeElement, isLinearElement, isRectangularElement, isTextElement, @@ -97,6 +96,8 @@ export const isBindingEnabled = (appState: AppState): boolean => { }; export const FIXED_BINDING_DISTANCE = 5; +export const BINDING_HIGHLIGHT_THICKNESS = 10; +export const BINDING_HIGHLIGHT_OFFSET = 4; const getNonDeletedElements = ( scene: Scene, @@ -213,6 +214,7 @@ const getOriginalBindingIfStillCloseOfLinearElementEdge = ( linearElement: NonDeleted, edge: "start" | "end", elementsMap: NonDeletedSceneElementsMap, + zoom?: AppState["zoom"], ): NonDeleted | null => { const coors = getLinearElementEdgeCoors(linearElement, edge, elementsMap); const elementId = @@ -223,7 +225,7 @@ const getOriginalBindingIfStillCloseOfLinearElementEdge = ( const element = elementsMap.get(elementId); if ( isBindableElement(element) && - bindingBorderTest(element, coors, elementsMap) + bindingBorderTest(element, coors, elementsMap, zoom) ) { return element; } @@ -235,12 +237,14 @@ const getOriginalBindingIfStillCloseOfLinearElementEdge = ( const getOriginalBindingsIfStillCloseToArrowEnds = ( linearElement: NonDeleted, elementsMap: NonDeletedSceneElementsMap, + zoom?: AppState["zoom"], ): (NonDeleted | null)[] => ["start", "end"].map((edge) => getOriginalBindingIfStillCloseOfLinearElementEdge( linearElement, edge as "start" | "end", elementsMap, + zoom, ), ); @@ -250,6 +254,7 @@ const getBindingStrategyForDraggingArrowEndpoints = ( draggingPoints: readonly number[], elementsMap: NonDeletedSceneElementsMap, elements: readonly NonDeletedExcalidrawElement[], + zoom?: AppState["zoom"], ): (NonDeleted | null | "keep")[] => { const startIdx = 0; const endIdx = selectedElement.points.length - 1; @@ -262,6 +267,7 @@ const getBindingStrategyForDraggingArrowEndpoints = ( "start", elementsMap, elements, + zoom, ) : null // If binding is disabled and start is dragged, break all binds : // We have to update the focus and gap of the binding, so let's rebind @@ -270,6 +276,7 @@ const getBindingStrategyForDraggingArrowEndpoints = ( "start", elementsMap, elements, + zoom, ); const end = endDragged ? isBindingEnabled @@ -278,6 +285,7 @@ const getBindingStrategyForDraggingArrowEndpoints = ( "end", elementsMap, elements, + zoom, ) : null // If binding is disabled and end is dragged, break all binds : // We have to update the focus and gap of the binding, so let's rebind @@ -286,6 +294,7 @@ const getBindingStrategyForDraggingArrowEndpoints = ( "end", elementsMap, elements, + zoom, ); return [start, end]; @@ -296,10 +305,12 @@ const getBindingStrategyForDraggingArrowOrJoints = ( elementsMap: NonDeletedSceneElementsMap, elements: readonly NonDeletedExcalidrawElement[], isBindingEnabled: boolean, + zoom?: AppState["zoom"], ): (NonDeleted | null | "keep")[] => { const [startIsClose, endIsClose] = getOriginalBindingsIfStillCloseToArrowEnds( selectedElement, elementsMap, + zoom, ); const start = startIsClose ? isBindingEnabled @@ -308,6 +319,7 @@ const getBindingStrategyForDraggingArrowOrJoints = ( "start", elementsMap, elements, + zoom, ) : null : null; @@ -318,6 +330,7 @@ const getBindingStrategyForDraggingArrowOrJoints = ( "end", elementsMap, elements, + zoom, ) : null : null; @@ -332,6 +345,7 @@ export const bindOrUnbindLinearElements = ( scene: Scene, isBindingEnabled: boolean, draggingPoints: readonly number[] | null, + zoom?: AppState["zoom"], ): void => { selectedElements.forEach((selectedElement) => { const [start, end] = draggingPoints?.length @@ -342,6 +356,7 @@ export const bindOrUnbindLinearElements = ( draggingPoints ?? [], elementsMap, elements, + zoom, ) : // The arrow itself (the shaft) or the inner joins are dragged getBindingStrategyForDraggingArrowOrJoints( @@ -349,6 +364,7 @@ export const bindOrUnbindLinearElements = ( elementsMap, elements, isBindingEnabled, + zoom, ); bindOrUnbindLinearElement(selectedElement, start, end, elementsMap, scene); @@ -358,6 +374,7 @@ export const bindOrUnbindLinearElements = ( export const getSuggestedBindingsForArrows = ( selectedElements: NonDeleted[], elementsMap: NonDeletedSceneElementsMap, + zoom: AppState["zoom"], ): SuggestedBinding[] => { // HOT PATH: Bail out if selected elements list is too large if (selectedElements.length > 50) { @@ -368,7 +385,7 @@ export const getSuggestedBindingsForArrows = ( selectedElements .filter(isLinearElement) .flatMap((element) => - getOriginalBindingsIfStillCloseToArrowEnds(element, elementsMap), + getOriginalBindingsIfStillCloseToArrowEnds(element, elementsMap, zoom), ) .filter( (element): element is NonDeleted => @@ -406,6 +423,7 @@ export const maybeBindLinearElement = ( pointerCoords, elements, elementsMap, + appState.zoom, isElbowArrow(linearElement) && isElbowArrow(linearElement), ); @@ -422,6 +440,26 @@ export const maybeBindLinearElement = ( } }; +const normalizePointBinding = ( + binding: { focus: number; gap: number }, + hoveredElement: ExcalidrawBindableElement, +) => { + let gap = binding.gap; + const maxGap = maxBindingGap( + hoveredElement, + hoveredElement.width, + hoveredElement.height, + ); + + if (gap > maxGap) { + gap = BINDING_HIGHLIGHT_THICKNESS + BINDING_HIGHLIGHT_OFFSET; + } + return { + ...binding, + gap, + }; +}; + export const bindLinearElement = ( linearElement: NonDeleted, hoveredElement: ExcalidrawBindableElement, @@ -433,11 +471,14 @@ export const bindLinearElement = ( } const binding: PointBinding = { elementId: hoveredElement.id, - ...calculateFocusAndGap( - linearElement, + ...normalizePointBinding( + calculateFocusAndGap( + linearElement, + hoveredElement, + startOrEnd, + elementsMap, + ), hoveredElement, - startOrEnd, - elementsMap, ), ...(isElbowArrow(linearElement) ? calculateFixedPointForElbowArrowBinding( @@ -462,6 +503,12 @@ export const bindLinearElement = ( }), }); } + + // update bound elements to make sure the binding tips are in sync with + // the normalized gap from above + if (!isElbowArrow(linearElement)) { + updateBoundElements(hoveredElement, elementsMap); + } }; // Don't bind both ends of a simple segment @@ -514,6 +561,7 @@ export const getHoveredElementForBinding = ( }, elements: readonly NonDeletedExcalidrawElement[], elementsMap: NonDeletedSceneElementsMap, + zoom?: AppState["zoom"], fullShape?: boolean, ): NonDeleted | null => { const hoveredElement = getElementAtPosition( @@ -524,11 +572,13 @@ export const getHoveredElementForBinding = ( element, pointerCoords, elementsMap, + zoom, // disable fullshape snapping for frame elements so we // can bind to frame children - fullShape && !isFrameLikeElement(element), + fullShape, ), ); + return hoveredElement as NonDeleted | null; }; @@ -578,9 +628,11 @@ export const updateBoundElements = ( simultaneouslyUpdated?: readonly ExcalidrawElement[]; newSize?: { width: number; height: number }; changedElements?: Map; + zoom?: AppState["zoom"]; }, ) => { - const { newSize, simultaneouslyUpdated, changedElements } = options ?? {}; + const { newSize, simultaneouslyUpdated, changedElements, zoom } = + options ?? {}; const simultaneouslyUpdatedElementIds = getSimultaneouslyUpdatedElementIds( simultaneouslyUpdated, ); @@ -670,6 +722,7 @@ export const updateBoundElements = ( }, { changedElements, + zoom, }, ); @@ -703,6 +756,7 @@ export const getHeadingForElbowArrowSnap = ( aabb: Bounds | undefined | null, elementsMap: ElementsMap, origPoint: GlobalPoint, + zoom?: AppState["zoom"], ): Heading => { const otherPointHeading = vectorToHeading(vectorFromPoint(otherPoint, p)); @@ -714,6 +768,7 @@ export const getHeadingForElbowArrowSnap = ( origPoint, bindableElement, elementsMap, + zoom, ); if (!distance) { @@ -737,6 +792,7 @@ const getDistanceForBinding = ( point: Readonly, bindableElement: ExcalidrawBindableElement, elementsMap: ElementsMap, + zoom?: AppState["zoom"], ) => { const distance = distanceToBindableElement( bindableElement, @@ -747,6 +803,7 @@ const getDistanceForBinding = ( bindableElement, bindableElement.width, bindableElement.height, + zoom, ); return distance > bindDistance ? null : distance; @@ -1174,11 +1231,13 @@ const getElligibleElementForBindingElement = ( startOrEnd: "start" | "end", elementsMap: NonDeletedSceneElementsMap, elements: readonly NonDeletedExcalidrawElement[], + zoom?: AppState["zoom"], ): NonDeleted | null => { return getHoveredElementForBinding( getLinearElementEdgeCoors(linearElement, startOrEnd, elementsMap), elements, elementsMap, + zoom, ); }; @@ -1341,9 +1400,11 @@ export const bindingBorderTest = ( element: NonDeleted, { x, y }: { x: number; y: number }, elementsMap: NonDeletedSceneElementsMap, + zoom?: AppState["zoom"], fullShape?: boolean, ): boolean => { - const threshold = maxBindingGap(element, element.width, element.height); + const threshold = maxBindingGap(element, element.width, element.height, zoom); + const shape = getElementShape(element, elementsMap); return ( isPointOnShape(pointFrom(x, y), shape, threshold) || @@ -1356,12 +1417,21 @@ export const maxBindingGap = ( element: ExcalidrawElement, elementWidth: number, elementHeight: number, + zoom?: AppState["zoom"], ): number => { + const zoomValue = zoom?.value && zoom.value < 1 ? zoom.value : 1; + // Aligns diamonds with rectangles const shapeRatio = element.type === "diamond" ? 1 / Math.sqrt(2) : 1; const smallerDimension = shapeRatio * Math.min(elementWidth, elementHeight); - // We make the bindable boundary bigger for bigger elements - return Math.max(16, Math.min(0.25 * smallerDimension, 32)); + + return Math.max( + 16, + // bigger bindable boundary for bigger elements + Math.min(0.25 * smallerDimension, 32), + // keep in sync with the zoomed highlight + BINDING_HIGHLIGHT_THICKNESS / zoomValue + BINDING_HIGHLIGHT_OFFSET, + ); }; export const distanceToBindableElement = ( diff --git a/packages/excalidraw/element/linearElementEditor.ts b/packages/excalidraw/element/linearElementEditor.ts index 0d1db7733..ea3644704 100644 --- a/packages/excalidraw/element/linearElementEditor.ts +++ b/packages/excalidraw/element/linearElementEditor.ts @@ -448,6 +448,7 @@ export class LinearElementEditor { ), elements, elementsMap, + appState.zoom, ) : null; @@ -787,6 +788,7 @@ export class LinearElementEditor { scenePointer, elements, elementsMap, + app.state.zoom, ), }; @@ -911,6 +913,7 @@ export class LinearElementEditor { element, [points.length - 1], elementsMap, + app.state.zoom, ); } return { @@ -964,6 +967,7 @@ export class LinearElementEditor { element, [{ point: newPoint }], elementsMap, + app.state.zoom, ); } return { @@ -1218,6 +1222,7 @@ export class LinearElementEditor { element: NonDeleted, pointIndices: readonly number[], elementsMap: NonDeletedSceneElementsMap | SceneElementsMap, + zoom: AppState["zoom"], ) { let offsetX = 0; let offsetY = 0; @@ -1260,6 +1265,7 @@ export class LinearElementEditor { element: NonDeleted, targetPoints: { point: LocalPoint }[], elementsMap: NonDeletedSceneElementsMap | SceneElementsMap, + zoom: AppState["zoom"], ) { const offsetX = 0; const offsetY = 0; @@ -1285,6 +1291,7 @@ export class LinearElementEditor { options?: { changedElements?: Map; isDragging?: boolean; + zoom?: AppState["zoom"]; }, ) { const { points } = element; @@ -1337,6 +1344,7 @@ export class LinearElementEditor { false, ), changedElements: options?.changedElements, + zoom: options?.zoom, }, ); } @@ -1451,6 +1459,7 @@ export class LinearElementEditor { options?: { changedElements?: Map; isDragging?: boolean; + zoom?: AppState["zoom"]; }, ) { if (isElbowArrow(element)) { @@ -1487,6 +1496,7 @@ export class LinearElementEditor { bindings, { isDragging: options?.isDragging, + zoom: options?.zoom, }, ); } else { diff --git a/packages/excalidraw/element/routing.ts b/packages/excalidraw/element/routing.ts index c8b1c2d43..acf4f849d 100644 --- a/packages/excalidraw/element/routing.ts +++ b/packages/excalidraw/element/routing.ts @@ -14,6 +14,7 @@ import { import BinaryHeap from "../binaryheap"; import { getSizeFromPoints } from "../points"; import { aabbForElement, pointInsideBounds } from "../shapes"; +import type { AppState } from "../types"; import { isAnyTrue, toBrandedType, tupleToCoors } from "../utils"; import { bindPointToSnapToElementOutline, @@ -79,6 +80,7 @@ export const mutateElbowArrow = ( options?: { isDragging?: boolean; informMutation?: boolean; + zoom?: AppState["zoom"]; }, ) => { const update = updateElbowArrow( @@ -112,6 +114,7 @@ export const updateElbowArrow = ( isDragging?: boolean; disableBinding?: boolean; informMutation?: boolean; + zoom?: AppState["zoom"]; }, ): ElementUpdate | null => { const origStartGlobalPoint: GlobalPoint = pointTranslate( @@ -136,7 +139,12 @@ export const updateElbowArrow = ( arrow.endBinding && getBindableElementForId(arrow.endBinding.elementId, elementsMap); const [hoveredStartElement, hoveredEndElement] = options?.isDragging - ? getHoveredElements(origStartGlobalPoint, origEndGlobalPoint, elementsMap) + ? getHoveredElements( + origStartGlobalPoint, + origEndGlobalPoint, + elementsMap, + options?.zoom, + ) : [startElement, endElement]; const startGlobalPoint = getGlobalPoint( arrow.startBinding?.fixedPoint, @@ -1072,6 +1080,7 @@ const getHoveredElements = ( origStartGlobalPoint: GlobalPoint, origEndGlobalPoint: GlobalPoint, elementsMap: NonDeletedSceneElementsMap | SceneElementsMap, + zoom?: AppState["zoom"], ) => { // TODO: Might be a performance bottleneck and the Map type // remembers the insertion order anyway... @@ -1084,12 +1093,14 @@ const getHoveredElements = ( tupleToCoors(origStartGlobalPoint), elements, nonDeletedSceneElementsMap, + zoom, true, ), getHoveredElementForBinding( tupleToCoors(origEndGlobalPoint), elements, nonDeletedSceneElementsMap, + zoom, true, ), ]; diff --git a/packages/excalidraw/renderer/interactiveScene.ts b/packages/excalidraw/renderer/interactiveScene.ts index f8ca0e366..b474a0d80 100644 --- a/packages/excalidraw/renderer/interactiveScene.ts +++ b/packages/excalidraw/renderer/interactiveScene.ts @@ -43,7 +43,11 @@ import type { SuggestedBinding, SuggestedPointBinding, } from "../element/binding"; -import { maxBindingGap } from "../element/binding"; +import { + BINDING_HIGHLIGHT_OFFSET, + BINDING_HIGHLIGHT_THICKNESS, + maxBindingGap, +} from "../element/binding"; import { LinearElementEditor } from "../element/linearElementEditor"; import { bootstrapCanvas, @@ -217,17 +221,18 @@ const renderBindingHighlightForBindableElement = ( context: CanvasRenderingContext2D, element: ExcalidrawBindableElement, elementsMap: ElementsMap, + zoom: InteractiveCanvasAppState["zoom"], ) => { const [x1, y1, x2, y2] = getElementAbsoluteCoords(element, elementsMap); const width = x2 - x1; const height = y2 - y1; - const thickness = 10; - // So that we don't overlap the element itself - const strokeOffset = 4; context.strokeStyle = "rgba(0,0,0,.05)"; - context.lineWidth = thickness - strokeOffset; - const padding = strokeOffset / 2 + thickness / 2; + // When zooming out, make line width greater for visibility + const zoomValue = zoom.value < 1 ? zoom.value : 1; + context.lineWidth = BINDING_HIGHLIGHT_THICKNESS / zoomValue; + // To ensure the binding highlight doesn't overlap the element itself + const padding = context.lineWidth / 2 + BINDING_HIGHLIGHT_OFFSET; const radius = getCornerRadius( Math.min(element.width, element.height), @@ -285,6 +290,7 @@ const renderBindingHighlightForSuggestedPointBinding = ( context: CanvasRenderingContext2D, suggestedBinding: SuggestedPointBinding, elementsMap: ElementsMap, + zoom: InteractiveCanvasAppState["zoom"], ) => { const [element, startOrEnd, bindableElement] = suggestedBinding; @@ -292,6 +298,7 @@ const renderBindingHighlightForSuggestedPointBinding = ( bindableElement, bindableElement.width, bindableElement.height, + zoom, ); context.strokeStyle = "rgba(0,0,0,0)"; @@ -390,7 +397,7 @@ const renderBindingHighlight = ( context.save(); context.translate(appState.scrollX, appState.scrollY); - renderHighlight(context, suggestedBinding as any, elementsMap); + renderHighlight(context, suggestedBinding as any, elementsMap, appState.zoom); context.restore(); }; diff --git a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap index 1d1466547..3a701fc5f 100644 --- a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap @@ -197,7 +197,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "fillStyle": "solid", "frameId": null, "groupIds": [], - "height": 99, + "height": 125, "id": "id166", "index": "a2", "isDeleted": false, @@ -211,8 +211,8 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl 0, ], [ - "98.20800", - 99, + 125, + 125, ], ], "roughness": 1, @@ -226,9 +226,9 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 40, - "width": "98.20800", - "x": 1, + "version": 47, + "width": 125, + "x": 0, "y": 0, } `; @@ -298,7 +298,7 @@ History { "focus": "0.00990", "gap": 1, }, - "height": "0.98017", + "height": "0.98000", "points": [ [ 0, @@ -306,7 +306,7 @@ History { ], [ 98, - "-0.98017", + "-0.98000", ], ], "startBinding": { @@ -320,10 +320,10 @@ History { "endBinding": { "elementId": "id165", "fixedPoint": null, - "focus": "-0.02000", + "focus": "-0.02040", "gap": 1, }, - "height": "0.00169", + "height": "0.02000", "points": [ [ 0, @@ -331,13 +331,13 @@ History { ], [ 98, - "0.00169", + "0.02000", ], ], "startBinding": { "elementId": "id164", "fixedPoint": null, - "focus": "0.02000", + "focus": "0.01959", "gap": 1, }, }, @@ -393,18 +393,20 @@ History { "focus": 0, "gap": 1, }, - "height": 99, + "height": 125, "points": [ [ 0, 0, ], [ - "98.20800", - 99, + 125, + 125, ], ], "startBinding": null, + "width": 125, + "x": 0, "y": 0, }, "inserted": { @@ -414,7 +416,7 @@ History { "focus": "0.00990", "gap": 1, }, - "height": "0.98161", + "height": "0.98000", "points": [ [ 0, @@ -422,7 +424,7 @@ History { ], [ 98, - "-0.98161", + "-0.98000", ], ], "startBinding": { @@ -431,7 +433,9 @@ History { "focus": "0.02970", "gap": 1, }, - "y": "0.99245", + "width": 98, + "x": 1, + "y": "0.99000", }, }, "id169" => Delta { @@ -823,9 +827,9 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 30, - "width": 0, - "x": 200, + "version": 37, + "width": 100, + "x": 150, "y": 0, } `; @@ -862,6 +866,8 @@ History { 0, ], ], + "width": 0, + "x": 149, }, "inserted": { "points": [ @@ -870,10 +876,12 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], + "width": "98.00000", + "x": "1.00000", }, }, }, @@ -930,6 +938,8 @@ History { ], ], "startBinding": null, + "width": 100, + "x": 150, }, "inserted": { "endBinding": { @@ -954,6 +964,8 @@ History { "focus": 0, "gap": 1, }, + "width": 0, + "x": 149, }, }, }, @@ -2363,9 +2375,9 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 10, + "version": 12, "width": 498, - "x": 1, + "x": "1.00000", "y": 0, } `; @@ -2504,7 +2516,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -2523,8 +2535,8 @@ History { "strokeStyle": "solid", "strokeWidth": 2, "type": "arrow", - "width": 100, - "x": 0, + "width": "98.00000", + "x": 1, "y": 0, }, "inserted": { @@ -15167,9 +15179,9 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 10, + "version": 12, "width": "98.00000", - "x": 1, + "x": "1.00000", "y": 0, } `; @@ -15208,7 +15220,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -15221,7 +15233,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -15517,7 +15529,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -15536,8 +15548,8 @@ History { "strokeStyle": "solid", "strokeWidth": 2, "type": "arrow", - "width": 100, - "x": 0, + "width": "98.00000", + "x": 1, "y": 0, }, "inserted": { @@ -15866,9 +15878,9 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 10, + "version": 12, "width": "98.00000", - "x": 1, + "x": "1.00000", "y": 0, } `; @@ -16140,7 +16152,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -16159,8 +16171,8 @@ History { "strokeStyle": "solid", "strokeWidth": 2, "type": "arrow", - "width": 100, - "x": 0, + "width": "98.00000", + "x": 1, "y": 0, }, "inserted": { @@ -16489,9 +16501,9 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 10, + "version": 12, "width": "98.00000", - "x": 1, + "x": "1.00000", "y": 0, } `; @@ -16763,7 +16775,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -16782,8 +16794,8 @@ History { "strokeStyle": "solid", "strokeWidth": 2, "type": "arrow", - "width": 100, - "x": 0, + "width": "98.00000", + "x": 1, "y": 0, }, "inserted": { @@ -17110,9 +17122,9 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 10, + "version": 12, "width": "98.00000", - "x": 1, + "x": "1.00000", "y": 0, } `; @@ -17168,7 +17180,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -17186,7 +17198,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -17455,7 +17467,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -17474,8 +17486,8 @@ History { "strokeStyle": "solid", "strokeWidth": 2, "type": "arrow", - "width": 100, - "x": 0, + "width": "98.00000", + "x": 1, "y": 0, }, "inserted": { @@ -17828,9 +17840,9 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 11, + "version": 13, "width": "98.00000", - "x": 1, + "x": "1.00000", "y": 0, } `; @@ -17901,7 +17913,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -17920,7 +17932,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -18189,7 +18201,7 @@ History { 0, ], [ - 100, + "98.00000", 0, ], ], @@ -18208,8 +18220,8 @@ History { "strokeStyle": "solid", "strokeWidth": 2, "type": "arrow", - "width": 100, - "x": 0, + "width": "98.00000", + "x": 1, "y": 0, }, "inserted": { diff --git a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap index 654eccfea..fdd294a30 100644 --- a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap @@ -173,7 +173,7 @@ exports[`move element > rectangles with binding arrow 6`] = ` "type": "rectangle", "updated": 1, "version": 7, - "versionNonce": 745419401, + "versionNonce": 2066753033, "width": 300, "x": 201, "y": 2, @@ -232,8 +232,8 @@ exports[`move element > rectangles with binding arrow 7`] = ` "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 11, - "versionNonce": 1996028265, + "version": 15, + "versionNonce": 271613161, "width": 81, "x": 110, "y": 50, diff --git a/packages/excalidraw/tests/history.test.tsx b/packages/excalidraw/tests/history.test.tsx index 6a5db9753..78f3cde92 100644 --- a/packages/excalidraw/tests/history.test.tsx +++ b/packages/excalidraw/tests/history.test.tsx @@ -4785,21 +4785,17 @@ describe("history", () => { expect.objectContaining({ id: rect2.id, boundElements: [] }), expect.objectContaining({ id: arrowId, - points: [ - [0, 0], - [100, 0], - ], startBinding: expect.objectContaining({ elementId: rect1.id, fixedPoint: null, - focus: expect.toBeNonNaNNumber(), - gap: expect.toBeNonNaNNumber(), + focus: 0, + gap: 1, }), endBinding: expect.objectContaining({ elementId: rect2.id, fixedPoint: null, - focus: expect.toBeNonNaNNumber(), - gap: expect.toBeNonNaNNumber(), + focus: 0, + gap: 1, }), isDeleted: true, }),