fix: Do not rebind undragged elbow arrow endpoint (#9191)

This commit is contained in:
Márk Tolmács 2025-03-10 16:25:33 +01:00 committed by GitHub
parent 4ec812bc18
commit d587b8a3de
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 90 additions and 60 deletions

View File

@ -73,6 +73,8 @@ import {
vectorCross, vectorCross,
pointsEqual, pointsEqual,
lineSegmentIntersectionPoints, lineSegmentIntersectionPoints,
round,
PRECISION,
} from "@excalidraw/math"; } from "@excalidraw/math";
import { intersectElementWithLineSegment } from "./collision"; import { intersectElementWithLineSegment } from "./collision";
import { distanceToBindableElement } from "./distance"; import { distanceToBindableElement } from "./distance";
@ -272,14 +274,16 @@ const getBindingStrategyForDraggingArrowEndpoints = (
zoom, zoom,
) )
: null // If binding is disabled and start is dragged, break all binds : 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 : !isElbowArrow(selectedElement)
? // We have to update the focus and gap of the binding, so let's rebind
getElligibleElementForBindingElement( getElligibleElementForBindingElement(
selectedElement, selectedElement,
"start", "start",
elementsMap, elementsMap,
elements, elements,
zoom, zoom,
); )
: "keep";
const end = endDragged const end = endDragged
? isBindingEnabled ? isBindingEnabled
? getElligibleElementForBindingElement( ? getElligibleElementForBindingElement(
@ -290,14 +294,16 @@ const getBindingStrategyForDraggingArrowEndpoints = (
zoom, zoom,
) )
: null // If binding is disabled and end is dragged, break all binds : 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 : !isElbowArrow(selectedElement)
? // We have to update the focus and gap of the binding, so let's rebind
getElligibleElementForBindingElement( getElligibleElementForBindingElement(
selectedElement, selectedElement,
"end", "end",
elementsMap, elementsMap,
elements, elements,
zoom, zoom,
); )
: "keep";
return [start, end]; return [start, end];
}; };
@ -309,6 +315,11 @@ const getBindingStrategyForDraggingArrowOrJoints = (
isBindingEnabled: boolean, isBindingEnabled: boolean,
zoom?: AppState["zoom"], zoom?: AppState["zoom"],
): (NonDeleted<ExcalidrawBindableElement> | null | "keep")[] => { ): (NonDeleted<ExcalidrawBindableElement> | null | "keep")[] => {
// Elbow arrows don't bind when dragged as a whole
if (isElbowArrow(selectedElement)) {
return ["keep", "keep"];
}
const [startIsClose, endIsClose] = getOriginalBindingsIfStillCloseToArrowEnds( const [startIsClose, endIsClose] = getOriginalBindingsIfStillCloseToArrowEnds(
selectedElement, selectedElement,
elementsMap, elementsMap,
@ -945,13 +956,17 @@ export const bindPointToSnapToElementOutline = (
const currentDistance = pointDistance(p, center); const currentDistance = pointDistance(p, center);
const fullDistance = Math.max( const fullDistance = Math.max(
pointDistance(intersection ?? p, center), pointDistance(intersection ?? p, center),
1e-5, PRECISION,
); );
const ratio = currentDistance / fullDistance; const ratio = round(currentDistance / fullDistance, 6);
switch (true) { switch (true) {
case ratio > 0.9: case ratio > 0.9:
if (currentDistance - fullDistance > FIXED_BINDING_DISTANCE) { if (
currentDistance - fullDistance > FIXED_BINDING_DISTANCE ||
// Too close to determine vector from intersection to p
pointDistanceSq(p, intersection) < PRECISION
) {
return p; return p;
} }

View File

@ -15,7 +15,7 @@ import {
import BinaryHeap from "../binaryheap"; import BinaryHeap from "../binaryheap";
import { getSizeFromPoints } from "../points"; import { getSizeFromPoints } from "../points";
import { aabbForElement, pointInsideBounds } from "../shapes"; import { aabbForElement, pointInsideBounds } from "../shapes";
import { invariant, isAnyTrue, toBrandedType, tupleToCoors } from "../utils"; import { invariant, isAnyTrue, tupleToCoors } from "../utils";
import type { AppState } from "../types"; import type { AppState } from "../types";
import { import {
bindPointToSnapToElementOutline, bindPointToSnapToElementOutline,
@ -52,6 +52,7 @@ import type {
ExcalidrawBindableElement, ExcalidrawBindableElement,
FixedPointBinding, FixedPointBinding,
FixedSegment, FixedSegment,
NonDeletedExcalidrawElement,
} from "./types"; } from "./types";
import { distanceToBindableElement } from "./distance"; import { distanceToBindableElement } from "./distance";
@ -101,7 +102,7 @@ export const BASE_PADDING = 40;
const handleSegmentRenormalization = ( const handleSegmentRenormalization = (
arrow: ExcalidrawElbowArrowElement, arrow: ExcalidrawElbowArrowElement,
elementsMap: NonDeletedSceneElementsMap | SceneElementsMap, elementsMap: NonDeletedSceneElementsMap,
) => { ) => {
const nextFixedSegments: FixedSegment[] | null = arrow.fixedSegments const nextFixedSegments: FixedSegment[] | null = arrow.fixedSegments
? arrow.fixedSegments.slice() ? arrow.fixedSegments.slice()
@ -234,6 +235,16 @@ const handleSegmentRenormalization = (
nextPoints.map((p) => nextPoints.map((p) =>
pointFrom<LocalPoint>(p[0] - arrow.x, p[1] - arrow.y), pointFrom<LocalPoint>(p[0] - arrow.x, p[1] - arrow.y),
), ),
arrow.startBinding &&
getBindableElementForId(
arrow.startBinding.elementId,
elementsMap,
),
arrow.endBinding &&
getBindableElementForId(
arrow.endBinding.elementId,
elementsMap,
),
), ),
) ?? [], ) ?? [],
), ),
@ -271,7 +282,7 @@ const handleSegmentRenormalization = (
const handleSegmentRelease = ( const handleSegmentRelease = (
arrow: ExcalidrawElbowArrowElement, arrow: ExcalidrawElbowArrowElement,
fixedSegments: readonly FixedSegment[], fixedSegments: readonly FixedSegment[],
elementsMap: NonDeletedSceneElementsMap | SceneElementsMap, elementsMap: NonDeletedSceneElementsMap,
) => { ) => {
const newFixedSegmentIndices = fixedSegments.map((segment) => segment.index); const newFixedSegmentIndices = fixedSegments.map((segment) => segment.index);
const oldFixedSegmentIndices = const oldFixedSegmentIndices =
@ -295,6 +306,8 @@ const handleSegmentRelease = (
// We need to render a sub-arrow path to restore deleted segments // We need to render a sub-arrow path to restore deleted segments
const x = arrow.x + (prevSegment ? prevSegment.end[0] : 0); const x = arrow.x + (prevSegment ? prevSegment.end[0] : 0);
const y = arrow.y + (prevSegment ? prevSegment.end[1] : 0); const y = arrow.y + (prevSegment ? prevSegment.end[1] : 0);
const startBinding = prevSegment ? null : arrow.startBinding;
const endBinding = nextSegment ? null : arrow.endBinding;
const { const {
startHeading, startHeading,
endHeading, endHeading,
@ -307,10 +320,11 @@ const handleSegmentRelease = (
{ {
x, x,
y, y,
startBinding: prevSegment ? null : arrow.startBinding, startBinding,
endBinding: nextSegment ? null : arrow.endBinding, endBinding,
startArrowhead: null, startArrowhead: null,
endArrowhead: null, endArrowhead: null,
points: arrow.points,
}, },
elementsMap, elementsMap,
[ [
@ -324,6 +338,9 @@ const handleSegmentRelease = (
y, y,
), ),
], ],
startBinding &&
getBindableElementForId(startBinding.elementId, elementsMap),
endBinding && getBindableElementForId(endBinding.elementId, elementsMap),
{ isDragging: false }, { isDragging: false },
); );
@ -870,7 +887,7 @@ const MAX_POS = 1e6;
*/ */
export const updateElbowArrowPoints = ( export const updateElbowArrowPoints = (
arrow: Readonly<ExcalidrawElbowArrowElement>, arrow: Readonly<ExcalidrawElbowArrowElement>,
elementsMap: NonDeletedSceneElementsMap | SceneElementsMap, elementsMap: NonDeletedSceneElementsMap,
updates: { updates: {
points?: readonly LocalPoint[]; points?: readonly LocalPoint[];
fixedSegments?: FixedSegment[] | null; fixedSegments?: FixedSegment[] | null;
@ -986,8 +1003,11 @@ export const updateElbowArrowPoints = (
typeof updates.endBinding !== "undefined" typeof updates.endBinding !== "undefined"
? updates.endBinding ? updates.endBinding
: arrow.endBinding; : arrow.endBinding;
const startElement = startBinding && elementsMap.get(startBinding.elementId); const startElement =
const endElement = endBinding && elementsMap.get(endBinding.elementId); startBinding &&
getBindableElementForId(startBinding.elementId, elementsMap);
const endElement =
endBinding && getBindableElementForId(endBinding.elementId, elementsMap);
if ( if (
(elementsMap.size === 0 && validateElbowPoints(updatedPoints)) || (elementsMap.size === 0 && validateElbowPoints(updatedPoints)) ||
startElement?.id !== startBinding?.elementId || startElement?.id !== startBinding?.elementId ||
@ -1019,9 +1039,12 @@ export const updateElbowArrowPoints = (
endBinding, endBinding,
startArrowhead: arrow.startArrowhead, startArrowhead: arrow.startArrowhead,
endArrowhead: arrow.endArrowhead, endArrowhead: arrow.endArrowhead,
points: arrow.points,
}, },
elementsMap, elementsMap,
updatedPoints, updatedPoints,
startElement,
endElement,
options, options,
); );
@ -1155,9 +1178,12 @@ const getElbowArrowData = (
endBinding: FixedPointBinding | null; endBinding: FixedPointBinding | null;
startArrowhead: Arrowhead | null; startArrowhead: Arrowhead | null;
endArrowhead: Arrowhead | null; endArrowhead: Arrowhead | null;
points: readonly LocalPoint[];
}, },
elementsMap: NonDeletedSceneElementsMap | SceneElementsMap, elementsMap: NonDeletedSceneElementsMap,
nextPoints: readonly LocalPoint[], nextPoints: readonly LocalPoint[],
startElement: ExcalidrawBindableElement | null,
endElement: ExcalidrawBindableElement | null,
options?: { options?: {
isDragging?: boolean; isDragging?: boolean;
zoom?: AppState["zoom"]; zoom?: AppState["zoom"];
@ -1171,20 +1197,27 @@ const getElbowArrowData = (
LocalPoint, LocalPoint,
GlobalPoint GlobalPoint
>(nextPoints[nextPoints.length - 1], vector(arrow.x, arrow.y)); >(nextPoints[nextPoints.length - 1], vector(arrow.x, arrow.y));
const startElement =
arrow.startBinding && let hoveredStartElement = startElement;
getBindableElementForId(arrow.startBinding.elementId, elementsMap); let hoveredEndElement = endElement;
const endElement = if (options?.isDragging) {
arrow.endBinding && const elements = Array.from(elementsMap.values());
getBindableElementForId(arrow.endBinding.elementId, elementsMap); hoveredStartElement =
const [hoveredStartElement, hoveredEndElement] = options?.isDragging getHoveredElement(
? getHoveredElements(
origStartGlobalPoint, origStartGlobalPoint,
elementsMap,
elements,
options?.zoom,
) || startElement;
hoveredEndElement =
getHoveredElement(
origEndGlobalPoint, origEndGlobalPoint,
elementsMap, elementsMap,
elements,
options?.zoom, options?.zoom,
) ) || endElement;
: [startElement, endElement]; }
const startGlobalPoint = getGlobalPoint( const startGlobalPoint = getGlobalPoint(
{ {
...arrow, ...arrow,
@ -2214,36 +2247,20 @@ const getBindPointHeading = (
origPoint, origPoint,
); );
const getHoveredElements = ( const getHoveredElement = (
origStartGlobalPoint: GlobalPoint, origPoint: GlobalPoint,
origEndGlobalPoint: GlobalPoint, elementsMap: NonDeletedSceneElementsMap,
elementsMap: NonDeletedSceneElementsMap | SceneElementsMap, elements: readonly NonDeletedExcalidrawElement[],
zoom?: AppState["zoom"], zoom?: AppState["zoom"],
) => { ) => {
// TODO: Might be a performance bottleneck and the Map type return getHoveredElementForBinding(
// remembers the insertion order anyway... tupleToCoors(origPoint),
const nonDeletedSceneElementsMap = toBrandedType<NonDeletedSceneElementsMap>( elements,
new Map([...elementsMap].filter((el) => !el[1].isDeleted)), elementsMap,
zoom,
true,
true,
); );
const elements = Array.from(elementsMap.values());
return [
getHoveredElementForBinding(
tupleToCoors(origStartGlobalPoint),
elements,
nonDeletedSceneElementsMap,
zoom,
true,
true,
),
getHoveredElementForBinding(
tupleToCoors(origEndGlobalPoint),
elements,
nonDeletedSceneElementsMap,
zoom,
true,
true,
),
];
}; };
const gridAddressesEqual = (a: GridAddress, b: GridAddress): boolean => const gridAddressesEqual = (a: GridAddress, b: GridAddress): boolean =>

View File

@ -10,7 +10,6 @@ import {
import { bindLinearElement } from "./binding"; import { bindLinearElement } from "./binding";
import { LinearElementEditor } from "./linearElementEditor"; import { LinearElementEditor } from "./linearElementEditor";
import { newArrowElement, newElement } from "./newElement"; import { newArrowElement, newElement } from "./newElement";
import type { SceneElementsMap } from "./types";
import { import {
type ElementsMap, type ElementsMap,
type ExcalidrawBindableElement, type ExcalidrawBindableElement,
@ -472,7 +471,7 @@ const createBindingArrow = (
const update = updateElbowArrowPoints( const update = updateElbowArrowPoints(
bindingArrow, bindingArrow,
toBrandedType<SceneElementsMap>( toBrandedType<NonDeletedSceneElementsMap>(
new Map([ new Map([
...elementsMap.entries(), ...elementsMap.entries(),
[startBindingElement.id, startBindingElement], [startBindingElement.id, startBindingElement],

View File

@ -1,4 +1,4 @@
import type { ExcalidrawElement, SceneElementsMap } from "./types"; import type { ExcalidrawElement, NonDeletedSceneElementsMap } from "./types";
import Scene from "../scene/Scene"; import Scene from "../scene/Scene";
import { getSizeFromPoints } from "../points"; import { getSizeFromPoints } from "../points";
import { randomInteger } from "../random"; import { randomInteger } from "../random";
@ -44,7 +44,7 @@ export const mutateElement = <TElement extends Mutable<ExcalidrawElement>>(
typeof startBinding !== "undefined" || typeof startBinding !== "undefined" ||
typeof endBinding !== "undefined") // manual binding to element typeof endBinding !== "undefined") // manual binding to element
) { ) {
const elementsMap = toBrandedType<SceneElementsMap>( const elementsMap = toBrandedType<NonDeletedSceneElementsMap>(
Scene.getScene(element)?.getNonDeletedElementsMap() ?? new Map(), Scene.getScene(element)?.getNonDeletedElementsMap() ?? new Map(),
); );

View File

@ -533,9 +533,8 @@ describe("arrow element", () => {
expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75); expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75);
UI.resize([rectangle, arrow], "nw", [300, 350]); UI.resize([rectangle, arrow], "nw", [300, 350]);
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(0);
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(-0.13); expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.25);
expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.11);
}); });
}); });