fix: Multi-point arrows and linears

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
This commit is contained in:
Mark Tolmacs
2025-09-04 15:03:22 +02:00
parent bcf3127fe5
commit 8a3ba853ab
35 changed files with 2061 additions and 1001 deletions

View File

@@ -278,7 +278,7 @@ const bindingStrategyForNewSimpleArrowEndpointDragging = (
// With new arrows it represents the continuous dragging of the end point
if (endDragged) {
const origin = appState?.selectedLinearElement?.pointerDownState.origin;
const origin = appState?.selectedLinearElement?.initialState.origin;
// Inside -> inside binding
if (hit && arrow.startBinding?.elementId === hit.id) {
@@ -309,7 +309,7 @@ const bindingStrategyForNewSimpleArrowEndpointDragging = (
invariant(otherElement, "Other element must be in the elements map");
const otherIsInsideBinding =
!!appState.selectedLinearElement?.pointerDownState.arrowStartIsInside;
!!appState.selectedLinearElement?.initialState.arrowStartIsInside;
const other: BindingStrategy = {
mode: otherIsInsideBinding ? "inside" : "orbit",

View File

@@ -1,7 +1,6 @@
import { toIterable } from "@excalidraw/common";
import { isInvisiblySmallElement } from "./sizeHelpers";
import { isLinearElementType } from "./typeChecks";
import type {
ExcalidrawElement,
@@ -52,27 +51,6 @@ export const isNonDeletedElement = <T extends ExcalidrawElement>(
element: T,
): element is NonDeleted<T> => !element.isDeleted;
const _clearElements = (
elements: readonly ExcalidrawElement[],
): ExcalidrawElement[] =>
getNonDeletedElements(elements).map((element) =>
isLinearElementType(element.type)
? { ...element, lastCommittedPoint: null }
: element,
);
export const clearElementsForDatabase = (
elements: readonly ExcalidrawElement[],
) => _clearElements(elements);
export const clearElementsForExport = (
elements: readonly ExcalidrawElement[],
) => _clearElements(elements);
export const clearElementsForLocalStorage = (
elements: readonly ExcalidrawElement[],
) => _clearElements(elements);
export * from "./align";
export * from "./binding";
export * from "./bounds";

View File

@@ -126,7 +126,7 @@ export class LinearElementEditor {
/** indices */
public readonly selectedPointsIndices: readonly number[] | null;
public readonly pointerDownState: Readonly<{
public readonly initialState: Readonly<{
prevSelectedPointsIndices: readonly number[] | null;
/** index */
lastClickedPoint: number;
@@ -142,6 +142,7 @@ export class LinearElementEditor {
/** whether you're dragging a point */
public readonly isDragging: boolean;
public readonly lastUncommittedPoint: LocalPoint | null;
public readonly lastCommittedPoint: LocalPoint | null;
public readonly pointerOffset: Readonly<{ x: number; y: number }>;
public readonly hoverPointIndex: number;
public readonly segmentMidPointHoveredCoords: GlobalPoint | null;
@@ -149,6 +150,11 @@ export class LinearElementEditor {
public readonly customLineAngle: number | null;
public readonly isEditing: boolean;
// @deprecated renamed to initialState because the data is used during linear
// element click creation as well (with multiple pointer down events)
// @ts-ignore
public readonly pointerDownState: never;
constructor(
element: NonDeleted<ExcalidrawLinearElement>,
elementsMap: ElementsMap,
@@ -167,9 +173,10 @@ export class LinearElementEditor {
}
this.selectedPointsIndices = null;
this.lastUncommittedPoint = null;
this.lastCommittedPoint = null;
this.isDragging = false;
this.pointerOffset = { x: 0, y: 0 };
this.pointerDownState = {
this.initialState = {
prevSelectedPointsIndices: null,
lastClickedPoint: -1,
origin: null,
@@ -396,25 +403,31 @@ export class LinearElementEditor {
): Pick<AppState, "suggestedBinding" | "selectedLinearElement"> | null {
const elementsMap = app.scene.getNonDeletedElementsMap();
const elements = app.scene.getNonDeletedElements();
const { elbowed, elementId, pointerDownState, selectedPointsIndices } =
const { elbowed, elementId, initialState, selectedPointsIndices } =
linearElementEditor;
const { lastClickedPoint } = pointerDownState;
const { lastClickedPoint } = initialState;
const element = LinearElementEditor.getElement(elementId, elementsMap);
invariant(element, "Element being dragged must exist in the scene");
invariant(element.points.length > 1, "Element must have at least 2 points");
invariant(
selectedPointsIndices,
"There must be selected points in order to drag them",
);
invariant(
lastClickedPoint > -1 && selectedPointsIndices.includes(lastClickedPoint),
"There must be a valid lastClickedPoint in order to drag it",
lastClickedPoint > -1 &&
selectedPointsIndices.includes(lastClickedPoint) &&
element.points[lastClickedPoint],
`There must be a valid lastClickedPoint in order to drag it. selectedPointsIndices(${JSON.stringify(
selectedPointsIndices,
)}) points(0..${
element.points.length - 1
}) lastClickedPoint(${lastClickedPoint})`,
);
invariant(element.points.length > 1, "Element must have at least 2 points");
invariant(
!elbowed ||
selectedPointsIndices?.filter(
@@ -551,8 +564,8 @@ export class LinearElementEditor {
const newLinearElementEditor = {
...linearElementEditor,
selectedPointsIndices: newSelectedPointsIndices,
pointerDownState: {
...linearElementEditor.pointerDownState,
initialState: {
...linearElementEditor.initialState,
lastClickedPoint: newLastClickedPoint,
},
segmentMidPointHoveredCoords: newSelectedMidPointHoveredCoords,
@@ -575,8 +588,12 @@ export class LinearElementEditor {
): LinearElementEditor {
const elementsMap = scene.getNonDeletedElementsMap();
const { elementId, selectedPointsIndices, isDragging, pointerDownState } =
editingLinearElement;
const {
elementId,
selectedPointsIndices,
isDragging,
initialState: pointerDownState,
} = editingLinearElement;
const element = LinearElementEditor.getElement(elementId, elementsMap);
if (!element) {
return editingLinearElement;
@@ -647,8 +664,8 @@ export class LinearElementEditor {
isDragging: false,
pointerOffset: { x: 0, y: 0 },
customLineAngle: null,
pointerDownState: {
...editingLinearElement.pointerDownState,
initialState: {
...editingLinearElement.initialState,
origin: null,
arrowStartIsInside: false,
},
@@ -948,7 +965,7 @@ export class LinearElementEditor {
store.scheduleCapture();
ret.linearElementEditor = {
...linearElementEditor,
pointerDownState: {
initialState: {
prevSelectedPointsIndices: linearElementEditor.selectedPointsIndices,
lastClickedPoint: -1,
origin: pointFrom<GlobalPoint>(scenePointer.x, scenePointer.y),
@@ -1009,7 +1026,7 @@ export class LinearElementEditor {
: null;
ret.linearElementEditor = {
...linearElementEditor,
pointerDownState: {
initialState: {
prevSelectedPointsIndices: linearElementEditor.selectedPointsIndices,
lastClickedPoint: clickedPointIndex,
origin: pointFrom<GlobalPoint>(scenePointer.x, scenePointer.y),
@@ -1082,19 +1099,16 @@ export class LinearElementEditor {
let newPoint: LocalPoint;
if (shouldRotateWithDiscreteAngle(event) && points.length >= 2) {
const lastCommittedPoint = points[points.length - 2];
const anchor = points[points.length - 2];
const [width, height] = LinearElementEditor._getShiftLockedDelta(
element,
elementsMap,
lastCommittedPoint,
anchor,
pointFrom(scenePointerX, scenePointerY),
event[KEYS.CTRL_OR_CMD] ? null : app.getEffectiveGridSize(),
);
newPoint = pointFrom(
width + lastCommittedPoint[0],
height + lastCommittedPoint[1],
);
newPoint = pointFrom(width + anchor[0], height + anchor[1]);
} else {
newPoint = LinearElementEditor.createPointAt(
element,
@@ -1530,18 +1544,18 @@ export class LinearElementEditor {
return false;
}
const { segmentMidpoint } = linearElementEditor.pointerDownState;
const { segmentMidpoint } = linearElementEditor.initialState;
if (
segmentMidpoint.added ||
segmentMidpoint.value === null ||
segmentMidpoint.index === null ||
linearElementEditor.pointerDownState.origin === null
linearElementEditor.initialState.origin === null
) {
return false;
}
const origin = linearElementEditor.pointerDownState.origin!;
const origin = linearElementEditor.initialState.origin!;
const dist = pointDistance(
origin,
pointFrom(pointerCoords.x, pointerCoords.y),
@@ -1570,12 +1584,12 @@ export class LinearElementEditor {
if (!element) {
return;
}
const { segmentMidpoint } = linearElementEditor.pointerDownState;
const { segmentMidpoint } = linearElementEditor.initialState;
const ret: {
pointerDownState: LinearElementEditor["pointerDownState"];
pointerDownState: LinearElementEditor["initialState"];
selectedPointsIndices: LinearElementEditor["selectedPointsIndices"];
} = {
pointerDownState: linearElementEditor.pointerDownState,
pointerDownState: linearElementEditor.initialState,
selectedPointsIndices: linearElementEditor.selectedPointsIndices,
};
@@ -1595,9 +1609,9 @@ export class LinearElementEditor {
scene.mutateElement(element, { points });
ret.pointerDownState = {
...linearElementEditor.pointerDownState,
...linearElementEditor.initialState,
segmentMidpoint: {
...linearElementEditor.pointerDownState.segmentMidpoint,
...linearElementEditor.initialState.segmentMidpoint,
added: true,
},
lastClickedPoint: segmentMidpoint.index!,
@@ -1915,7 +1929,7 @@ export class LinearElementEditor {
scene: Scene,
): Pick<
LinearElementEditor,
"segmentMidPointHoveredCoords" | "pointerDownState"
"segmentMidPointHoveredCoords" | "initialState"
> {
const elementsMap = scene.getNonDeletedElementsMap();
const element = LinearElementEditor.getElement(
@@ -1978,8 +1992,8 @@ export class LinearElementEditor {
return {
...linearElement,
segmentMidPointHoveredCoords: point,
pointerDownState: {
...linearElement.pointerDownState,
initialState: {
...linearElement.initialState,
segmentMidpoint: {
added: false,
index: element.fixedSegments![offset].index,

View File

@@ -452,7 +452,6 @@ export const newFreeDrawElement = (
points: opts.points || [],
pressures: opts.pressures || [],
simulatePressure: opts.simulatePressure,
lastCommittedPoint: null,
};
};
@@ -466,7 +465,7 @@ export const newLinearElement = (
const element = {
..._newElementBase<ExcalidrawLinearElement>(opts.type, opts),
points: opts.points || [],
lastCommittedPoint: null,
startBinding: null,
endBinding: null,
startArrowhead: null,
@@ -501,7 +500,6 @@ export const newArrowElement = <T extends boolean>(
return {
..._newElementBase<ExcalidrawElbowArrowElement>(opts.type, opts),
points: opts.points || [],
lastCommittedPoint: null,
startBinding: null,
endBinding: null,
startArrowhead: opts.startArrowhead || null,
@@ -516,7 +514,6 @@ export const newArrowElement = <T extends boolean>(
return {
..._newElementBase<ExcalidrawArrowElement>(opts.type, opts),
points: opts.points || [],
lastCommittedPoint: null,
startBinding: null,
endBinding: null,
startArrowhead: opts.startArrowhead || null,

View File

@@ -1103,7 +1103,7 @@ export function getFreeDrawSvgPath(element: ExcalidrawFreeDrawElement) {
smoothing: 0.5,
streamline: 0.5,
easing: (t) => Math.sin((t * Math.PI) / 2), // https://easings.net/#easeOutSine
last: !!element.lastCommittedPoint, // LastCommittedPoint is added on pointerup
last: true,
};
return getSvgPathFromStroke(getStroke(inputPoints as number[][], options));

View File

@@ -321,7 +321,6 @@ export type ExcalidrawLinearElement = _ExcalidrawElementBase &
Readonly<{
type: "line" | "arrow";
points: readonly LocalPoint[];
lastCommittedPoint: LocalPoint | null;
startBinding: FixedPointBinding | null;
endBinding: FixedPointBinding | null;
startArrowhead: Arrowhead | null;
@@ -378,7 +377,6 @@ export type ExcalidrawFreeDrawElement = _ExcalidrawElementBase &
points: readonly LocalPoint[];
pressures: readonly number[];
simulatePressure: boolean;
lastCommittedPoint: LocalPoint | null;
}>;
export type FileId = string & { _brand: "FileId" };

View File

@@ -44,3 +44,14 @@ exports[`Test Linear Elements > Test bound text element > should resize and posi
"Online whiteboard
collaboration made easy"
`;
exports[`Test Linear Elements > Test bound text element > should wrap the bound text when arrow bound container moves 1`] = `
"Online whiteboard
collaboration made easy"
`;
exports[`Test Linear Elements > Test bound text element > should wrap the bound text when arrow bound container moves 2`] = `
"Online whiteboard
collaboration made
easy"
`;

View File

@@ -257,8 +257,8 @@ describe("binding for simple arrows", () => {
const arrow = API.getSelectedElement() as ExcalidrawLinearElement;
expect(arrow.x).toBe(10);
expect(arrow.y).toBe(10);
expect(arrow.width).toBeCloseTo(86.4669660940663);
expect(arrow.height).toBeCloseTo(86.46696609406821);
expect(arrow.width).toBeCloseTo(85.75985931287957);
expect(arrow.height).toBeCloseTo(85.75985931288186);
// Should bind to the rectangle since endpoint is inside
expect(arrow.startBinding).toBe(null);
@@ -280,8 +280,8 @@ describe("binding for simple arrows", () => {
// Check if the arrow moved
expect(arrow.x).toBe(10);
expect(arrow.y).toBe(10);
expect(arrow.width).toBeCloseTo(235);
expect(arrow.height).toBeCloseTo(117.5);
expect(arrow.width).toBeCloseTo(234);
expect(arrow.height).toBeCloseTo(117);
// Restore bindable
mouse.reset();
@@ -309,49 +309,9 @@ describe("binding for simple arrows", () => {
expect(arrow.width).toBeCloseTo(86, 0);
expect(arrow.height).toBeCloseTo(86, 0);
});
it("should happen even if the arrow is not pointing at the element", () => {
// Create a rectangle positioned so the extended arrow segment will miss it
const rect = API.createElement({
type: "rectangle",
x: 100,
y: 100,
width: 100,
height: 100,
});
API.setElements([rect]);
// Draw an arrow that doesn't point at the rectangle (extended segment will miss)
UI.clickTool("arrow");
mouse.reset();
mouse.downAt(125, 93); // Start point
mouse.moveTo(175, 93); // End point - arrow direction is horizontal, misses rectangle
mouse.up();
const arrow = API.getSelectedElement() as ExcalidrawLinearElement;
// Should create a fixed point binding since the extended line segment
// from the last arrow segment misses the rectangle
expect(arrow.startBinding?.elementId).toBe(rect.id);
expect(arrow.startBinding).toHaveProperty("fixedPoint");
expect(
(arrow.startBinding as FixedPointBinding).fixedPoint[0],
).toBeGreaterThanOrEqual(0);
expect(
(arrow.startBinding as FixedPointBinding).fixedPoint[0],
).toBeLessThanOrEqual(1);
expect(
(arrow.startBinding as FixedPointBinding).fixedPoint[1],
).toBeLessThanOrEqual(0.5);
expect(
(arrow.startBinding as FixedPointBinding).fixedPoint[1],
).toBeLessThanOrEqual(1);
expect(arrow.endBinding).toBe(null);
});
});
describe("", () => {
describe("additional binding behavior", () => {
beforeEach(async () => {
mouse.reset();
@@ -411,8 +371,8 @@ describe("binding for simple arrows", () => {
const arrow = UI.createElement("arrow", {
x: 0,
y: 0,
size: 50,
y: 5,
size: 70,
});
expect(arrow.endBinding?.elementId).toBe(rectangle.id);
@@ -435,9 +395,9 @@ describe("binding for simple arrows", () => {
height: 500,
});
const arrow = UI.createElement("arrow", {
x: 210,
x: 190,
y: 250,
width: 180,
width: 220,
height: 1,
});
expect(arrow.startBinding?.elementId).toBe(rectLeft.id);
@@ -475,11 +435,11 @@ describe("binding for simple arrows", () => {
UI.clickTool("arrow");
mouse.reset();
mouse.clickAt(210, 250);
mouse.clickAt(190, 250);
mouse.moveTo(300, 200);
mouse.clickAt(300, 200);
mouse.moveTo(390, 251);
mouse.clickAt(390, 251);
mouse.moveTo(410, 251);
mouse.clickAt(410, 251);
const arrow = API.getSelectedElement() as ExcalidrawArrowElement;
@@ -517,9 +477,9 @@ describe("binding for simple arrows", () => {
height: 500,
});
const arrow = UI.createElement("arrow", {
x: 210,
x: 190,
y: 250,
width: 177,
width: 217,
height: 1,
});
expect(arrow.startBinding?.elementId).toBe(rectLeft.id);
@@ -668,7 +628,7 @@ describe("binding for simple arrows", () => {
const arrow = UI.createElement("arrow", {
x: 0,
y: 0,
size: 50,
size: 65,
});
expect(arrow.endBinding?.elementId).toBe(text.id);
@@ -693,7 +653,7 @@ describe("binding for simple arrows", () => {
it("should unbind on text element deletion by submitting empty text", async () => {
const text = API.createElement({
type: "text",
text: "ola",
text: "¡olá!",
x: 60,
y: 0,
width: 100,
@@ -705,7 +665,7 @@ describe("binding for simple arrows", () => {
const arrow = UI.createElement("arrow", {
x: 0,
y: 0,
size: 50,
size: 65,
});
expect(arrow.endBinding?.elementId).toBe(text.id);

View File

@@ -814,7 +814,7 @@ describe("duplication z-order", () => {
const arrow = UI.createElement("arrow", {
x: -100,
y: 50,
width: 95,
width: 115,
height: 0,
});

View File

@@ -131,6 +131,11 @@ describe("elbow arrow segment move", () => {
});
describe("elbow arrow routing", () => {
beforeEach(async () => {
localStorage.clear();
await render(<Excalidraw handleKeyboardGlobally={true} />);
});
it("can properly generate orthogonal arrow points", () => {
const scene = new Scene();
const arrow = API.createElement({
@@ -194,9 +199,9 @@ describe("elbow arrow routing", () => {
expect(arrow.points).toEqual([
[0, 0],
[45, 0],
[45, 200],
[90, 200],
[44, 0],
[44, 200],
[88, 200],
]);
});
});
@@ -235,9 +240,9 @@ describe("elbow arrow ui", () => {
expect(h.state.currentItemArrowType).toBe(ARROW_TYPE.elbow);
mouse.reset();
mouse.moveTo(-43, -99);
mouse.moveTo(-53, -99);
mouse.click();
mouse.moveTo(43, 99);
mouse.moveTo(53, 99);
mouse.click();
const arrow = h.scene.getSelectedElements(
@@ -248,9 +253,9 @@ describe("elbow arrow ui", () => {
expect(arrow.elbowed).toBe(true);
expect(arrow.points).toEqual([
[0, 0],
[45, 0],
[45, 200],
[90, 200],
[44, 0],
[44, 200],
[88, 200],
]);
});
@@ -272,9 +277,9 @@ describe("elbow arrow ui", () => {
UI.clickOnTestId("elbow-arrow");
mouse.reset();
mouse.moveTo(-43, -99);
mouse.moveTo(-53, -99);
mouse.click();
mouse.moveTo(43, 99);
mouse.moveTo(53, 99);
mouse.click();
const arrow = h.scene.getSelectedElements(
@@ -290,9 +295,11 @@ describe("elbow arrow ui", () => {
expect(arrow.points.map((point) => point.map(Math.round))).toEqual([
[0, 0],
[35, 0],
[35, 165],
[103, 165],
[36, 0],
[36, 90],
[28, 90],
[28, 164],
[101, 164],
]);
});
@@ -314,9 +321,9 @@ describe("elbow arrow ui", () => {
UI.clickOnTestId("elbow-arrow");
mouse.reset();
mouse.moveTo(-43, -99);
mouse.moveTo(-53, -99);
mouse.click();
mouse.moveTo(43, 99);
mouse.moveTo(53, 99);
mouse.click();
const arrow = h.scene.getSelectedElements(
@@ -346,9 +353,9 @@ describe("elbow arrow ui", () => {
expect(duplicatedArrow.elbowed).toBe(true);
expect(duplicatedArrow.points).toEqual([
[0, 0],
[45, 0],
[45, 200],
[90, 200],
[44, 0],
[44, 200],
[88, 200],
]);
expect(arrow.startBinding).not.toBe(null);
expect(arrow.endBinding).not.toBe(null);
@@ -372,9 +379,9 @@ describe("elbow arrow ui", () => {
UI.clickOnTestId("elbow-arrow");
mouse.reset();
mouse.moveTo(-43, -99);
mouse.moveTo(-53, -99);
mouse.click();
mouse.moveTo(43, 99);
mouse.moveTo(53, 99);
mouse.click();
const arrow = h.scene.getSelectedElements(
@@ -401,8 +408,8 @@ describe("elbow arrow ui", () => {
expect(duplicatedArrow.points).toEqual([
[0, 0],
[0, 100],
[90, 100],
[90, 200],
[88, 100],
[88, 200],
]);
});
});

View File

@@ -1303,7 +1303,7 @@ describe("Test Linear Elements", () => {
const arrow = UI.createElement("arrow", {
x: -10,
y: 250,
width: 400,
width: 410,
height: 1,
});
@@ -1316,7 +1316,7 @@ describe("Test Linear Elements", () => {
const textElement = h.elements[2] as ExcalidrawTextElementWithContainer;
expect(arrow.endBinding?.elementId).toBe(rect.id);
expect(arrow.width).toBeCloseTo(405);
expect(arrow.width).toBeCloseTo(404);
expect(rect.x).toBe(400);
expect(rect.y).toBe(0);
expect(
@@ -1335,7 +1335,7 @@ describe("Test Linear Elements", () => {
mouse.downAt(rect.x, rect.y);
mouse.moveTo(200, 0);
mouse.upAt(200, 0);
expect(arrow.width).toBeCloseTo(205);
expect(arrow.width).toBeCloseTo(204);
expect(rect.x).toBe(200);
expect(rect.y).toBe(0);
expect(handleBindTextResizeSpy).toHaveBeenCalledWith(

View File

@@ -510,12 +510,12 @@ describe("arrow element", () => {
h.state,
)[0] as ExcalidrawElbowArrowElement;
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1.05);
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1.06);
expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75);
UI.resize(rectangle, "se", [-200, -150]);
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1.05);
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1.06);
expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75);
});
@@ -538,11 +538,11 @@ describe("arrow element", () => {
h.state,
)[0] as ExcalidrawElbowArrowElement;
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1.05);
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1.06);
expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75);
UI.resize([rectangle, arrow], "nw", [300, 350]);
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(-0.05);
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(-0.06);
expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.25);
});
});
@@ -1350,8 +1350,8 @@ describe("multiple selection", () => {
expect(boundArrow.x).toBeCloseTo(380 * scaleX);
expect(boundArrow.y).toBeCloseTo(240 * scaleY);
expect(boundArrow.points[1][0]).toBeCloseTo(64.1246);
expect(boundArrow.points[1][1]).toBeCloseTo(-85.4995);
expect(boundArrow.points[1][0]).toBeCloseTo(66.3157);
expect(boundArrow.points[1][1]).toBeCloseTo(-88.421);
expect(arrowLabelPos.x + arrowLabel.width / 2).toBeCloseTo(
boundArrow.x + boundArrow.points[1][0] / 2,