fix: Elbow arrow fixedpoint flipping now properly flips on inverted resize and flip action (#8324)

* Flipping action now properly mirrors selections with elbow arrows
* Flipping action now re-centers the selection to the original center to avoid "walking" selections on repeated flipping
pull/8536/head
Márk Tolmács 6 months ago committed by GitHub
parent 44a1c8d857
commit f3f0ab7c83
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,89 @@
import React from "react";
import { Excalidraw } from "../index";
import { render } from "../tests/test-utils";
import { API } from "../tests/helpers/api";
import { point } from "../../math";
import { actionFlipHorizontal } from "./actionFlip";
const { h } = window;
const testElements = [
API.createElement({
type: "rectangle",
id: "rec1",
x: 1046,
y: 541,
width: 100,
height: 100,
boundElements: [
{
id: "arr",
type: "arrow",
},
],
}),
API.createElement({
type: "rectangle",
id: "rec2",
x: 1169,
y: 777,
width: 102,
height: 115,
boundElements: [
{
id: "arr",
type: "arrow",
},
],
}),
API.createElement({
type: "arrow",
id: "arrow",
x: 1103.0717787616313,
y: 536.8531862198708,
width: 159.68539325842903,
height: 333.0396003698186,
startBinding: {
elementId: "rec1",
focus: 0.1366906474820229,
gap: 5.000000000000057,
fixedPoint: [0.5683453237410123, -0.05014327585315258],
},
endBinding: {
elementId: "rec2",
focus: 0.0014925373134265828,
gap: 5,
fixedPoint: [-0.04862325174825108, 0.4992537313432874],
},
points: [
point(0, 0),
point(0, -35),
point(-97.80898876404626, -35),
point(-97.80898876404626, 298.0396003698186),
point(61.87640449438277, 298.0396003698186),
],
elbowed: true,
}),
];
describe("flipping action", () => {
it("flip re-centers the selection even after multiple flip actions", async () => {
await render(<Excalidraw initialData={{ elements: testElements }} />);
API.setSelectedElements(testElements);
expect(Object.keys(h.state.selectedElementIds).length).toBe(3);
API.executeAction(actionFlipHorizontal);
API.executeAction(actionFlipHorizontal);
API.executeAction(actionFlipHorizontal);
const rec1 = h.elements.find((el) => el.id === "rec1");
expect(rec1?.x).toBeCloseTo(1113.78, 0);
expect(rec1?.y).toBeCloseTo(541, 0);
const rec2 = h.elements.find((el) => el.id === "rec2");
expect(rec2?.x).toBeCloseTo(988.72, 0);
expect(rec2?.y).toBeCloseTo(777, 0);
});
});

@ -2,6 +2,7 @@ import { register } from "./register";
import { getSelectedElements } from "../scene";
import { getNonDeletedElements } from "../element";
import type {
ExcalidrawElbowArrowElement,
ExcalidrawElement,
NonDeleted,
NonDeletedSceneElementsMap,
@ -18,7 +19,9 @@ import {
import { updateFrameMembershipOfSelectedElements } from "../frame";
import { flipHorizontal, flipVertical } from "../components/icons";
import { StoreAction } from "../store";
import { isLinearElement } from "../element/typeChecks";
import { isElbowArrow, isLinearElement } from "../element/typeChecks";
import { mutateElbowArrow } from "../element/routing";
import { mutateElement } from "../element/mutateElement";
export const actionFlipHorizontal = register({
name: "flipHorizontal",
@ -109,7 +112,8 @@ const flipElements = (
flipDirection: "horizontal" | "vertical",
app: AppClassProperties,
): ExcalidrawElement[] => {
const { minX, minY, maxX, maxY } = getCommonBoundingBox(selectedElements);
const { minX, minY, maxX, maxY, midX, midY } =
getCommonBoundingBox(selectedElements);
resizeMultipleElements(
elementsMap,
@ -131,5 +135,48 @@ const flipElements = (
[],
);
// ---------------------------------------------------------------------------
// flipping arrow elements (and potentially other) makes the selection group
// "move" across the canvas because of how arrows can bump against the "wall"
// of the selection, so we need to center the group back to the original
// position so that repeated flips don't accumulate the offset
const { elbowArrows, otherElements } = selectedElements.reduce(
(
acc: {
elbowArrows: ExcalidrawElbowArrowElement[];
otherElements: ExcalidrawElement[];
},
element,
) =>
isElbowArrow(element)
? { ...acc, elbowArrows: acc.elbowArrows.concat(element) }
: { ...acc, otherElements: acc.otherElements.concat(element) },
{ elbowArrows: [], otherElements: [] },
);
const { midX: newMidX, midY: newMidY } =
getCommonBoundingBox(selectedElements);
const [diffX, diffY] = [midX - newMidX, midY - newMidY];
otherElements.forEach((element) =>
mutateElement(element, {
x: element.x + diffX,
y: element.y + diffY,
}),
);
elbowArrows.forEach((element) =>
mutateElbowArrow(
element,
elementsMap,
element.points,
undefined,
undefined,
{
informMutation: false,
},
),
);
// ---------------------------------------------------------------------------
return selectedElements;
};

@ -1685,19 +1685,6 @@ export const actionChangeArrowType = register({
: {}),
},
);
} else {
mutateElement(
newElement,
{
startBinding: newElement.startBinding
? { ...newElement.startBinding, fixedPoint: null }
: null,
endBinding: newElement.endBinding
? { ...newElement.endBinding, fixedPoint: null }
: null,
},
false,
);
}
return newElement;

@ -185,6 +185,7 @@ import type {
MagicGenerationData,
ExcalidrawNonSelectionElement,
ExcalidrawArrowElement,
NonDeletedSceneElementsMap,
} from "../element/types";
import { getCenter, getDistance } from "../gesture";
import {
@ -287,6 +288,7 @@ import {
getDateTime,
isShallowEqual,
arrayToMap,
toBrandedType,
} from "../utils";
import {
createSrcDoc,
@ -3109,22 +3111,44 @@ class App extends React.Component<AppProps, AppState> {
retainSeed?: boolean;
fitToContent?: boolean;
}) => {
let elements = opts.elements.map((el) =>
isElbowArrow(el)
? {
...el,
...updateElbowArrow(
{
...el,
startBinding: null,
endBinding: null,
},
this.scene.getNonDeletedElementsMap(),
[el.points[0], el.points[el.points.length - 1]],
let elements = opts.elements.map((el, _, elements) => {
if (isElbowArrow(el)) {
const startEndElements = [
el.startBinding &&
elements.find((l) => l.id === el.startBinding?.elementId),
el.endBinding &&
elements.find((l) => l.id === el.endBinding?.elementId),
];
const startBinding = startEndElements[0] ? el.startBinding : null;
const endBinding = startEndElements[1] ? el.endBinding : null;
return {
...el,
...updateElbowArrow(
{
...el,
startBinding,
endBinding,
},
toBrandedType<NonDeletedSceneElementsMap>(
new Map(
startEndElements
.filter((x) => x != null)
.map(
(el) =>
[el!.id, el] as [
string,
Ordered<NonDeletedExcalidrawElement>,
],
),
),
),
}
: el,
);
[el.points[0], el.points[el.points.length - 1]],
),
};
}
return el;
});
elements = restoreElements(elements, null, undefined);
const [minX, minY, maxX, maxY] = getCommonBounds(elements);

@ -5,6 +5,7 @@ import type {
ExcalidrawLinearElement,
ExcalidrawSelectionElement,
ExcalidrawTextElement,
FixedPointBinding,
FontFamilyValues,
OrderedExcalidrawElement,
PointBinding,
@ -21,6 +22,7 @@ import {
import {
isArrowElement,
isElbowArrow,
isFixedPointBinding,
isLinearElement,
isTextElement,
isUsingAdaptiveRadius,
@ -101,8 +103,8 @@ const getFontFamilyByName = (fontFamilyName: string): FontFamilyValues => {
const repairBinding = (
element: ExcalidrawLinearElement,
binding: PointBinding | null,
): PointBinding | null => {
binding: PointBinding | FixedPointBinding | null,
): PointBinding | FixedPointBinding | null => {
if (!binding) {
return null;
}
@ -110,9 +112,11 @@ const repairBinding = (
return {
...binding,
focus: binding.focus || 0,
fixedPoint: isElbowArrow(element)
? normalizeFixedPoint(binding.fixedPoint ?? [0, 0])
: null,
...(isElbowArrow(element) && isFixedPointBinding(binding)
? {
fixedPoint: normalizeFixedPoint(binding.fixedPoint ?? [0, 0]),
}
: {}),
};
};

@ -39,6 +39,7 @@ import {
isBindingElement,
isBoundToContainer,
isElbowArrow,
isFixedPointBinding,
isFrameLikeElement,
isLinearElement,
isRectangularElement,
@ -797,7 +798,7 @@ export const bindPointToSnapToElementOutline = (
isVertical
? Math.abs(p[1] - i[1]) < 0.1
: Math.abs(p[0] - i[0]) < 0.1,
)[0] ?? point;
)[0] ?? p;
}
return p;
@ -1013,7 +1014,7 @@ const updateBoundPoint = (
const direction = startOrEnd === "startBinding" ? -1 : 1;
const edgePointIndex = direction === -1 ? 0 : linearElement.points.length - 1;
if (isElbowArrow(linearElement)) {
if (isElbowArrow(linearElement) && isFixedPointBinding(binding)) {
const fixedPoint =
normalizeFixedPoint(binding.fixedPoint) ??
calculateFixedPointForElbowArrowBinding(

@ -35,7 +35,6 @@ export const dragSelectedElements = (
) => {
if (
_selectedElements.length === 1 &&
isArrowElement(_selectedElements[0]) &&
isElbowArrow(_selectedElements[0]) &&
(_selectedElements[0].startBinding || _selectedElements[0].endBinding)
) {
@ -43,13 +42,7 @@ export const dragSelectedElements = (
}
const selectedElements = _selectedElements.filter(
(el) =>
!(
isArrowElement(el) &&
isElbowArrow(el) &&
el.startBinding &&
el.endBinding
),
(el) => !(isElbowArrow(el) && el.startBinding && el.endBinding),
);
// we do not want a frame and its elements to be selected at the same time

@ -102,6 +102,7 @@ export class LinearElementEditor {
public readonly endBindingElement: ExcalidrawBindableElement | null | "keep";
public readonly hoverPointIndex: number;
public readonly segmentMidPointHoveredCoords: GlobalPoint | null;
public readonly elbowed: boolean;
constructor(element: NonDeleted<ExcalidrawLinearElement>) {
this.elementId = element.id as string & {
@ -131,6 +132,7 @@ export class LinearElementEditor {
};
this.hoverPointIndex = -1;
this.segmentMidPointHoveredCoords = null;
this.elbowed = isElbowArrow(element) && element.elbowed;
}
// ---------------------------------------------------------------------------
@ -1477,7 +1479,9 @@ export class LinearElementEditor {
nextPoints,
vector(offsetX, offsetY),
bindings,
options,
{
isDragging: options?.isDragging,
},
);
} else {
const nextCoords = getElementPointsCoords(element, nextPoints);

@ -9,6 +9,7 @@ import type {
ExcalidrawTextElementWithContainer,
ExcalidrawImageElement,
ElementsMap,
ExcalidrawArrowElement,
NonDeletedSceneElementsMap,
SceneElementsMap,
} from "./types";
@ -909,6 +910,8 @@ export const resizeMultipleElements = (
fontSize?: ExcalidrawTextElement["fontSize"];
scale?: ExcalidrawImageElement["scale"];
boundTextFontSize?: ExcalidrawTextElement["fontSize"];
startBinding?: ExcalidrawArrowElement["startBinding"];
endBinding?: ExcalidrawArrowElement["endBinding"];
};
}[] = [];
@ -993,19 +996,6 @@ export const resizeMultipleElements = (
mutateElement(element, update, false);
if (isArrowElement(element) && isElbowArrow(element)) {
mutateElbowArrow(
element,
elementsMap,
element.points,
undefined,
undefined,
{
informMutation: false,
},
);
}
updateBoundElements(element, elementsMap, {
simultaneouslyUpdated: elementsToUpdate,
oldSize: { width: oldWidth, height: oldHeight },
@ -1059,7 +1049,7 @@ const rotateMultipleElements = (
(centerAngle + origAngle - element.angle) as Radians,
);
if (isArrowElement(element) && isElbowArrow(element)) {
if (isElbowArrow(element)) {
const points = getArrowLocalFixedPoints(element, elementsMap);
mutateElbowArrow(element, elementsMap, points);
} else {

@ -41,7 +41,6 @@ import { mutateElement } from "./mutateElement";
import { isBindableElement, isRectanguloidElement } from "./typeChecks";
import type {
ExcalidrawElbowArrowElement,
FixedPointBinding,
NonDeletedSceneElementsMap,
SceneElementsMap,
} from "./types";
@ -73,13 +72,12 @@ export const mutateElbowArrow = (
elementsMap: NonDeletedSceneElementsMap | SceneElementsMap,
nextPoints: readonly LocalPoint[],
offset?: Vector,
otherUpdates?: {
startBinding?: FixedPointBinding | null;
endBinding?: FixedPointBinding | null;
},
otherUpdates?: Omit<
ElementUpdate<ExcalidrawElbowArrowElement>,
"angle" | "x" | "y" | "width" | "height" | "elbowed" | "points"
>,
options?: {
isDragging?: boolean;
disableBinding?: boolean;
informMutation?: boolean;
},
) => {

@ -320,9 +320,12 @@ export const getDefaultRoundnessTypeForElement = (
};
export const isFixedPointBinding = (
binding: PointBinding,
binding: PointBinding | FixedPointBinding,
): binding is FixedPointBinding => {
return binding.fixedPoint != null;
return (
Object.hasOwn(binding, "fixedPoint") &&
(binding as FixedPointBinding).fixedPoint != null
);
};
// TODO: Move this to @excalidraw/math

@ -193,6 +193,7 @@ export type ExcalidrawElement =
| ExcalidrawGenericElement
| ExcalidrawTextElement
| ExcalidrawLinearElement
| ExcalidrawArrowElement
| ExcalidrawFreeDrawElement
| ExcalidrawImageElement
| ExcalidrawFrameElement
@ -268,15 +269,19 @@ export type PointBinding = {
elementId: ExcalidrawBindableElement["id"];
focus: number;
gap: number;
// Represents the fixed point binding information in form of a vertical and
// horizontal ratio (i.e. a percentage value in the 0.0-1.0 range). This ratio
// gives the user selected fixed point by multiplying the bound element width
// with fixedPoint[0] and the bound element height with fixedPoint[1] to get the
// bound element-local point coordinate.
fixedPoint: FixedPoint | null;
};
export type FixedPointBinding = Merge<PointBinding, { fixedPoint: FixedPoint }>;
export type FixedPointBinding = Merge<
PointBinding,
{
// Represents the fixed point binding information in form of a vertical and
// horizontal ratio (i.e. a percentage value in the 0.0-1.0 range). This ratio
// gives the user selected fixed point by multiplying the bound element width
// with fixedPoint[0] and the bound element height with fixedPoint[1] to get the
// bound element-local point coordinate.
fixedPoint: FixedPoint;
}
>;
export type Arrowhead =
| "arrow"

@ -52,7 +52,6 @@ import {
} from "./helpers";
import oc from "open-color";
import {
isArrowElement,
isElbowArrow,
isFrameLikeElement,
isLinearElement,
@ -807,7 +806,6 @@ const _renderInteractiveScene = ({
// Elbow arrow elements cannot be selected when bound on either end
(
isSingleLinearElementSelected &&
isArrowElement(element) &&
isElbowArrow(element) &&
(element.startBinding || element.endBinding)
)

@ -8430,6 +8430,7 @@ exports[`regression tests > key 5 selects arrow tool > [end of test] appState 1`
"selectedElementsAreBeingDragged": false,
"selectedGroupIds": {},
"selectedLinearElement": LinearElementEditor {
"elbowed": false,
"elementId": "id0",
"endBindingElement": "keep",
"hoverPointIndex": -1,
@ -8649,6 +8650,7 @@ exports[`regression tests > key 6 selects line tool > [end of test] appState 1`]
"selectedElementsAreBeingDragged": false,
"selectedGroupIds": {},
"selectedLinearElement": LinearElementEditor {
"elbowed": false,
"elementId": "id0",
"endBindingElement": "keep",
"hoverPointIndex": -1,
@ -9058,6 +9060,7 @@ exports[`regression tests > key a selects arrow tool > [end of test] appState 1`
"selectedElementsAreBeingDragged": false,
"selectedGroupIds": {},
"selectedLinearElement": LinearElementEditor {
"elbowed": false,
"elementId": "id0",
"endBindingElement": "keep",
"hoverPointIndex": -1,
@ -9454,6 +9457,7 @@ exports[`regression tests > key l selects line tool > [end of test] appState 1`]
"selectedElementsAreBeingDragged": false,
"selectedGroupIds": {},
"selectedLinearElement": LinearElementEditor {
"elbowed": false,
"elementId": "id0",
"endBindingElement": "keep",
"hoverPointIndex": -1,

@ -9,6 +9,8 @@ import type {
ExcalidrawFrameElement,
ExcalidrawElementType,
ExcalidrawMagicFrameElement,
ExcalidrawElbowArrowElement,
ExcalidrawArrowElement,
} from "../../element/types";
import { newElement, newTextElement, newLinearElement } from "../../element";
import { DEFAULT_VERTICAL_ALIGN, ROUNDNESS } from "../../constants";
@ -179,10 +181,10 @@ export class API {
scale?: T extends "image" ? ExcalidrawImageElement["scale"] : never;
status?: T extends "image" ? ExcalidrawImageElement["status"] : never;
startBinding?: T extends "arrow"
? ExcalidrawLinearElement["startBinding"]
? ExcalidrawArrowElement["startBinding"] | ExcalidrawElbowArrowElement["startBinding"]
: never;
endBinding?: T extends "arrow"
? ExcalidrawLinearElement["endBinding"]
? ExcalidrawArrowElement["endBinding"] | ExcalidrawElbowArrowElement["endBinding"]
: never;
elbowed?: boolean;
}): T extends "arrow" | "line"

@ -31,6 +31,7 @@ import type {
ExcalidrawGenericElement,
ExcalidrawLinearElement,
ExcalidrawTextElement,
FixedPointBinding,
FractionalIndex,
SceneElementsMap,
} from "../element/types";
@ -2049,13 +2050,13 @@ describe("history", () => {
focus: -0.001587301587301948,
gap: 5,
fixedPoint: [1.0318471337579618, 0.49920634920634904],
},
} as FixedPointBinding,
endBinding: {
elementId: "u2JGnnmoJ0VATV4vCNJE5",
focus: -0.0016129032258049847,
gap: 3.537079145500037,
fixedPoint: [0.4991935483870975, -0.03875193720914723],
},
} as FixedPointBinding,
},
],
storeAction: StoreAction.CAPTURE,
@ -4455,7 +4456,7 @@ describe("history", () => {
elements: [
h.elements[0],
newElementWith(h.elements[1], { boundElements: [] }),
newElementWith(h.elements[2] as ExcalidrawLinearElement, {
newElementWith(h.elements[2] as ExcalidrawElbowArrowElement, {
endBinding: {
elementId: remoteContainer.id,
gap: 1,
@ -4655,7 +4656,7 @@ describe("history", () => {
// Simulate remote update
API.updateScene({
elements: [
newElementWith(h.elements[0] as ExcalidrawLinearElement, {
newElementWith(h.elements[0] as ExcalidrawElbowArrowElement, {
startBinding: {
elementId: rect1.id,
gap: 1,

@ -4,6 +4,7 @@ import { render } from "./test-utils";
import { reseed } from "../random";
import { UI, Keyboard, Pointer } from "./helpers/ui";
import type {
ExcalidrawElbowArrowElement,
ExcalidrawFreeDrawElement,
ExcalidrawLinearElement,
} from "../element/types";
@ -333,6 +334,62 @@ describe("arrow element", () => {
expect(label.angle).toBeCloseTo(0);
expect(label.fontSize).toEqual(20);
});
it("flips the fixed point binding on negative resize for single bindable", () => {
const rectangle = UI.createElement("rectangle", {
x: -100,
y: -75,
width: 95,
height: 100,
});
UI.clickTool("arrow");
UI.clickOnTestId("elbow-arrow");
mouse.reset();
mouse.moveTo(-5, 0);
mouse.click();
mouse.moveTo(120, 200);
mouse.click();
const arrow = h.scene.getSelectedElements(
h.state,
)[0] as ExcalidrawElbowArrowElement;
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1.05);
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?.[1]).toBeCloseTo(0.75);
});
it("flips the fixed point binding on negative resize for group selection", () => {
const rectangle = UI.createElement("rectangle", {
x: -100,
y: -75,
width: 95,
height: 100,
});
UI.clickTool("arrow");
UI.clickOnTestId("elbow-arrow");
mouse.reset();
mouse.moveTo(-5, 0);
mouse.click();
mouse.moveTo(120, 200);
mouse.click();
const arrow = h.scene.getSelectedElements(
h.state,
)[0] as ExcalidrawElbowArrowElement;
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1.05);
expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75);
UI.resize([rectangle, arrow], "nw", [300, 350]);
expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(-0.144, 2);
expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.25);
});
});
describe("text element", () => {
@ -828,7 +885,6 @@ describe("multiple selection", () => {
expect(leftBoundArrow.endBinding?.elementId).toBe(
leftArrowBinding.elementId,
);
expect(leftBoundArrow.endBinding?.fixedPoint).toBeNull();
expect(leftBoundArrow.endBinding?.focus).toBe(leftArrowBinding.focus);
expect(rightBoundArrow.x).toBeCloseTo(210);
@ -843,7 +899,6 @@ describe("multiple selection", () => {
expect(rightBoundArrow.endBinding?.elementId).toBe(
rightArrowBinding.elementId,
);
expect(rightBoundArrow.endBinding?.fixedPoint).toBeNull();
expect(rightBoundArrow.endBinding?.focus).toBe(rightArrowBinding.focus);
});

Loading…
Cancel
Save