From 381f3ac70e5fecdbc58b99909ce69928fab648a3 Mon Sep 17 00:00:00 2001 From: "gowtham.selvaraj" Date: Fri, 2 May 2025 10:55:11 +0530 Subject: [PATCH] fix: proper deletion & sync of multi-point elements - Fixed synchronization issue for multi-point elements - Updated version checks in shouldDiscardRemoteElement --- .../excalidraw/actions/actionFinalize.tsx | 9 ++++--- packages/excalidraw/data/reconcile.ts | 26 +++++++++++++------ packages/excalidraw/data/restore.ts | 5 +++- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/packages/excalidraw/actions/actionFinalize.tsx b/packages/excalidraw/actions/actionFinalize.tsx index 22638ee91..4f56e11b8 100644 --- a/packages/excalidraw/actions/actionFinalize.tsx +++ b/packages/excalidraw/actions/actionFinalize.tsx @@ -105,10 +105,13 @@ export const actionFinalize = register({ } } - if (isInvisiblySmallElement(multiPointElement)) { + if ( + multiPointElement.points.length < 2 || + isInvisiblySmallElement(multiPointElement) + ) { // TODO: #7348 in theory this gets recorded by the store, so the invisible elements could be restored by the undo/redo, which might be not what we would want - newElements = newElements.filter( - (el) => el.id !== multiPointElement.id, + newElements = newElements.map((el) => + el.id === multiPointElement.id ? { ...el, isDeleted: true } : el, ); } diff --git a/packages/excalidraw/data/reconcile.ts b/packages/excalidraw/data/reconcile.ts index a69ee2dee..24e606c84 100644 --- a/packages/excalidraw/data/reconcile.ts +++ b/packages/excalidraw/data/reconcile.ts @@ -29,17 +29,27 @@ const shouldDiscardRemoteElement = ( local && // local element is being edited (local.id === localAppState.editingTextElement?.id || - local.id === localAppState.resizingElement?.id || - local.id === localAppState.newElement?.id || // TODO: Is this still valid? As newElement is selection element, which is never part of the elements array - // local element is newer - local.version > remote.version || - // resolve conflicting edits deterministically by taking the one with - // the lowest versionNonce - (local.version === remote.version && - local.versionNonce < remote.versionNonce)) + local.id === localAppState.resizingElement?.id) ) { return true; } + + if (local?.version !== undefined && remote.version !== undefined) { + if (remote.isDeleted && remote.version > local.version) { + return false; + } + if (local.isDeleted && !remote.isDeleted) { + return true; + } + + if (local.version > remote.version) { + return true; + } + if (local.version === remote.version) { + return local.versionNonce < remote.versionNonce; + } + } + return false; }; diff --git a/packages/excalidraw/data/restore.ts b/packages/excalidraw/data/restore.ts index cafc0bdd6..f967ff201 100644 --- a/packages/excalidraw/data/restore.ts +++ b/packages/excalidraw/data/restore.ts @@ -522,7 +522,10 @@ export const restoreElements = ( (elements || []).reduce((elements, element) => { // filtering out selection, which is legacy, no longer kept in elements, // and causing issues if retained - if (element.type !== "selection" && !isInvisiblySmallElement(element)) { + if ( + element.type !== "selection" && + (!isInvisiblySmallElement(element) || element.isDeleted) + ) { let migratedElement: ExcalidrawElement | null = restoreElement(element); if (migratedElement) { const localElement = localElementsMap?.get(element.id);