fix: Add binding update to manual stat changes (#8183)

Manual stats changes now respect previous element bindings.
This commit is contained in:
Márk Tolmács 2024-07-01 09:45:31 +02:00 committed by GitHub
parent 04668d8263
commit 66a2f24296
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 327 additions and 162 deletions

View file

@ -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<AppProps, AppState> {
nonDeletedElementsMap,
),
),
this,
this.scene.getNonDeletedElementsMap(),
);
}
@ -4008,7 +4000,7 @@ class App extends React.Component<AppProps, AppState> {
this.setState({
suggestedBindings: getSuggestedBindingsForArrows(
selectedElements,
this,
this.scene.getNonDeletedElementsMap(),
),
});
@ -4179,7 +4171,7 @@ class App extends React.Component<AppProps, AppState> {
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<AppProps, AppState> {
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<AppProps, AppState> {
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<AppProps, AppState> {
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<AppProps, AppState> {
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<AppProps, AppState> {
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<AppProps, AppState> {
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<AppProps, AppState> {
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<AppProps, AppState> {
const boundElement = getHoveredElementForBinding(
pointerDownState.origin,
this,
this.scene.getNonDeletedElementsMap(),
);
this.scene.insertElement(element);
this.setState({
@ -7070,7 +7027,7 @@ class App extends React.Component<AppProps, AppState> {
});
const boundElement = getHoveredElementForBinding(
pointerDownState.origin,
this,
this.scene.getNonDeletedElementsMap(),
);
this.scene.insertElement(element);
@ -7540,7 +7497,7 @@ class App extends React.Component<AppProps, AppState> {
this.setState({
suggestedBindings: getSuggestedBindingsForArrows(
selectedElements,
this,
this.scene.getNonDeletedElementsMap(),
),
});
@ -8061,7 +8018,7 @@ class App extends React.Component<AppProps, AppState> {
draggingElement,
this.state,
pointerCoords,
this,
this.scene.getNonDeletedElementsMap(),
);
}
this.setState({ suggestedBindings: [], startBoundElement: null });
@ -8551,7 +8508,10 @@ class App extends React.Component<AppProps, AppState> {
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<AppProps, AppState> {
bindOrUnbindLinearElements(
linearElements,
this,
this.scene.getNonDeletedElementsMap(),
isBindingEnabled(this.state),
this.state.selectedLinearElement?.selectedPointsIndices ?? [],
);
@ -9107,7 +9067,7 @@ class App extends React.Component<AppProps, AppState> {
}): void => {
const hoveredBindableElement = getHoveredElementForBinding(
pointerCoords,
this,
this.scene.getNonDeletedElementsMap(),
);
this.setState({
suggestedBindings:
@ -9134,7 +9094,7 @@ class App extends React.Component<AppProps, AppState> {
(acc: NonDeleted<ExcalidrawBindableElement>[], coords) => {
const hoveredBindableElement = getHoveredElementForBinding(
coords,
this,
this.scene.getNonDeletedElementsMap(),
);
if (
hoveredBindableElement != null &&
@ -9666,7 +9626,7 @@ class App extends React.Component<AppProps, AppState> {
) {
const suggestedBindings = getSuggestedBindingsForArrows(
selectedElements,
this,
this.scene.getNonDeletedElementsMap(),
);
const elementsToHighlight = new Set<ExcalidrawElement>();

View file

@ -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<AngleProps["property"]> = ({
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<AngleProps["property"]> = ({
mutateElement(latestElement, {
angle: nextAngle,
});
updateBindings(latestElement, elementsMap);
const boundTextElement = getBoundTextElement(latestElement, elementsMap);
if (boundTextElement && !isArrowElement(latestElement)) {

View file

@ -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

View file

@ -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;

View file

@ -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(<Excalidraw handleKeyboardGlobally={true} />);
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 () => {

View file

@ -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);
}
};