From 66a2f242962b480dc855b761dd2411d632cbab59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rk=20Tolm=C3=A1cs?= Date: Mon, 1 Jul 2024 09:45:31 +0200 Subject: [PATCH] fix: Add binding update to manual stat changes (#8183) Manual stats changes now respect previous element bindings. --- .../excalidraw/actions/actionFinalize.tsx | 7 +- packages/excalidraw/actions/actionFlip.ts | 2 +- packages/excalidraw/components/App.tsx | 142 +++++++----------- .../excalidraw/components/Stats/Angle.tsx | 5 +- .../components/Stats/MultiDimension.tsx | 12 +- .../components/Stats/MultiPosition.tsx | 12 +- .../components/Stats/stats.test.tsx | 88 +++++++++++ packages/excalidraw/components/Stats/utils.ts | 45 ++++-- packages/excalidraw/element/binding.ts | 106 +++++++------ .../excalidraw/element/linearElementEditor.ts | 7 +- packages/excalidraw/shapes.tsx | 62 ++++++++ packages/excalidraw/types.ts | 1 - 12 files changed, 327 insertions(+), 162 deletions(-) diff --git a/packages/excalidraw/actions/actionFinalize.tsx b/packages/excalidraw/actions/actionFinalize.tsx index 9661154f76..15956b3a39 100644 --- a/packages/excalidraw/actions/actionFinalize.tsx +++ b/packages/excalidraw/actions/actionFinalize.tsx @@ -131,7 +131,12 @@ export const actionFinalize = register({ -1, arrayToMap(elements), ); - maybeBindLinearElement(multiPointElement, appState, { x, y }, app); + maybeBindLinearElement( + multiPointElement, + appState, + { x, y }, + elementsMap, + ); } } diff --git a/packages/excalidraw/actions/actionFlip.ts b/packages/excalidraw/actions/actionFlip.ts index 0aab7f903c..3f521d27f8 100644 --- a/packages/excalidraw/actions/actionFlip.ts +++ b/packages/excalidraw/actions/actionFlip.ts @@ -124,7 +124,7 @@ const flipElements = ( bindOrUnbindLinearElements( selectedElements.filter(isLinearElement), - app, + elementsMap, isBindingEnabled(appState), [], ); diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 3fcde774a5..7be5295943 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -225,16 +225,9 @@ import type { ScrollBars, } from "../scene/types"; import { getStateForZoom } from "../scene/zoom"; -import { findShapeByKey } from "../shapes"; +import { findShapeByKey, getElementShape } from "../shapes"; import type { GeometricShape } from "../../utils/geometry/shape"; -import { - getClosedCurveShape, - getCurveShape, - getEllipseShape, - getFreedrawShape, - getPolygonShape, - getSelectionBoxShape, -} from "../../utils/geometry/shape"; +import { getSelectionBoxShape } from "../../utils/geometry/shape"; import { isPointInShape } from "../../utils/collision"; import type { AppClassProperties, @@ -424,7 +417,6 @@ import { hitElementBoundText, hitElementBoundingBoxOnly, hitElementItself, - shouldTestInside, } from "../element/collision"; import { textWysiwyg } from "../element/textWysiwyg"; import { isOverScrollBars } from "../scene/scrollbars"; @@ -2819,7 +2811,7 @@ class App extends React.Component { nonDeletedElementsMap, ), ), - this, + this.scene.getNonDeletedElementsMap(), ); } @@ -4008,7 +4000,7 @@ class App extends React.Component { this.setState({ suggestedBindings: getSuggestedBindingsForArrows( selectedElements, - this, + this.scene.getNonDeletedElementsMap(), ), }); @@ -4179,7 +4171,7 @@ class App extends React.Component { if (isArrowKey(event.key)) { bindOrUnbindLinearElements( this.scene.getSelectedElements(this.state).filter(isLinearElement), - this, + this.scene.getNonDeletedElementsMap(), isBindingEnabled(this.state), this.state.selectedLinearElement?.selectedPointsIndices ?? [], ); @@ -4491,59 +4483,6 @@ class App extends React.Component { return null; } - /** - * get the pure geometric shape of an excalidraw element - * which is then used for hit detection - */ - public getElementShape(element: ExcalidrawElement): GeometricShape { - switch (element.type) { - case "rectangle": - case "diamond": - case "frame": - case "magicframe": - case "embeddable": - case "image": - case "iframe": - case "text": - case "selection": - return getPolygonShape(element); - case "arrow": - case "line": { - const roughShape = - ShapeCache.get(element)?.[0] ?? - ShapeCache.generateElementShape(element, null)[0]; - const [, , , , cx, cy] = getElementAbsoluteCoords( - element, - this.scene.getNonDeletedElementsMap(), - ); - - return shouldTestInside(element) - ? getClosedCurveShape( - element, - roughShape, - [element.x, element.y], - element.angle, - [cx, cy], - ) - : getCurveShape(roughShape, [element.x, element.y], element.angle, [ - cx, - cy, - ]); - } - - case "ellipse": - return getEllipseShape(element); - - case "freedraw": { - const [, , , , cx, cy] = getElementAbsoluteCoords( - element, - this.scene.getNonDeletedElementsMap(), - ); - return getFreedrawShape(element, [cx, cy], shouldTestInside(element)); - } - } - } - private getBoundTextShape(element: ExcalidrawElement): GeometricShape | null { const boundTextElement = getBoundTextElement( element, @@ -4552,18 +4491,24 @@ class App extends React.Component { if (boundTextElement) { if (element.type === "arrow") { - return this.getElementShape({ - ...boundTextElement, - // arrow's bound text accurate position is not stored in the element's property - // but rather calculated and returned from the following static method - ...LinearElementEditor.getBoundTextElementPosition( - element, - boundTextElement, - this.scene.getNonDeletedElementsMap(), - ), - }); + return getElementShape( + { + ...boundTextElement, + // arrow's bound text accurate position is not stored in the element's property + // but rather calculated and returned from the following static method + ...LinearElementEditor.getBoundTextElementPosition( + element, + boundTextElement, + this.scene.getNonDeletedElementsMap(), + ), + }, + this.scene.getNonDeletedElementsMap(), + ); } - return this.getElementShape(boundTextElement); + return getElementShape( + boundTextElement, + this.scene.getNonDeletedElementsMap(), + ); } return null; @@ -4602,7 +4547,10 @@ class App extends React.Component { x, y, element: elementWithHighestZIndex, - shape: this.getElementShape(elementWithHighestZIndex), + shape: getElementShape( + elementWithHighestZIndex, + this.scene.getNonDeletedElementsMap(), + ), // when overlapping, we would like to be more precise // this also avoids the need to update past tests threshold: this.getElementHitThreshold() / 2, @@ -4707,7 +4655,7 @@ class App extends React.Component { x, y, element, - shape: this.getElementShape(element), + shape: getElementShape(element, this.scene.getNonDeletedElementsMap()), threshold: this.getElementHitThreshold(), frameNameBound: isFrameLikeElement(element) ? this.frameNameBoundsCache.get(element) @@ -4739,7 +4687,10 @@ class App extends React.Component { x, y, element: elements[index], - shape: this.getElementShape(elements[index]), + shape: getElementShape( + elements[index], + this.scene.getNonDeletedElementsMap(), + ), threshold: this.getElementHitThreshold(), }) ) { @@ -4997,7 +4948,10 @@ class App extends React.Component { x: sceneX, y: sceneY, element: container, - shape: this.getElementShape(container), + shape: getElementShape( + container, + this.scene.getNonDeletedElementsMap(), + ), threshold: this.getElementHitThreshold(), }) ) { @@ -5689,7 +5643,10 @@ class App extends React.Component { x: scenePointerX, y: scenePointerY, element, - shape: this.getElementShape(element), + shape: getElementShape( + element, + this.scene.getNonDeletedElementsMap(), + ), }) ) { hoverPointIndex = LinearElementEditor.getPointIndexUnderCursor( @@ -6808,7 +6765,7 @@ class App extends React.Component { const boundElement = getHoveredElementForBinding( pointerDownState.origin, - this, + this.scene.getNonDeletedElementsMap(), ); this.scene.insertElement(element); this.setState({ @@ -7070,7 +7027,7 @@ class App extends React.Component { }); const boundElement = getHoveredElementForBinding( pointerDownState.origin, - this, + this.scene.getNonDeletedElementsMap(), ); this.scene.insertElement(element); @@ -7540,7 +7497,7 @@ class App extends React.Component { this.setState({ suggestedBindings: getSuggestedBindingsForArrows( selectedElements, - this, + this.scene.getNonDeletedElementsMap(), ), }); @@ -8061,7 +8018,7 @@ class App extends React.Component { draggingElement, this.state, pointerCoords, - this, + this.scene.getNonDeletedElementsMap(), ); } this.setState({ suggestedBindings: [], startBoundElement: null }); @@ -8551,7 +8508,10 @@ class App extends React.Component { x: pointerDownState.origin.x, y: pointerDownState.origin.y, element: hitElement, - shape: this.getElementShape(hitElement), + shape: getElementShape( + hitElement, + this.scene.getNonDeletedElementsMap(), + ), threshold: this.getElementHitThreshold(), frameNameBound: isFrameLikeElement(hitElement) ? this.frameNameBoundsCache.get(hitElement) @@ -8619,7 +8579,7 @@ class App extends React.Component { bindOrUnbindLinearElements( linearElements, - this, + this.scene.getNonDeletedElementsMap(), isBindingEnabled(this.state), this.state.selectedLinearElement?.selectedPointsIndices ?? [], ); @@ -9107,7 +9067,7 @@ class App extends React.Component { }): void => { const hoveredBindableElement = getHoveredElementForBinding( pointerCoords, - this, + this.scene.getNonDeletedElementsMap(), ); this.setState({ suggestedBindings: @@ -9134,7 +9094,7 @@ class App extends React.Component { (acc: NonDeleted[], coords) => { const hoveredBindableElement = getHoveredElementForBinding( coords, - this, + this.scene.getNonDeletedElementsMap(), ); if ( hoveredBindableElement != null && @@ -9666,7 +9626,7 @@ class App extends React.Component { ) { const suggestedBindings = getSuggestedBindingsForArrows( selectedElements, - this, + this.scene.getNonDeletedElementsMap(), ); const elementsToHighlight = new Set(); diff --git a/packages/excalidraw/components/Stats/Angle.tsx b/packages/excalidraw/components/Stats/Angle.tsx index 23cae2b6b6..6727a39562 100644 --- a/packages/excalidraw/components/Stats/Angle.tsx +++ b/packages/excalidraw/components/Stats/Angle.tsx @@ -6,7 +6,7 @@ import { degreeToRadian, radianToDegree } from "../../math"; import { angleIcon } from "../icons"; import DragInput from "./DragInput"; import type { DragInputCallbackType } from "./DragInput"; -import { getStepSizedValue, isPropertyEditable } from "./utils"; +import { getStepSizedValue, isPropertyEditable, updateBindings } from "./utils"; import type Scene from "../../scene/Scene"; import type { AppState } from "../../types"; @@ -33,11 +33,13 @@ const handleDegreeChange: DragInputCallbackType = ({ if (!latestElement) { return; } + if (nextValue !== undefined) { const nextAngle = degreeToRadian(nextValue); mutateElement(latestElement, { angle: nextAngle, }); + updateBindings(latestElement, elementsMap); const boundTextElement = getBoundTextElement(latestElement, elementsMap); if (boundTextElement && !isArrowElement(latestElement)) { @@ -63,6 +65,7 @@ const handleDegreeChange: DragInputCallbackType = ({ mutateElement(latestElement, { angle: nextAngle, }); + updateBindings(latestElement, elementsMap); const boundTextElement = getBoundTextElement(latestElement, elementsMap); if (boundTextElement && !isArrowElement(latestElement)) { diff --git a/packages/excalidraw/components/Stats/MultiDimension.tsx b/packages/excalidraw/components/Stats/MultiDimension.tsx index e6fd715e9f..89b746a5f9 100644 --- a/packages/excalidraw/components/Stats/MultiDimension.tsx +++ b/packages/excalidraw/components/Stats/MultiDimension.tsx @@ -7,7 +7,11 @@ import { getBoundTextElement, handleBindTextResize, } from "../../element/textElement"; -import type { ElementsMap, ExcalidrawElement } from "../../element/types"; +import type { + ElementsMap, + ExcalidrawElement, + NonDeletedSceneElementsMap, +} from "../../element/types"; import type Scene from "../../scene/Scene"; import type { AppState, Point } from "../../types"; import DragInput from "./DragInput"; @@ -20,7 +24,7 @@ import { MIN_WIDTH_OR_HEIGHT } from "../../constants"; interface MultiDimensionProps { property: "width" | "height"; elements: readonly ExcalidrawElement[]; - elementsMap: ElementsMap; + elementsMap: NonDeletedSceneElementsMap; atomicUnits: AtomicUnit[]; scene: Scene; appState: AppState; @@ -60,7 +64,7 @@ const resizeElementInGroup = ( scale: number, latestElement: ExcalidrawElement, origElement: ExcalidrawElement, - elementsMap: ElementsMap, + elementsMap: NonDeletedSceneElementsMap, originalElementsMap: ElementsMap, ) => { const updates = getResizedUpdates(anchorX, anchorY, scale, origElement); @@ -103,7 +107,7 @@ const resizeGroup = ( property: MultiDimensionProps["property"], latestElements: ExcalidrawElement[], originalElements: ExcalidrawElement[], - elementsMap: ElementsMap, + elementsMap: NonDeletedSceneElementsMap, originalElementsMap: ElementsMap, ) => { // keep aspect ratio for groups diff --git a/packages/excalidraw/components/Stats/MultiPosition.tsx b/packages/excalidraw/components/Stats/MultiPosition.tsx index c7f3491b4a..e203908489 100644 --- a/packages/excalidraw/components/Stats/MultiPosition.tsx +++ b/packages/excalidraw/components/Stats/MultiPosition.tsx @@ -1,4 +1,8 @@ -import type { ElementsMap, ExcalidrawElement } from "../../element/types"; +import type { + ElementsMap, + ExcalidrawElement, + NonDeletedSceneElementsMap, +} from "../../element/types"; import { rotate } from "../../math"; import type Scene from "../../scene/Scene"; import StatsDragInput from "./DragInput"; @@ -27,7 +31,7 @@ const moveElements = ( changeInTopY: number, elements: readonly ExcalidrawElement[], originalElements: readonly ExcalidrawElement[], - elementsMap: ElementsMap, + elementsMap: NonDeletedSceneElementsMap, originalElementsMap: ElementsMap, ) => { for (let i = 0; i < elements.length; i++) { @@ -66,8 +70,9 @@ const moveGroupTo = ( nextX: number, nextY: number, originalElements: ExcalidrawElement[], - elementsMap: ElementsMap, + elementsMap: NonDeletedSceneElementsMap, originalElementsMap: ElementsMap, + scene: Scene, ) => { const [x1, y1, ,] = getCommonBounds(originalElements); const offsetX = nextX - x1; @@ -146,6 +151,7 @@ const handlePositionChange: DragInputCallbackType< elementsInUnit.map((el) => el.original), elementsMap, originalElementsMap, + scene, ); } else { const origElement = elementsInUnit[0]?.original; diff --git a/packages/excalidraw/components/Stats/stats.test.tsx b/packages/excalidraw/components/Stats/stats.test.tsx index 16e8e733c0..365abf7758 100644 --- a/packages/excalidraw/components/Stats/stats.test.tsx +++ b/packages/excalidraw/components/Stats/stats.test.tsx @@ -15,6 +15,7 @@ import { Excalidraw, mutateElement } from "../.."; import { t } from "../../i18n"; import type { ExcalidrawElement, + ExcalidrawLinearElement, ExcalidrawTextElement, } from "../../element/types"; import { degreeToRadian, rotate } from "../../math"; @@ -23,6 +24,7 @@ import { getCommonBounds, isTextElement } from "../../element"; import { API } from "../../tests/helpers/api"; import { actionGroup } from "../../actions"; import { isInGroup } from "../../groups"; +import React from "react"; const { h } = window; const mouse = new Pointer("mouse"); @@ -99,6 +101,92 @@ describe("step sized value", () => { }); }); +describe("binding with linear elements", () => { + beforeEach(async () => { + localStorage.clear(); + renderStaticScene.mockClear(); + reseed(19); + setDateTimeForTests("201933152653"); + + await render(); + + h.elements = []; + + fireEvent.contextMenu(GlobalTestState.interactiveCanvas, { + button: 2, + clientX: 1, + clientY: 1, + }); + const contextMenu = UI.queryContextMenu(); + fireEvent.click(queryByTestId(contextMenu!, "stats")!); + stats = UI.queryStats(); + + UI.clickTool("rectangle"); + mouse.down(); + mouse.up(200, 100); + + UI.clickTool("arrow"); + mouse.down(5, 0); + mouse.up(300, 50); + + elementStats = stats?.querySelector("#elementStats"); + }); + + beforeAll(() => { + mockBoundingClientRect(); + }); + + afterAll(() => { + restoreOriginalGetBoundingClientRect(); + }); + + it("should remain bound to linear element on small position change", async () => { + const linear = h.elements[1] as ExcalidrawLinearElement; + const inputX = getStatsProperty("X")?.querySelector( + ".drag-input", + ) as HTMLInputElement; + + expect(linear.startBinding).not.toBe(null); + expect(inputX).not.toBeNull(); + editInput(inputX, String("204")); + expect(linear.startBinding).not.toBe(null); + }); + + it("should remain bound to linear element on small angle change", async () => { + const linear = h.elements[1] as ExcalidrawLinearElement; + const inputAngle = getStatsProperty("A")?.querySelector( + ".drag-input", + ) as HTMLInputElement; + + expect(linear.startBinding).not.toBe(null); + editInput(inputAngle, String("1")); + expect(linear.startBinding).not.toBe(null); + }); + + it("should unbind linear element on large position change", async () => { + const linear = h.elements[1] as ExcalidrawLinearElement; + const inputX = getStatsProperty("X")?.querySelector( + ".drag-input", + ) as HTMLInputElement; + + expect(linear.startBinding).not.toBe(null); + expect(inputX).not.toBeNull(); + editInput(inputX, String("254")); + expect(linear.startBinding).toBe(null); + }); + + it("should remain bound to linear element on small angle change", async () => { + const linear = h.elements[1] as ExcalidrawLinearElement; + const inputAngle = getStatsProperty("A")?.querySelector( + ".drag-input", + ) as HTMLInputElement; + + expect(linear.startBinding).not.toBe(null); + editInput(inputAngle, String("45")); + expect(linear.startBinding).toBe(null); + }); +}); + // single element describe("stats for a generic element", () => { beforeEach(async () => { diff --git a/packages/excalidraw/components/Stats/utils.ts b/packages/excalidraw/components/Stats/utils.ts index 1408b8a681..591fcff90f 100644 --- a/packages/excalidraw/components/Stats/utils.ts +++ b/packages/excalidraw/components/Stats/utils.ts @@ -1,4 +1,7 @@ -import { updateBoundElements } from "../../element/binding"; +import { + bindOrUnbindLinearElements, + updateBoundElements, +} from "../../element/binding"; import { mutateElement } from "../../element/mutateElement"; import { measureFontSizeFromWidth, @@ -11,11 +14,16 @@ import { getBoundTextMaxWidth, handleBindTextResize, } from "../../element/textElement"; -import { isFrameLikeElement, isTextElement } from "../../element/typeChecks"; +import { + isFrameLikeElement, + isLinearElement, + isTextElement, +} from "../../element/typeChecks"; import type { ElementsMap, ExcalidrawElement, NonDeletedExcalidrawElement, + NonDeletedSceneElementsMap, } from "../../element/types"; import { getSelectedGroupIds, @@ -115,7 +123,7 @@ export const resizeElement = ( nextHeight: number, keepAspectRatio: boolean, origElement: ExcalidrawElement, - elementsMap: ElementsMap, + elementsMap: NonDeletedSceneElementsMap, shouldInformMutation = true, ) => { const latestElement = elementsMap.get(origElement.id); @@ -156,6 +164,12 @@ export const resizeElement = ( }, shouldInformMutation, ); + updateBindings(latestElement, elementsMap, { + newSize: { + width: nextWidth, + height: nextHeight, + }, + }); if (boundTextElement) { boundTextFont = { @@ -179,13 +193,6 @@ export const resizeElement = ( } } - updateBoundElements(latestElement, elementsMap, { - newSize: { - width: nextWidth, - height: nextHeight, - }, - }); - if (boundTextElement && boundTextFont) { mutateElement(boundTextElement, { fontSize: boundTextFont.fontSize, @@ -198,7 +205,7 @@ export const moveElement = ( newTopLeftX: number, newTopLeftY: number, originalElement: ExcalidrawElement, - elementsMap: ElementsMap, + elementsMap: NonDeletedSceneElementsMap, originalElementsMap: ElementsMap, shouldInformMutation = true, ) => { @@ -237,6 +244,7 @@ export const moveElement = ( }, shouldInformMutation, ); + updateBindings(latestElement, elementsMap); const boundTextElement = getBoundTextElement( originalElement, @@ -276,3 +284,18 @@ export const getAtomicUnits = ( }); return _atomicUnits; }; + +export const updateBindings = ( + latestElement: ExcalidrawElement, + elementsMap: NonDeletedSceneElementsMap, + options?: { + simultaneouslyUpdated?: readonly ExcalidrawElement[]; + newSize?: { width: number; height: number }; + }, +) => { + if (isLinearElement(latestElement)) { + bindOrUnbindLinearElements([latestElement], elementsMap, true, []); + } else { + updateBoundElements(latestElement, elementsMap, options); + } +}; diff --git a/packages/excalidraw/element/binding.ts b/packages/excalidraw/element/binding.ts index ba0a110c64..1bec392390 100644 --- a/packages/excalidraw/element/binding.ts +++ b/packages/excalidraw/element/binding.ts @@ -25,7 +25,7 @@ import type { } from "./types"; import { getElementAbsoluteCoords } from "./bounds"; -import type { AppClassProperties, AppState, Point } from "../types"; +import type { AppState, Point } from "../types"; import { isPointOnShape } from "../../utils/collision"; import { getElementAtPosition } from "../scene"; import { @@ -43,6 +43,7 @@ import { LinearElementEditor } from "./linearElementEditor"; import { arrayToMap, tupleToCoors } from "../utils"; import { KEYS } from "../keys"; import { getBoundTextElement, handleBindTextResize } from "./textElement"; +import { getElementShape } from "../shapes"; export type SuggestedBinding = | NonDeleted @@ -179,9 +180,8 @@ const bindOrUnbindLinearElementEdge = ( const getOriginalBindingIfStillCloseOfLinearElementEdge = ( linearElement: NonDeleted, edge: "start" | "end", - app: AppClassProperties, + elementsMap: NonDeletedSceneElementsMap, ): NonDeleted | null => { - const elementsMap = app.scene.getNonDeletedElementsMap(); const coors = getLinearElementEdgeCoors(linearElement, edge, elementsMap); const elementId = edge === "start" @@ -189,7 +189,10 @@ const getOriginalBindingIfStillCloseOfLinearElementEdge = ( : linearElement.endBinding?.elementId; if (elementId) { const element = elementsMap.get(elementId); - if (isBindableElement(element) && bindingBorderTest(element, coors, app)) { + if ( + isBindableElement(element) && + bindingBorderTest(element, coors, elementsMap) + ) { return element; } } @@ -199,13 +202,13 @@ const getOriginalBindingIfStillCloseOfLinearElementEdge = ( const getOriginalBindingsIfStillCloseToArrowEnds = ( linearElement: NonDeleted, - app: AppClassProperties, + elementsMap: NonDeletedSceneElementsMap, ): (NonDeleted | null)[] => ["start", "end"].map((edge) => getOriginalBindingIfStillCloseOfLinearElementEdge( linearElement, edge as "start" | "end", - app, + elementsMap, ), ); @@ -213,7 +216,7 @@ const getBindingStrategyForDraggingArrowEndpoints = ( selectedElement: NonDeleted, isBindingEnabled: boolean, draggingPoints: readonly number[], - app: AppClassProperties, + elementsMap: NonDeletedSceneElementsMap, ): (NonDeleted | null | "keep")[] => { const startIdx = 0; const endIdx = selectedElement.points.length - 1; @@ -221,37 +224,57 @@ const getBindingStrategyForDraggingArrowEndpoints = ( const endDragged = draggingPoints.findIndex((i) => i === endIdx) > -1; const start = startDragged ? isBindingEnabled - ? getElligibleElementForBindingElement(selectedElement, "start", app) + ? getElligibleElementForBindingElement( + selectedElement, + "start", + elementsMap, + ) : 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 - getElligibleElementForBindingElement(selectedElement, "start", app); + getElligibleElementForBindingElement( + selectedElement, + "start", + elementsMap, + ); const end = endDragged ? isBindingEnabled - ? getElligibleElementForBindingElement(selectedElement, "end", app) + ? getElligibleElementForBindingElement( + selectedElement, + "end", + elementsMap, + ) : 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 - getElligibleElementForBindingElement(selectedElement, "end", app); + getElligibleElementForBindingElement(selectedElement, "end", elementsMap); return [start, end]; }; const getBindingStrategyForDraggingArrowOrJoints = ( selectedElement: NonDeleted, - app: AppClassProperties, + elementsMap: NonDeletedSceneElementsMap, isBindingEnabled: boolean, ): (NonDeleted | null | "keep")[] => { const [startIsClose, endIsClose] = getOriginalBindingsIfStillCloseToArrowEnds( selectedElement, - app, + elementsMap, ); const start = startIsClose ? isBindingEnabled - ? getElligibleElementForBindingElement(selectedElement, "start", app) + ? getElligibleElementForBindingElement( + selectedElement, + "start", + elementsMap, + ) : null : null; const end = endIsClose ? isBindingEnabled - ? getElligibleElementForBindingElement(selectedElement, "end", app) + ? getElligibleElementForBindingElement( + selectedElement, + "end", + elementsMap, + ) : null : null; @@ -260,7 +283,7 @@ const getBindingStrategyForDraggingArrowOrJoints = ( export const bindOrUnbindLinearElements = ( selectedElements: NonDeleted[], - app: AppClassProperties, + elementsMap: NonDeletedSceneElementsMap, isBindingEnabled: boolean, draggingPoints: readonly number[] | null, ): void => { @@ -271,27 +294,22 @@ export const bindOrUnbindLinearElements = ( selectedElement, isBindingEnabled, draggingPoints ?? [], - app, + elementsMap, ) : // The arrow itself (the shaft) or the inner joins are dragged getBindingStrategyForDraggingArrowOrJoints( selectedElement, - app, + elementsMap, isBindingEnabled, ); - bindOrUnbindLinearElement( - selectedElement, - start, - end, - app.scene.getNonDeletedElementsMap(), - ); + bindOrUnbindLinearElement(selectedElement, start, end, elementsMap); }); }; export const getSuggestedBindingsForArrows = ( selectedElements: NonDeleted[], - app: AppClassProperties, + elementsMap: NonDeletedSceneElementsMap, ): SuggestedBinding[] => { // HOT PATH: Bail out if selected elements list is too large if (selectedElements.length > 50) { @@ -302,7 +320,7 @@ export const getSuggestedBindingsForArrows = ( selectedElements .filter(isLinearElement) .flatMap((element) => - getOriginalBindingsIfStillCloseToArrowEnds(element, app), + getOriginalBindingsIfStillCloseToArrowEnds(element, elementsMap), ) .filter( (element): element is NonDeleted => @@ -324,17 +342,20 @@ export const maybeBindLinearElement = ( linearElement: NonDeleted, appState: AppState, pointerCoords: { x: number; y: number }, - app: AppClassProperties, + elementsMap: NonDeletedSceneElementsMap, ): void => { if (appState.startBoundElement != null) { bindLinearElement( linearElement, appState.startBoundElement, "start", - app.scene.getNonDeletedElementsMap(), + elementsMap, ); } - const hoveredElement = getHoveredElementForBinding(pointerCoords, app); + const hoveredElement = getHoveredElementForBinding( + pointerCoords, + elementsMap, + ); if ( hoveredElement != null && !isLinearElementSimpleAndAlreadyBoundOnOppositeEdge( @@ -343,12 +364,7 @@ export const maybeBindLinearElement = ( "end", ) ) { - bindLinearElement( - linearElement, - hoveredElement, - "end", - app.scene.getNonDeletedElementsMap(), - ); + bindLinearElement(linearElement, hoveredElement, "end", elementsMap); } }; @@ -432,13 +448,13 @@ export const getHoveredElementForBinding = ( x: number; y: number; }, - app: AppClassProperties, + elementsMap: NonDeletedSceneElementsMap, ): NonDeleted | null => { const hoveredElement = getElementAtPosition( - app.scene.getNonDeletedElements(), + [...elementsMap].map(([_, value]) => value), (element) => isBindableElement(element, false) && - bindingBorderTest(element, pointerCoords, app), + bindingBorderTest(element, pointerCoords, elementsMap), ); return hoveredElement as NonDeleted | null; }; @@ -662,15 +678,11 @@ const maybeCalculateNewGapWhenScaling = ( const getElligibleElementForBindingElement = ( linearElement: NonDeleted, startOrEnd: "start" | "end", - app: AppClassProperties, + elementsMap: NonDeletedSceneElementsMap, ): NonDeleted | null => { return getHoveredElementForBinding( - getLinearElementEdgeCoors( - linearElement, - startOrEnd, - app.scene.getNonDeletedElementsMap(), - ), - app, + getLinearElementEdgeCoors(linearElement, startOrEnd, elementsMap), + elementsMap, ); }; @@ -834,10 +846,10 @@ const newBoundElements = ( const bindingBorderTest = ( element: NonDeleted, { x, y }: { x: number; y: number }, - app: AppClassProperties, + elementsMap: ElementsMap, ): boolean => { const threshold = maxBindingGap(element, element.width, element.height); - const shape = app.getElementShape(element); + const shape = getElementShape(element, elementsMap); return isPointOnShape([x, y], shape, threshold); }; diff --git a/packages/excalidraw/element/linearElementEditor.ts b/packages/excalidraw/element/linearElementEditor.ts index 244bc275aa..9719227628 100644 --- a/packages/excalidraw/element/linearElementEditor.ts +++ b/packages/excalidraw/element/linearElementEditor.ts @@ -381,7 +381,7 @@ export class LinearElementEditor { elementsMap, ), ), - app, + elementsMap, ) : null; @@ -715,7 +715,10 @@ export class LinearElementEditor { }, selectedPointsIndices: [element.points.length - 1], lastUncommittedPoint: null, - endBindingElement: getHoveredElementForBinding(scenePointer, app), + endBindingElement: getHoveredElementForBinding( + scenePointer, + elementsMap, + ), }; ret.didAddPoint = true; diff --git a/packages/excalidraw/shapes.tsx b/packages/excalidraw/shapes.tsx index a8b9b53012..5c0e986769 100644 --- a/packages/excalidraw/shapes.tsx +++ b/packages/excalidraw/shapes.tsx @@ -1,3 +1,11 @@ +import { + getClosedCurveShape, + getCurveShape, + getEllipseShape, + getFreedrawShape, + getPolygonShape, + type GeometricShape, +} from "../utils/geometry/shape"; import { ArrowIcon, DiamondIcon, @@ -10,7 +18,11 @@ import { SelectionIcon, TextIcon, } from "./components/icons"; +import { getElementAbsoluteCoords } from "./element"; +import { shouldTestInside } from "./element/collision"; +import type { ElementsMap, ExcalidrawElement } from "./element/types"; import { KEYS } from "./keys"; +import { ShapeCache } from "./scene/ShapeCache"; export const SHAPES = [ { @@ -97,3 +109,53 @@ export const findShapeByKey = (key: string) => { }); return shape?.value || null; }; + +/** + * get the pure geometric shape of an excalidraw element + * which is then used for hit detection + */ +export const getElementShape = ( + element: ExcalidrawElement, + elementsMap: ElementsMap, +): GeometricShape => { + switch (element.type) { + case "rectangle": + case "diamond": + case "frame": + case "magicframe": + case "embeddable": + case "image": + case "iframe": + case "text": + case "selection": + return getPolygonShape(element); + case "arrow": + case "line": { + const roughShape = + ShapeCache.get(element)?.[0] ?? + ShapeCache.generateElementShape(element, null)[0]; + const [, , , , cx, cy] = getElementAbsoluteCoords(element, elementsMap); + + return shouldTestInside(element) + ? getClosedCurveShape( + element, + roughShape, + [element.x, element.y], + element.angle, + [cx, cy], + ) + : getCurveShape(roughShape, [element.x, element.y], element.angle, [ + cx, + cy, + ]); + } + + case "ellipse": + return getEllipseShape(element); + + case "freedraw": { + const [, , , , cx, cy] = getElementAbsoluteCoords(element, elementsMap); + return getFreedrawShape(element, [cx, cy], shouldTestInside(element)); + } + } +}; diff --git a/packages/excalidraw/types.ts b/packages/excalidraw/types.ts index cb428f695d..5c2e188514 100644 --- a/packages/excalidraw/types.ts +++ b/packages/excalidraw/types.ts @@ -614,7 +614,6 @@ export type AppClassProperties = { setOpenDialog: App["setOpenDialog"]; insertEmbeddableElement: App["insertEmbeddableElement"]; onMagicframeToolSelect: App["onMagicframeToolSelect"]; - getElementShape: App["getElementShape"]; getName: App["getName"]; };