diff --git a/packages/excalidraw/change.ts b/packages/excalidraw/change.ts index 492b7f79e..dd3c88db7 100644 --- a/packages/excalidraw/change.ts +++ b/packages/excalidraw/change.ts @@ -16,6 +16,7 @@ import { isBoundToContainer, isImageElement, isTextElement, + isArrowElement, } from "./element/typeChecks"; import { orderByFractionalIndex, syncMovedIndices } from "./fractionalIndex"; import { getNonDeletedGroupIds } from "./groups"; @@ -1493,13 +1494,73 @@ export class ElementsChange implements Change { elements: SceneElementsMap, changed: Map, ) { + // First, collect all arrow elements that need to be updated + const arrowsToUpdate = new Set(); + + // Check for bindable elements that were changed for (const element of changed.values()) { if (!element.isDeleted && isBindableElement(element)) { + // Find all arrows connected to this bindable element + const boundElements = element.boundElements || []; + for (const binding of boundElements) { + if (binding.type === "arrow") { + arrowsToUpdate.add(binding.id); + } + } + + // Update bound elements for this bindable element updateBoundElements(element, elements, { changedElements: changed, }); } } + + // Check for arrow elements that were changed + for (const element of changed.values()) { + if (!element.isDeleted && isArrowElement(element)) { + arrowsToUpdate.add(element.id); + } + } + + // Process all arrows that need updating + for (const arrowId of arrowsToUpdate) { + const arrowElement = elements.get(arrowId); + if ( + arrowElement && + isArrowElement(arrowElement) && + !arrowElement.isDeleted + ) { + // Cast to ExcalidrawLinearElement to access binding properties + const arrow = arrowElement as NonDeleted; + + // Make sure startBinding and endBinding are consistent + if (arrow.startBinding) { + const bindTarget = elements.get(arrow.startBinding.elementId); + if (!bindTarget || bindTarget.isDeleted) { + // If the target was deleted, remove the binding + mutateElement(arrow, { startBinding: null }); + } else { + // Ensure the bound element has this arrow in its boundElements + updateBoundElements(bindTarget, elements, { + simultaneouslyUpdated: [arrow], + }); + } + } + + if (arrow.endBinding) { + const bindTarget = elements.get(arrow.endBinding.elementId); + if (!bindTarget || bindTarget.isDeleted) { + // If the target was deleted, remove the binding + mutateElement(arrow, { endBinding: null }); + } else { + // Ensure the bound element has this arrow in its boundElements + updateBoundElements(bindTarget, elements, { + simultaneouslyUpdated: [arrow], + }); + } + } + } + } } private static reorderElements( diff --git a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap index 3f523d005..512e1c118 100644 --- a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap @@ -226,7 +226,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 40, + "version": 56, "width": "101.77517", "x": "0.70711", "y": 0, @@ -817,7 +817,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 30, + "version": 44, "width": 0, "x": "149.29289", "y": 0, @@ -1275,7 +1275,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 11, + "version": 15, "width": "98.58579", "x": "0.70711", "y": 0, @@ -1646,7 +1646,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 11, + "version": 15, "width": "98.58579", "x": "0.70711", "y": 0, @@ -2353,7 +2353,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 10, + "version": 12, "width": "502.78936", "x": "-0.83465", "y": "-36.58211", @@ -15149,7 +15149,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 10, + "version": 12, "width": "98.58579", "x": "0.70711", "y": 0, @@ -15846,7 +15846,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 10, + "version": 12, "width": "98.58579", "x": "0.70711", "y": 0, @@ -16465,7 +16465,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 10, + "version": 12, "width": "98.58579", "x": "0.70711", "y": 0, @@ -17082,7 +17082,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 10, + "version": 12, "width": "98.58579", "x": "0.70711", "y": 0, @@ -17795,7 +17795,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 11, + "version": 13, "width": "98.58579", "x": "0.70711", "y": 0, diff --git a/packages/excalidraw/tests/arrow-bindings.test.tsx b/packages/excalidraw/tests/arrow-bindings.test.tsx new file mode 100644 index 000000000..cfc4e1cd1 --- /dev/null +++ b/packages/excalidraw/tests/arrow-bindings.test.tsx @@ -0,0 +1,164 @@ +import { vi } from "vitest"; + +import { Excalidraw } from "../index"; + +import { API } from "./helpers/api"; +import { Keyboard, Pointer, UI } from "./helpers/ui"; +import { GlobalTestState, render, unmountComponent } from "./test-utils"; + +import type { + ExcalidrawLinearElement, + ExcalidrawRectangleElement, +} from "../element/types"; +import type { NormalizedZoomValue } from "../types"; + +const { h } = window; +const mouse = new Pointer("mouse"); + +describe("arrow bindings", () => { + beforeEach(async () => { + unmountComponent(); + + mouse.reset(); + localStorage.clear(); + sessionStorage.clear(); + vi.clearAllMocks(); + + Object.assign(document, { + elementFromPoint: () => GlobalTestState.canvas, + }); + await render(); + API.setAppState({ + zoom: { + value: 1 as NormalizedZoomValue, + }, + }); + }); + + it("should preserve arrow bindings after undo/redo", async () => { + const rect = UI.createElement("rectangle", { + x: 100, + y: 100, + width: 100, + height: 100, + }); + + UI.clickTool("arrow"); + mouse.downAt(50, 150); + mouse.moveTo(100, 150); + mouse.up(); + + const arrow = h.elements[1] as ExcalidrawLinearElement; + + expect(arrow.endBinding).toEqual( + expect.objectContaining({ + elementId: rect.id, + focus: expect.any(Number), + gap: expect.any(Number), + }), + ); + + expect(rect.boundElements).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: arrow.id, + type: "arrow", + }), + ]), + ); + + Keyboard.undo(); + + expect(h.elements).toEqual([ + expect.objectContaining({ id: rect.id, boundElements: [] }), + expect.objectContaining({ id: arrow.id, isDeleted: true }), + ]); + + Keyboard.redo(); + + expect(h.elements).toEqual([ + expect.objectContaining({ + id: rect.id, + boundElements: expect.arrayContaining([ + expect.objectContaining({ + id: arrow.id, + type: "arrow", + }), + ]), + }), + expect.objectContaining({ + id: arrow.id, + isDeleted: false, + endBinding: expect.objectContaining({ + elementId: rect.id, + focus: expect.any(Number), + gap: expect.any(Number), + }), + }), + ]); + }); + + it("should preserve arrow bindings after moving rectangle and undo/redo", async () => { + const rect = UI.createElement("rectangle", { + x: 100, + y: 100, + width: 100, + height: 100, + }); + + UI.clickTool("arrow"); + mouse.downAt(50, 150); + mouse.moveTo(100, 150); + mouse.up(); + + mouse.select(rect); + mouse.downAt(150, 150); + mouse.moveTo(250, 150); + mouse.up(); + + const movedArrow = h.elements[1] as ExcalidrawLinearElement; + const movedRect = h.elements[0] as ExcalidrawRectangleElement; + + expect(movedRect.x).toBe(200); + expect(movedArrow.endBinding).toEqual( + expect.objectContaining({ + elementId: rect.id, + focus: expect.any(Number), + gap: expect.any(Number), + }), + ); + + Keyboard.undo(); + + const undoRect = h.elements[0] as ExcalidrawRectangleElement; + const undoArrow = h.elements[1] as ExcalidrawLinearElement; + + expect(undoRect.x).toBe(100); + + expect(undoArrow.endBinding).toEqual( + expect.objectContaining({ + elementId: rect.id, + focus: expect.any(Number), + gap: expect.any(Number), + }), + ); + + Keyboard.redo(); + + const redoRect = h.elements[0] as ExcalidrawRectangleElement; + const redoArrow = h.elements[1] as ExcalidrawLinearElement; + + expect(redoRect.x).toBe(200); + + expect(redoArrow.endBinding).toEqual( + expect.objectContaining({ + elementId: rect.id, + focus: expect.any(Number), + gap: expect.any(Number), + }), + ); + + expect(redoRect.x).not.toEqual(undoRect.x); + expect(redoArrow.endBinding?.elementId).toEqual(rect.id); + }); +});