feat: updates to test

This commit is contained in:
psmyrdek 2025-03-16 10:15:43 +01:00
parent 60ff21041c
commit 4c3516a5b9
3 changed files with 144 additions and 56 deletions

View file

@ -1131,7 +1131,9 @@ export class ElementsChange implements Change<SceneElementsMap> {
); );
// Need ordered nextElements to avoid z-index binding issues // Need ordered nextElements to avoid z-index binding issues
ElementsChange.redrawBoundArrows(nextElements, changedElements); ElementsChange.redrawBoundArrows(nextElements, changedElements, {
preservePoints: true,
});
} catch (e) { } catch (e) {
console.error( console.error(
`Couldn't mutate elements after applying elements change`, `Couldn't mutate elements after applying elements change`,
@ -1266,34 +1268,19 @@ export class ElementsChange implements Change<SceneElementsMap> {
(directlyApplicablePartial as any).points && (directlyApplicablePartial as any).points &&
!directlyApplicablePartial.isDeleted !directlyApplicablePartial.isDeleted
) { ) {
// Only update points if there's a binding change that would affect the arrow shape const oldStart = element.startBinding;
const oldEnd = element.endBinding;
const newStart = (directlyApplicablePartial as any).startBinding;
const newEnd = (directlyApplicablePartial as any).endBinding;
const startBindingChanged = const startBindingChanged =
(directlyApplicablePartial as any).startBinding !== undefined && JSON.stringify(oldStart || null) !== JSON.stringify(newStart || null);
JSON.stringify((directlyApplicablePartial as any).startBinding) !==
JSON.stringify(element.startBinding);
const endBindingChanged = const endBindingChanged =
(directlyApplicablePartial as any).endBinding !== undefined && JSON.stringify(oldEnd || null) !== JSON.stringify(newEnd || null);
JSON.stringify((directlyApplicablePartial as any).endBinding) !==
JSON.stringify(element.endBinding);
// If binding relationship changed significantly, we need to update points if (!startBindingChanged && !endBindingChanged) {
if (startBindingChanged || endBindingChanged) { delete (directlyApplicablePartial as any).points;
// Let the points be updated by the delta
return newElementWith(
element,
directlyApplicablePartial as ElementUpdate<typeof element>,
);
} }
// Otherwise preserve the original points
const partialWithoutPoints = { ...directlyApplicablePartial };
delete (partialWithoutPoints as any).points;
return newElementWith(
element,
partialWithoutPoints as ElementUpdate<typeof element>,
);
} }
return newElementWith(element, directlyApplicablePartial); return newElementWith(element, directlyApplicablePartial);
@ -1525,14 +1512,13 @@ export class ElementsChange implements Change<SceneElementsMap> {
private static redrawBoundArrows( private static redrawBoundArrows(
elements: SceneElementsMap, elements: SceneElementsMap,
changed: Map<string, OrderedExcalidrawElement>, changed: Map<string, OrderedExcalidrawElement>,
options?: { preservePoints?: boolean },
) { ) {
for (const element of changed.values()) { for (const element of changed.values()) {
if (!element.isDeleted && isBindableElement(element)) { if (!element.isDeleted && isBindableElement(element)) {
// Only preserve points during undo/redo when the binding relationship hasn't changed significantly
// This helps maintain arrow shape while allowing necessary updates when bindings change
updateBoundElements(element, elements, { updateBoundElements(element, elements, {
changedElements: changed, changedElements: changed,
preservePoints: true, // Preserve arrow points during undo/redo preservePoints: options?.preservePoints === true,
}); });
} }
} }

View file

@ -739,13 +739,13 @@ export const updateBoundElements = (
simultaneouslyUpdated?: readonly ExcalidrawElement[]; simultaneouslyUpdated?: readonly ExcalidrawElement[];
newSize?: { width: number; height: number }; newSize?: { width: number; height: number };
changedElements?: Map<string, OrderedExcalidrawElement>; changedElements?: Map<string, OrderedExcalidrawElement>;
preservePoints?: boolean; // Add this option to preserve arrow points during undo/redo preservePoints?: boolean; // [FIX] added
}, },
) => { ) => {
const { const {
newSize, newSize,
simultaneouslyUpdated, simultaneouslyUpdated,
preservePoints = false, preservePoints = false, // [FIX] default false
} = options ?? {}; } = options ?? {};
const simultaneouslyUpdatedElementIds = getSimultaneouslyUpdatedElementIds( const simultaneouslyUpdatedElementIds = getSimultaneouslyUpdatedElementIds(
simultaneouslyUpdated, simultaneouslyUpdated,
@ -760,12 +760,10 @@ export const updateBoundElements = (
return; return;
} }
// In case the boundElements are stale
if (!doesNeedUpdate(element, changedElement)) { if (!doesNeedUpdate(element, changedElement)) {
return; return;
} }
// Check for intersections before updating bound elements incase connected elements overlap
const startBindingElement = element.startBinding const startBindingElement = element.startBinding
? elementsMap.get(element.startBinding.elementId) ? elementsMap.get(element.startBinding.elementId)
: null; : null;
@ -793,36 +791,23 @@ export const updateBoundElements = (
), ),
}; };
// `linearElement` is being moved/scaled already, just update the binding
if (simultaneouslyUpdatedElementIds.has(element.id)) { if (simultaneouslyUpdatedElementIds.has(element.id)) {
mutateElement(element, bindings, true); mutateElement(element, bindings, true);
return; return;
} }
// If preservePoints is true, only update the bindings without changing the points // [FIX] If preservePoints is true, skip adjusting arrow geometry.
// This is specifically for undo/redo operations to maintain arrow shape
if (preservePoints && isArrowElement(element)) { if (preservePoints && isArrowElement(element)) {
// Only preserve points if the binding relationship hasn't changed // Only update the binding fields
const startBindingChanged = mutateElement(element, {
changedElement.id === element.startBinding?.elementId && ...(changedElement.id === element.startBinding?.elementId
bindings.startBinding !== element.startBinding; ? { startBinding: bindings.startBinding }
: {}),
const endBindingChanged = ...(changedElement.id === element.endBinding?.elementId
changedElement.id === element.endBinding?.elementId && ? { endBinding: bindings.endBinding }
bindings.endBinding !== element.endBinding; : {}),
});
// If binding relationship changed, we need to update points return;
if (!startBindingChanged && !endBindingChanged) {
mutateElement(element, {
...(changedElement.id === element.startBinding?.elementId
? { startBinding: bindings.startBinding }
: {}),
...(changedElement.id === element.endBinding?.elementId
? { endBinding: bindings.endBinding }
: {}),
});
return;
}
} }
const updates = bindableElementsVisitor( const updates = bindableElementsVisitor(

View file

@ -0,0 +1,117 @@
import React from "react";
import { pointFrom } from "@excalidraw/math";
import { Excalidraw } from "../index";
import { KEYS } from "../keys";
import { UI } from "./helpers/ui";
import { Keyboard, Pointer } from "./helpers/ui";
import { render } from "./test-utils";
import type { ExcalidrawElement } from "../element/types";
const mouse = new Pointer("mouse");
describe("arrow binding movement", () => {
beforeEach(async () => {
await render(<Excalidraw handleKeyboardGlobally={true} />);
});
it("should maintain arrow position when bound shape is moved", () => {
// Create a rectangle
const rect = UI.createElement("rectangle", {
x: 100,
y: 100,
width: 100,
height: 100,
});
// Create an arrow starting from rectangle border
const arrow = UI.createElement("arrow", {
x: 150,
y: 100,
width: 100,
height: 50,
points: [pointFrom(0, 0), pointFrom(100, 50)],
});
// Store original arrow position relative to rectangle
const originalArrowStart = [...arrow.points[0]];
const originalArrowEnd = [...arrow.points[1]];
const originalRelativeEndX = originalArrowEnd[0] - (rect.x - arrow.x);
const originalRelativeEndY = originalArrowEnd[1] - (rect.y - arrow.y);
// Move the rectangle
UI.clickTool("selection");
mouse.clickOn(rect);
mouse.downAt(rect.x + 50, rect.y + 50);
mouse.moveTo(rect.x + 100, rect.y + 50); // Move 50px to the right
mouse.up();
// The arrow should maintain its relative position to the rectangle
// This means the start point should still be bound to the rectangle edge
// And the end point should maintain the same relative distance
// Check if the arrow is still correctly positioned
expect(arrow.points[0]).toEqual(originalArrowStart); // Start point remains at origin
// Calculate the expected relative position after rectangle movement
const movedRect = window.h.elements.find(
(el: ExcalidrawElement) => el.id === rect.id,
)!;
const expectedRelativeEndX = originalRelativeEndX + (movedRect.x - rect.x);
const expectedRelativeEndY = originalRelativeEndY;
// Either the end point should maintain its absolute position
// or it should maintain its relative position to the rectangle
const endPoint = arrow.points[1];
const endPointMaintainsRelativePosition =
Math.abs(endPoint[0] - expectedRelativeEndX) < 1 &&
Math.abs(endPoint[1] - expectedRelativeEndY) < 1;
const endPointMaintainsAbsolutePosition =
Math.abs(endPoint[0] - originalArrowEnd[0]) < 1 &&
Math.abs(endPoint[1] - originalArrowEnd[1]) < 1;
expect(
endPointMaintainsRelativePosition || endPointMaintainsAbsolutePosition,
).toBe(true);
});
it("should restore arrow shape after undo", () => {
// Create a rectangle
const rect = UI.createElement("rectangle", {
x: 100,
y: 100,
width: 100,
height: 100,
});
// Create an arrow starting from rectangle border
const arrow = UI.createElement("arrow", {
x: 150,
y: 100,
width: 100,
height: 50,
points: [pointFrom(0, 0), pointFrom(100, 50)],
});
// Store original arrow points
const originalPoints = [...arrow.points];
// Move the rectangle
UI.clickTool("selection");
mouse.clickOn(rect);
mouse.downAt(rect.x + 50, rect.y + 50);
mouse.moveTo(rect.x + 100, rect.y + 50); // Move 50px to the right
mouse.up();
// Perform undo
Keyboard.withModifierKeys({ ctrl: true }, () => {
Keyboard.keyPress(KEYS.Z);
});
// Verify arrow points are exactly the same after undo
expect(arrow.points).toEqual(originalPoints);
});
});