From 74b8f7ee26d78fb22797bb981a394d486eb1409a Mon Sep 17 00:00:00 2001 From: Shivansh Kumar <104713531+ShivanshKumar760@users.noreply.github.com> Date: Mon, 10 Feb 2025 01:14:48 +0530 Subject: [PATCH 1/2] "This ensures proper saving of collaboration data when user leaves the room Fixes #9104" --- excalidraw-app/App.tsx | 6 +++--- excalidraw-app/collab/Collab.tsx | 12 ++++++++---- excalidraw-app/share/ShareDialog.tsx | 9 +++++---- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/excalidraw-app/App.tsx b/excalidraw-app/App.tsx index f7842505a..0def0213d 100644 --- a/excalidraw-app/App.tsx +++ b/excalidraw-app/App.tsx @@ -476,7 +476,7 @@ const ExcalidrawWrapper = () => { collabAPI?.isCollaborating() && !isCollaborationLink(window.location.href) ) { - collabAPI.stopCollaboration(false); + await collabAPI.stopCollaboration(false); } excalidrawAPI.updateScene({ appState: { isLoading: true } }); @@ -968,9 +968,9 @@ const ExcalidrawWrapper = () => { "exit", "collaboration", ], - perform: () => { + perform: async () => { if (collabAPI) { - collabAPI.stopCollaboration(); + await collabAPI.stopCollaboration(); if (!collabAPI.isCollaborating()) { setShareDialogState({ isOpen: false }); } diff --git a/excalidraw-app/collab/Collab.tsx b/excalidraw-app/collab/Collab.tsx index 1097aeda9..b993e9c1f 100644 --- a/excalidraw-app/collab/Collab.tsx +++ b/excalidraw-app/collab/Collab.tsx @@ -340,13 +340,14 @@ class Collab extends PureComponent { } }; - stopCollaboration = (keepRemoteState = true) => { + stopCollaboration = async (keepRemoteState = true) => { this.queueBroadcastAllElements.cancel(); this.queueSaveToFirebase.cancel(); this.loadImageFiles.cancel(); this.resetErrorIndicator(true); - this.saveCollabRoomToFirebase( + // Ensure we save the latest state before stopping collaboration + await this.saveCollabRoomToFirebase( getSyncableElements( this.excalidrawAPI.getSceneElementsIncludingDeleted(), ), @@ -367,6 +368,9 @@ class Collab extends PureComponent { // that could have been saved in other tabs while we were collaborating resetBrowserStateVersions(); + // Ensure all pending changes are saved before pushing new state + this.queueSaveToFirebase.flush(); + window.history.pushState({}, APP_NAME, window.location.origin); this.destroySocketClient(); @@ -938,9 +942,9 @@ class Collab extends PureComponent { }, SYNC_FULL_SCENE_INTERVAL_MS); queueSaveToFirebase = throttle( - () => { + async () => { if (this.portal.socketInitialized) { - this.saveCollabRoomToFirebase( + await this.saveCollabRoomToFirebase( getSyncableElements( this.excalidrawAPI.getSceneElementsIncludingDeleted(), ), diff --git a/excalidraw-app/share/ShareDialog.tsx b/excalidraw-app/share/ShareDialog.tsx index b3a307526..d3c8f1577 100644 --- a/excalidraw-app/share/ShareDialog.tsx +++ b/excalidraw-app/share/ShareDialog.tsx @@ -164,10 +164,11 @@ const ActiveRoomDialog = ({ icon={playerStopFilledIcon} onClick={() => { trackEvent("share", "room closed"); - collabAPI.stopCollaboration(); - if (!collabAPI.isCollaborating()) { - handleClose(); - } + collabAPI.stopCollaboration().then(() => { + if (!collabAPI.isCollaborating()) { + handleClose(); + } + }); }} /> From 1053a51f64eb5e192d71a71cbcb11e8375bf2449 Mon Sep 17 00:00:00 2001 From: Shivansh Kumar <104713531+ShivanshKumar760@users.noreply.github.com> Date: Mon, 10 Feb 2025 11:13:59 +0530 Subject: [PATCH 2/2] Race condiition fixed for fix:9104 --- excalidraw-app/share/ShareDialog.tsx | 4 +--- packages/excalidraw/components/App.tsx | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/excalidraw-app/share/ShareDialog.tsx b/excalidraw-app/share/ShareDialog.tsx index d3c8f1577..8ae211d12 100644 --- a/excalidraw-app/share/ShareDialog.tsx +++ b/excalidraw-app/share/ShareDialog.tsx @@ -165,9 +165,7 @@ const ActiveRoomDialog = ({ onClick={() => { trackEvent("share", "room closed"); collabAPI.stopCollaboration().then(() => { - if (!collabAPI.isCollaborating()) { - handleClose(); - } + handleClose(); }); }} /> diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 9699c8a8a..e8fbebcb3 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -2250,7 +2250,7 @@ class App extends React.Component { this.setState((state) => ({ ...getDefaultAppState(), isLoading: opts?.resetLoadingState ? false : state.isLoading, - theme: this.state.theme, + theme: state.theme, })); this.resetStore(); this.resetHistory(); @@ -2874,8 +2874,21 @@ class App extends React.Component { // init, which would trigger onChange with empty elements, which would then // override whatever is in localStorage currently. if (!this.state.isLoading) { - this.props.onChange?.(elements, this.state, this.files); - this.onChangeEmitter.trigger(elements, this.state, this.files); + // Only trigger onChange when there are actual changes to elements or important app state + const prevElements = this.scene.getElementsIncludingDeleted(); + const hasElementChanges = elements.length !== prevElements.length || + elements.some((element, index) => element !== prevElements[index]); + + const hasImportantStateChanges = + this.state.selectedElementIds !== prevState.selectedElementIds || + this.state.width !== prevState.width || + this.state.height !== prevState.height || + this.state.editingTextElement !== prevState.editingTextElement; + + if (hasElementChanges || hasImportantStateChanges) { + this.props.onChange?.(elements, this.state, this.files); + this.onChangeEmitter.trigger(elements, this.state, this.files); + } } }