diff --git a/packages/element/src/binding.ts b/packages/element/src/binding.ts index 8420b5df5a..ee5d037a8c 100644 --- a/packages/element/src/binding.ts +++ b/packages/element/src/binding.ts @@ -27,9 +27,7 @@ import { PRECISION, } from "@excalidraw/math"; -import { isPointInShape } from "@excalidraw/utils/collision"; - -import { getEllipseShape, getPolygonShape } from "@excalidraw/utils/shape"; +import { isPointOnShape } from "@excalidraw/utils/collision"; import type { LocalPoint, Radians } from "@excalidraw/math"; @@ -46,6 +44,7 @@ import { intersectElementWithLineSegment } from "./collision"; import { distanceToBindableElement } from "./distance"; import { headingForPointFromElement, + headingIsHorizontal, vectorToHeading, type Heading, } from "./heading"; @@ -64,7 +63,7 @@ import { isTextElement, } from "./typeChecks"; -import { aabbForElement } from "./shapes"; +import { aabbForElement, getElementShape, pointInsideBounds } from "./shapes"; import { updateElbowArrowPoints } from "./elbowArrow"; import type Scene from "./Scene"; @@ -108,7 +107,7 @@ export const isBindingEnabled = (appState: AppState): boolean => { }; export const FIXED_BINDING_DISTANCE = 5; -const BINDING_HIGHLIGHT_THICKNESS = 10; +export const BINDING_HIGHLIGHT_THICKNESS = 10; export const BINDING_HIGHLIGHT_OFFSET = 4; const getNonDeletedElements = ( @@ -442,15 +441,19 @@ 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: Math.min(binding.gap, maxGap), + gap, }; }; @@ -562,25 +565,21 @@ export const getHoveredElementForBinding = ( let cullRest = false; const candidateElements = getAllElementsAtPositionForBinding( elements, - (element) => { - const result = - 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), - ); - - return result; - }, + (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; @@ -888,12 +887,7 @@ export const getHeadingForElbowArrowSnap = ( return otherPointHeading; } - const distance = getDistanceForBinding( - origPoint, - bindableElement, - true, - zoom, - ); + const distance = getDistanceForBinding(origPoint, bindableElement, zoom); if (!distance) { return vectorToHeading( @@ -904,10 +898,9 @@ export const getHeadingForElbowArrowSnap = ( return headingForPointFromElement(bindableElement, aabb, p); }; -export const getDistanceForBinding = ( +const getDistanceForBinding = ( point: Readonly, bindableElement: ExcalidrawBindableElement, - fullShape: boolean, zoom?: AppState["zoom"], ) => { const distance = distanceToBindableElement(bindableElement, point); @@ -917,16 +910,8 @@ export const getDistanceForBinding = ( bindableElement.height, zoom, ); - const isInside = fullShape - ? isPointInShape( - point, - bindableElement.type === "ellipse" - ? getEllipseShape(bindableElement) - : getPolygonShape(bindableElement), - ) - : false; - return distance > bindDistance && !isInside ? null : distance; + return distance > bindDistance ? null : distance; }; export const bindPointToSnapToElementOutline = ( @@ -962,16 +947,23 @@ export const bindPointToSnapToElementOutline = ( let intersection: GlobalPoint | null = null; if (elbowed) { + const isHorizontal = headingIsHorizontal( + headingForPointFromElement(bindableElement, aabb, globalP), + ); + const otherPoint = pointFrom( + isHorizontal ? center[0] : edgePoint[0], + !isHorizontal ? center[1] : edgePoint[1], + ); intersection = intersectElementWithLineSegment( bindableElement, lineSegment( - center, + otherPoint, pointFromVector( vectorScale( - vectorNormalize(vectorFromPoint(edgePoint, center)), + vectorNormalize(vectorFromPoint(edgePoint, otherPoint)), Math.max(bindableElement.width, bindableElement.height) * 2, ), - center, + otherPoint, ), ), )[0]; @@ -1179,48 +1171,6 @@ export const snapToMid = ( center, angle, ); - } else if (element.type === "diamond") { - const distance = FIXED_BINDING_DISTANCE - 1; - const topLeft = pointFrom( - x + width / 4 - distance, - y + height / 4 - distance, - ); - const topRight = pointFrom( - x + (3 * width) / 4 + distance, - y + height / 4 - distance, - ); - const bottomLeft = pointFrom( - x + width / 4 - distance, - y + (3 * height) / 4 + distance, - ); - const bottomRight = pointFrom( - x + (3 * width) / 4 + distance, - y + (3 * height) / 4 + distance, - ); - if ( - pointDistance(topLeft, nonRotated) < - Math.max(horizontalThrehsold, verticalThrehsold) - ) { - return pointRotateRads(topLeft, center, angle); - } - if ( - pointDistance(topRight, nonRotated) < - Math.max(horizontalThrehsold, verticalThrehsold) - ) { - return pointRotateRads(topRight, center, angle); - } - if ( - pointDistance(bottomLeft, nonRotated) < - Math.max(horizontalThrehsold, verticalThrehsold) - ) { - return pointRotateRads(bottomLeft, center, angle); - } - if ( - pointDistance(bottomRight, nonRotated) < - Math.max(horizontalThrehsold, verticalThrehsold) - ) { - return pointRotateRads(bottomRight, center, angle); - } } return p; @@ -1561,14 +1511,14 @@ export const bindingBorderTest = ( zoom?: AppState["zoom"], fullShape?: boolean, ): boolean => { - const distance = getDistanceForBinding( - pointFrom(x, y), - element, - !!fullShape, - zoom, - ); + const threshold = maxBindingGap(element, element.width, element.height, zoom); - return !!distance; + const shape = getElementShape(element, elementsMap); + return ( + isPointOnShape(pointFrom(x, y), shape, threshold) || + (fullShape === true && + pointInsideBounds(pointFrom(x, y), aabbForElement(element))) + ); }; export const maxBindingGap = ( diff --git a/packages/element/src/elbowArrow.ts b/packages/element/src/elbowArrow.ts index 22ae427136..95a2aa8ef5 100644 --- a/packages/element/src/elbowArrow.ts +++ b/packages/element/src/elbowArrow.ts @@ -31,7 +31,6 @@ import { getGlobalFixedPointForBindableElement, snapToMid, getHoveredElementForBinding, - getDistanceForBinding, } from "./binding"; import { distanceToBindableElement } from "./distance"; import { @@ -1256,7 +1255,6 @@ const getElbowArrowData = ( origStartGlobalPoint, hoveredStartElement, options?.isDragging, - options?.zoom, ); const endGlobalPoint = getGlobalPoint( { @@ -1270,7 +1268,6 @@ const getElbowArrowData = ( origEndGlobalPoint, hoveredEndElement, options?.isDragging, - options?.zoom, ); const startHeading = getBindPointHeading( startGlobalPoint, @@ -2214,14 +2211,16 @@ const getGlobalPoint = ( initialPoint: GlobalPoint, element?: ExcalidrawBindableElement | null, isDragging?: boolean, - zoom?: AppState["zoom"], ): GlobalPoint => { if (isDragging) { - if (element && getDistanceForBinding(initialPoint, element, true, zoom)) { - return snapToMid( + if (element) { + const snapPoint = bindPointToSnapToElementOutline( + arrow, element, - bindPointToSnapToElementOutline(arrow, element, startOrEnd), + startOrEnd, ); + + return snapToMid(element, snapPoint); } return initialPoint; diff --git a/packages/element/src/sizeHelpers.ts b/packages/element/src/sizeHelpers.ts index 02f6ea9231..bd3d3fb0c4 100644 --- a/packages/element/src/sizeHelpers.ts +++ b/packages/element/src/sizeHelpers.ts @@ -3,12 +3,10 @@ import { viewportCoordsToSceneCoords, } from "@excalidraw/common"; -import { pointsEqual } from "@excalidraw/math"; - import type { AppState, Offsets, Zoom } from "@excalidraw/excalidraw/types"; import { getCommonBounds, getElementBounds } from "./bounds"; -import { isElbowArrow, isFreeDrawElement, isLinearElement } from "./typeChecks"; +import { isFreeDrawElement, isLinearElement } from "./typeChecks"; import type { ElementsMap, ExcalidrawElement } from "./types"; @@ -18,12 +16,6 @@ import type { ElementsMap, ExcalidrawElement } from "./types"; export const isInvisiblySmallElement = ( element: ExcalidrawElement, ): boolean => { - if (isElbowArrow(element)) { - return ( - element.points.length < 2 || - pointsEqual(element.points[0], element.points[element.points.length - 1]) - ); - } if (isLinearElement(element) || isFreeDrawElement(element)) { return element.points.length < 2; } diff --git a/packages/element/tests/elbowArrow.test.tsx b/packages/element/tests/elbowArrow.test.tsx index 01794345e0..25f64072e7 100644 --- a/packages/element/tests/elbowArrow.test.tsx +++ b/packages/element/tests/elbowArrow.test.tsx @@ -199,7 +199,7 @@ describe("elbow arrow routing", () => { points: [pointFrom(0, 0), pointFrom(90, 200)], }); - expect(arrow.points).toCloselyEqualPoints([ + expect(arrow.points).toEqual([ [0, 0], [45, 0], [45, 200], @@ -253,7 +253,7 @@ describe("elbow arrow ui", () => { expect(arrow.type).toBe("arrow"); expect(arrow.elbowed).toBe(true); - expect(arrow.points).toCloselyEqualPoints([ + expect(arrow.points).toEqual([ [0, 0], [45, 0], [45, 200], @@ -351,7 +351,7 @@ describe("elbow arrow ui", () => { expect(duplicatedArrow.id).not.toBe(originalArrowId); expect(duplicatedArrow.type).toBe("arrow"); expect(duplicatedArrow.elbowed).toBe(true); - expect(duplicatedArrow.points).toCloselyEqualPoints([ + expect(duplicatedArrow.points).toEqual([ [0, 0], [45, 0], [45, 200], @@ -405,98 +405,11 @@ describe("elbow arrow ui", () => { expect(duplicatedArrow.id).not.toBe(originalArrowId); expect(duplicatedArrow.type).toBe("arrow"); expect(duplicatedArrow.elbowed).toBe(true); - expect(duplicatedArrow.points).toCloselyEqualPoints([ + expect(duplicatedArrow.points).toEqual([ [0, 0], [0, 100], [90, 100], [90, 200], ]); }); - - it("elbow arrow snap at diamond quarter point too", async () => { - UI.createElement("diamond", { - x: -50, - y: -50, - width: 100, - height: 100, - }); - - UI.clickTool("arrow"); - UI.clickOnTestId("elbow-arrow"); - - mouse.reset(); - mouse.moveTo(43, 99); - mouse.click(); - mouse.moveTo(27, 25); - mouse.click(); - - let arrow = h.scene.getSelectedElements( - h.state, - )[0] as ExcalidrawArrowElement; - - expect(arrow.endBinding).not.toBe(null); - expect(arrow.x + arrow.points[arrow.points.length - 1][0]).toBeCloseTo( - 29.0355, - ); - expect(arrow.y + arrow.points[arrow.points.length - 1][1]).toBeCloseTo( - 29.0355, - ); - - UI.clickTool("arrow"); - UI.clickOnTestId("elbow-arrow"); - - mouse.reset(); - mouse.moveTo(43, 99); - mouse.click(); - mouse.moveTo(-23, 25); - mouse.click(); - - arrow = h.scene.getSelectedElements(h.state)[0] as ExcalidrawArrowElement; - - expect(arrow.endBinding).not.toBe(null); - expect(arrow.x + arrow.points[arrow.points.length - 1][0]).toBeCloseTo( - -28.5559, - ); - expect(arrow.y + arrow.points[arrow.points.length - 1][1]).toBeCloseTo( - 28.5559, - ); - - UI.clickTool("arrow"); - UI.clickOnTestId("elbow-arrow"); - - mouse.reset(); - mouse.moveTo(43, 99); - mouse.click(); - mouse.moveTo(-27, -25); - mouse.click(); - - arrow = h.scene.getSelectedElements(h.state)[0] as ExcalidrawArrowElement; - - expect(arrow.endBinding).not.toBe(null); - expect(arrow.x + arrow.points[arrow.points.length - 1][0]).toBeCloseTo( - -28.0355, - ); - expect(arrow.y + arrow.points[arrow.points.length - 1][1]).toBeCloseTo( - -28.0355, - ); - - UI.clickTool("arrow"); - UI.clickOnTestId("elbow-arrow"); - - mouse.reset(); - mouse.moveTo(43, 99); - mouse.click(); - mouse.moveTo(23, -25); - mouse.click(); - - arrow = h.scene.getSelectedElements(h.state)[0] as ExcalidrawArrowElement; - - expect(arrow.endBinding).not.toBe(null); - expect(arrow.x + arrow.points[arrow.points.length - 1][0]).toBeCloseTo( - 28.5559, - ); - expect(arrow.y + arrow.points[arrow.points.length - 1][1]).toBeCloseTo( - -28.5559, - ); - }); }); diff --git a/packages/excalidraw/actions/actionFinalize.tsx b/packages/excalidraw/actions/actionFinalize.tsx index 9e3f69107f..22638ee917 100644 --- a/packages/excalidraw/actions/actionFinalize.tsx +++ b/packages/excalidraw/actions/actionFinalize.tsx @@ -7,7 +7,6 @@ import { import { LinearElementEditor } from "@excalidraw/element/linearElementEditor"; import { - isArrowElement, isBindingElement, isLinearElement, } from "@excalidraw/element/typeChecks"; @@ -17,12 +16,6 @@ import { isPathALoop } from "@excalidraw/element/shapes"; import { isInvisiblySmallElement } from "@excalidraw/element/sizeHelpers"; -import type { - ExcalidrawElement, - ExcalidrawLinearElement, - NonDeleted, -} from "@excalidraw/element/types"; - import { t } from "../i18n"; import { resetCursor } from "../cursor"; import { done } from "../components/icons"; @@ -69,7 +62,6 @@ export const actionFinalize = register({ captureUpdate: CaptureUpdateAction.IMMEDIATELY, }; } - } else if (isArrowElement(appState.newElement)) { } let newElements = elements; @@ -90,55 +82,48 @@ export const actionFinalize = register({ focusContainer(); } - let element: NonDeleted | null = null; - if (appState.multiElement) { - element = appState.multiElement; - } else if ( - appState.newElement?.type === "freedraw" || - isBindingElement(appState.newElement) - ) { - element = appState.newElement; - } else if (Object.keys(appState.selectedElementIds).length === 1) { - const candidate = elementsMap.get( - Object.keys(appState.selectedElementIds)[0], - ) as NonDeleted | undefined; - if (candidate) { - element = candidate; - } - } + const multiPointElement = appState.multiElement + ? appState.multiElement + : appState.newElement?.type === "freedraw" + ? appState.newElement + : null; - if (element) { + if (multiPointElement) { // pen and mouse have hover if ( - appState.multiElement && - element.type !== "freedraw" && + multiPointElement.type !== "freedraw" && appState.lastPointerDownWith !== "touch" ) { - const { points, lastCommittedPoint } = element; + const { points, lastCommittedPoint } = multiPointElement; if ( !lastCommittedPoint || points[points.length - 1] !== lastCommittedPoint ) { - scene.mutateElement(element, { - points: element.points.slice(0, -1), + scene.mutateElement(multiPointElement, { + points: multiPointElement.points.slice(0, -1), }); } } - if (isInvisiblySmallElement(element)) { + if (isInvisiblySmallElement(multiPointElement)) { // TODO: #7348 in theory this gets recorded by the store, so the invisible elements could be restored by the undo/redo, which might be not what we would want - newElements = newElements.filter((el) => el.id !== element!.id); + newElements = newElements.filter( + (el) => el.id !== multiPointElement.id, + ); } // If the multi point line closes the loop, // set the last point to first point. // This ensures that loop remains closed at different scales. - const isLoop = isPathALoop(element.points, appState.zoom.value); - if (element.type === "line" || element.type === "freedraw") { + const isLoop = isPathALoop(multiPointElement.points, appState.zoom.value); + if ( + multiPointElement.type === "line" || + multiPointElement.type === "freedraw" + ) { if (isLoop) { - const linePoints = element.points; + const linePoints = multiPointElement.points; const firstPoint = linePoints[0]; - scene.mutateElement(element, { + scene.mutateElement(multiPointElement, { points: linePoints.map((p, index) => index === linePoints.length - 1 ? pointFrom(firstPoint[0], firstPoint[1]) @@ -149,24 +134,23 @@ export const actionFinalize = register({ } if ( - isBindingElement(element) && + isBindingElement(multiPointElement) && !isLoop && - element.points.length > 1 && - !appState.selectedElementIds[element.id] + multiPointElement.points.length > 1 ) { const [x, y] = LinearElementEditor.getPointAtIndexGlobalCoordinates( - element, + multiPointElement, -1, arrayToMap(elements), ); - maybeBindLinearElement(element, appState, { x, y }, scene); + maybeBindLinearElement(multiPointElement, appState, { x, y }, scene); } } if ( (!appState.activeTool.locked && appState.activeTool.type !== "freedraw") || - !element + !multiPointElement ) { resetCursor(interactiveCanvas); } @@ -193,7 +177,7 @@ export const actionFinalize = register({ activeTool: (appState.activeTool.locked || appState.activeTool.type === "freedraw") && - element + multiPointElement ? appState.activeTool : activeTool, activeEmbeddable: null, @@ -204,18 +188,21 @@ export const actionFinalize = register({ startBoundElement: null, suggestedBindings: [], selectedElementIds: - element && + multiPointElement && !appState.activeTool.locked && appState.activeTool.type !== "freedraw" ? { ...appState.selectedElementIds, - [element.id]: true, + [multiPointElement.id]: true, } : appState.selectedElementIds, // To select the linear element when user has finished mutipoint editing selectedLinearElement: - element && isLinearElement(element) - ? new LinearElementEditor(element, arrayToMap(newElements)) + multiPointElement && isLinearElement(multiPointElement) + ? new LinearElementEditor( + multiPointElement, + arrayToMap(newElements), + ) : appState.selectedLinearElement, pendingImageElementId: null, }, diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 7c7d2ba495..ddb071981f 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -9022,7 +9022,6 @@ class App extends React.Component { linearElementEditor; const element = this.scene.getElement(linearElementEditor.elementId); if (isBindingElement(element)) { - this.actionManager.executeAction(actionFinalize); bindOrUnbindLinearElement( element, startBindingElement, diff --git a/packages/excalidraw/renderer/interactiveScene.ts b/packages/excalidraw/renderer/interactiveScene.ts index bfcac96b60..69c6a8196e 100644 --- a/packages/excalidraw/renderer/interactiveScene.ts +++ b/packages/excalidraw/renderer/interactiveScene.ts @@ -17,6 +17,7 @@ import { import { BINDING_HIGHLIGHT_OFFSET, + BINDING_HIGHLIGHT_THICKNESS, maxBindingGap, } from "@excalidraw/element/binding"; import { LinearElementEditor } from "@excalidraw/element/linearElementEditor"; @@ -260,9 +261,9 @@ const renderBindingHighlightForBindableElement = ( const height = y2 - y1; context.strokeStyle = "rgba(0,0,0,.05)"; - context.lineWidth = - maxBindingGap(element, element.width, element.height, zoom) - - BINDING_HIGHLIGHT_OFFSET; + // 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;