From 3a0b6fb41b82dfdc6000573fb53052f2407ffe0a Mon Sep 17 00:00:00 2001 From: David Luzar Date: Wed, 21 Apr 2021 23:37:44 +0200 Subject: [PATCH] refactor: move getSyncableElements to CollabWrapper & expose isInvisiblySmallElement helper (#3471) Co-authored-by: Aakansha Doshi --- src/element/index.ts | 7 ------- src/excalidraw-app/collab/CollabWrapper.tsx | 13 ++++++++----- src/excalidraw-app/collab/Portal.tsx | 5 +++-- src/packages/excalidraw/CHANGELOG.md | 11 +++++++++++ src/packages/excalidraw/README_NEXT.md | 8 ++++---- src/packages/excalidraw/index.tsx | 2 +- 6 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/element/index.ts b/src/element/index.ts index bf12e118..240ac58a 100644 --- a/src/element/index.ts +++ b/src/element/index.ts @@ -58,13 +58,6 @@ export { } from "./sizeHelpers"; export { showSelectedShapeActions } from "./showSelectedShapeActions"; -export const getSyncableElements = ( - elements: readonly ExcalidrawElement[], // There are places in Excalidraw where synthetic invisibly small elements are added and removed. -) => - // It's probably best to keep those local otherwise there might be a race condition that - // gets the app into an invalid state. I've never seen it happen but I'm worried about it :) - elements.filter((el) => el.isDeleted || !isInvisiblySmallElement(el)); - export const getElementMap = (elements: readonly ExcalidrawElement[]) => elements.reduce( (acc: { [key: string]: ExcalidrawElement }, element: ExcalidrawElement) => { diff --git a/src/excalidraw-app/collab/CollabWrapper.tsx b/src/excalidraw-app/collab/CollabWrapper.tsx index 8fe753f8..f961e5ad 100644 --- a/src/excalidraw-app/collab/CollabWrapper.tsx +++ b/src/excalidraw-app/collab/CollabWrapper.tsx @@ -8,7 +8,6 @@ import { ExcalidrawElement } from "../../element/types"; import { getElementMap, getSceneVersion, - getSyncableElements, } from "../../packages/excalidraw/index"; import { Collaborator, Gesture } from "../../types"; import { resolvablePromise, withBatchedUpdates } from "../../utils"; @@ -41,6 +40,7 @@ import { t } from "../../i18n"; import { UserIdleState } from "../../types"; import { IDLE_THRESHOLD, ACTIVE_THRESHOLD } from "../../constants"; import { trackEvent } from "../../analytics"; +import { isInvisiblySmallElement } from "../../element"; interface CollabState { modalIsShown: boolean; @@ -146,7 +146,7 @@ class CollabWrapper extends PureComponent { }; private beforeUnload = withBatchedUpdates((event: BeforeUnloadEvent) => { - const syncableElements = getSyncableElements( + const syncableElements = this.getSyncableElements( this.getSceneElementsIncludingDeleted(), ); @@ -177,7 +177,7 @@ class CollabWrapper extends PureComponent { }); saveCollabRoomToFirebase = async ( - syncableElements: ExcalidrawElement[] = getSyncableElements( + syncableElements: ExcalidrawElement[] = this.getSyncableElements( this.excalidrawAPI.getSceneElementsIncludingDeleted(), ), ) => { @@ -565,7 +565,7 @@ class CollabWrapper extends PureComponent { ) { this.portal.broadcastScene( SCENE.UPDATE, - getSyncableElements(elements), + this.getSyncableElements(elements), false, ); this.lastBroadcastedOrReceivedSceneVersion = getSceneVersion(elements); @@ -576,7 +576,7 @@ class CollabWrapper extends PureComponent { queueBroadcastAllElements = throttle(() => { this.portal.broadcastScene( SCENE.UPDATE, - getSyncableElements( + this.getSyncableElements( this.excalidrawAPI.getSceneElementsIncludingDeleted(), ), true, @@ -604,6 +604,9 @@ class CollabWrapper extends PureComponent { }); }; + getSyncableElements = (elements: readonly ExcalidrawElement[]) => + elements.filter((el) => el.isDeleted || !isInvisiblySmallElement(el)); + /** PRIVATE. Use `this.getContextValue()` instead. */ private contextValue: CollabAPI | null = null; diff --git a/src/excalidraw-app/collab/Portal.tsx b/src/excalidraw-app/collab/Portal.tsx index b58cc2b9..dcd83cb7 100644 --- a/src/excalidraw-app/collab/Portal.tsx +++ b/src/excalidraw-app/collab/Portal.tsx @@ -6,7 +6,6 @@ import { import CollabWrapper from "./CollabWrapper"; -import { getSyncableElements } from "../../packages/excalidraw/index"; import { ExcalidrawElement } from "../../element/types"; import { BROADCAST, SCENE } from "../app_constants"; import { UserIdleState } from "../../types"; @@ -39,7 +38,9 @@ class Portal { this.socket.on("new-user", async (_socketId: string) => { this.broadcastScene( SCENE.INIT, - getSyncableElements(this.collab.getSceneElementsIncludingDeleted()), + this.collab.getSyncableElements( + this.collab.getSceneElementsIncludingDeleted(), + ), /* syncAll */ true, ); }); diff --git a/src/packages/excalidraw/CHANGELOG.md b/src/packages/excalidraw/CHANGELOG.md index c82aa4a2..6142815e 100644 --- a/src/packages/excalidraw/CHANGELOG.md +++ b/src/packages/excalidraw/CHANGELOG.md @@ -40,6 +40,17 @@ Please add the latest change on the top under the correct section. - When switching theme, apply it only to the active Excalidraw component. This fixes a case where the theme was getting applied to the first Excalidraw component if you had multiple Excalidraw components on the same page [#3446](https://github.com/excalidraw/excalidraw/pull/3446) +### Refactor + +- #### BREAKING CHANGE + + - Removed exposing `getSyncableElements` helper which was specific to excalidraw app collab implementation [#3471](https://github.com/excalidraw/excalidraw/pull/3471). If you happened to use it, you can easily reimplement it yourself using the newly exposed [isInvisiblySmallElement](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#isInvisiblySmallElement) helper: + + ```ts + const getSyncableElements = (elements: readonly ExcalidrawElement[]) => + elements.filter((el) => el.isDeleted || !isInvisiblySmallElement(el)); + ``` + ## Types - Renamed the following types in case you depend on them (via [#3427](https://github.com/excalidraw/excalidraw/pull/3427)): diff --git a/src/packages/excalidraw/README_NEXT.md b/src/packages/excalidraw/README_NEXT.md index c1d69e3e..68317af4 100644 --- a/src/packages/excalidraw/README_NEXT.md +++ b/src/packages/excalidraw/README_NEXT.md @@ -629,21 +629,21 @@ getSceneVersion(elements: ExcalidrawElement[]):ExcalidrawElement[] +isInvisiblySmallElement(element: ExcalidrawElement): boolean **How to use** ```js -import { getSyncableElements } from "@excalidraw/excalidraw"; +import { isInvisiblySmallElement } from "@excalidraw/excalidraw"; ``` -Returns all elements including deleted ones, excluding elements which are are invisibly small (e.g. width & height are zero). Useful when you want to sync elements during collaboration. +Returns `true` if element is invisibly small (e.g. width & height are zero). #### `getElementMap` diff --git a/src/packages/excalidraw/index.tsx b/src/packages/excalidraw/index.tsx index 3a385cdd..6e42a926 100644 --- a/src/packages/excalidraw/index.tsx +++ b/src/packages/excalidraw/index.tsx @@ -115,8 +115,8 @@ const forwardedRefComp = forwardRef< export default React.memo(forwardedRefComp, areEqual); export { getSceneVersion, - getSyncableElements, getElementMap, + isInvisiblySmallElement, } from "../../element"; export { defaultLang, languages } from "../../i18n"; export { restore, restoreAppState, restoreElements } from "../../data/restore";