Some cleanup in App.tsx (#1681)

src/components/App.tsx

@@ -110,10 +110,12 @@ import {
   SCENE,
   EVENT,
   ENV,
+  CANVAS_ONLY_ACTIONS,
 } from "../constants";
 import {
   INITAL_SCENE_UPDATE_TIMEOUT,
   TAP_TWICE_TIMEOUT,
+  SYNC_FULL_SCENE_INTERVAL_MS,
 } from "../time_constants";
 
 import LayerUI from "./LayerUI";
@@ -169,8 +171,6 @@ const gesture: Gesture = {
   initialScale: null,
 };
 
-const SYNC_FULL_SCENE_INTERVAL_MS = 20000;
-
 class App extends React.Component<any, AppState> {
   canvas: HTMLCanvasElement | null = null;
   rc: RoughCanvas | null = null;
@@ -178,9 +178,8 @@ class App extends React.Component<any, AppState> {
   lastBroadcastedOrReceivedSceneVersion: number = -1;
   broadcastedElementVersions: Map<string, number> = new Map();
   removeSceneCallback: SceneStateCallbackRemover | null = null;
-
+  unmounted: boolean = false;
   actionManager: ActionManager;
-  canvasOnlyActions = ["selectAll"];
 
   public state: AppState = {
     ...getDefaultAppState(),
@@ -362,12 +361,10 @@ class App extends React.Component<any, AppState> {
     }
   };
 
-  private unmounted = false;
-
   public async componentDidMount() {
     if (
-      process.env.NODE_ENV === "test" ||
-      process.env.NODE_ENV === "development"
+      process.env.NODE_ENV === ENV.TEST ||
+      process.env.NODE_ENV === ENV.DEVELOPMENT
     ) {
       const setState = this.setState.bind(this);
       Object.defineProperties(window.h, {
@@ -394,50 +391,24 @@ class App extends React.Component<any, AppState> {
       this.onSceneUpdated,
     );
 
-    document.addEventListener(EVENT.COPY, this.onCopy);
-    document.addEventListener(EVENT.PASTE, this.pasteFromClipboard);
-    document.addEventListener(EVENT.CUT, this.onCut);
-
-    document.addEventListener(EVENT.KEYDOWN, this.onKeyDown, false);
-    document.addEventListener(EVENT.KEYUP, this.onKeyUp, { passive: true });
-    document.addEventListener(
-      EVENT.MOUSE_MOVE,
-      this.updateCurrentCursorPosition,
-    );
-    window.addEventListener(EVENT.RESIZE, this.onResize, false);
-    window.addEventListener(EVENT.UNLOAD, this.onUnload, false);
-    window.addEventListener(EVENT.BLUR, this.onBlur, false);
-    window.addEventListener(EVENT.DRAG_OVER, this.disableEvent, false);
-    window.addEventListener(EVENT.DROP, this.disableEvent, false);
-
-    // rerender text elements on font load to fix #637 && #1553
-    document.fonts?.addEventListener?.("loadingdone", this.onFontLoaded);
-
-    // Safari-only desktop pinch zoom
-    document.addEventListener(
-      EVENT.GESTURE_START,
-      this.onGestureStart as any,
-      false,
-    );
-    document.addEventListener(
-      EVENT.GESTURE_CHANGE,
-      this.onGestureChange as any,
-      false,
-    );
-    document.addEventListener(
-      EVENT.GESTURE_END,
-      this.onGestureEnd as any,
-      false,
-    );
-    window.addEventListener(EVENT.BEFORE_UNLOAD, this.beforeUnload);
-
+    this.addEventListeners();
     this.initializeScene();
   }
 
   public componentWillUnmount() {
     this.unmounted = true;
     this.removeSceneCallback!();
+    this.removeEventListeners();
+  }
 
+  private onResize = withBatchedUpdates(() => {
+    globalSceneState
+      .getElementsIncludingDeleted()
+      .forEach((element) => invalidateShapeForElement(element));
+    this.setState({});
+  });
+
+  private removeEventListeners() {
     document.removeEventListener(EVENT.COPY, this.onCopy);
     document.removeEventListener(EVENT.PASTE, this.pasteFromClipboard);
     document.removeEventListener(EVENT.CUT, this.onCut);
@@ -472,12 +443,45 @@ class App extends React.Component<any, AppState> {
     );
     window.removeEventListener(EVENT.BEFORE_UNLOAD, this.beforeUnload);
   }
-  private onResize = withBatchedUpdates(() => {
-    globalSceneState
-      .getElementsIncludingDeleted()
-      .forEach((element) => invalidateShapeForElement(element));
-    this.setState({});
-  });
+
+  private addEventListeners() {
+    document.addEventListener(EVENT.COPY, this.onCopy);
+    document.addEventListener(EVENT.PASTE, this.pasteFromClipboard);
+    document.addEventListener(EVENT.CUT, this.onCut);
+
+    document.addEventListener(EVENT.KEYDOWN, this.onKeyDown, false);
+    document.addEventListener(EVENT.KEYUP, this.onKeyUp, { passive: true });
+    document.addEventListener(
+      EVENT.MOUSE_MOVE,
+      this.updateCurrentCursorPosition,
+    );
+    window.addEventListener(EVENT.RESIZE, this.onResize, false);
+    window.addEventListener(EVENT.UNLOAD, this.onUnload, false);
+    window.addEventListener(EVENT.BLUR, this.onBlur, false);
+    window.addEventListener(EVENT.DRAG_OVER, this.disableEvent, false);
+    window.addEventListener(EVENT.DROP, this.disableEvent, false);
+
+    // rerender text elements on font load to fix #637 && #1553
+    document.fonts?.addEventListener?.("loadingdone", this.onFontLoaded);
+
+    // Safari-only desktop pinch zoom
+    document.addEventListener(
+      EVENT.GESTURE_START,
+      this.onGestureStart as any,
+      false,
+    );
+    document.addEventListener(
+      EVENT.GESTURE_CHANGE,
+      this.onGestureChange as any,
+      false,
+    );
+    document.addEventListener(
+      EVENT.GESTURE_END,
+      this.onGestureEnd as any,
+      false,
+    );
+    window.addEventListener(EVENT.BEFORE_UNLOAD, this.beforeUnload);
+  }
 
   private beforeUnload = withBatchedUpdates((event: BeforeUnloadEvent) => {
     if (
@@ -640,7 +644,7 @@ class App extends React.Component<any, AppState> {
     );
   };
 
-  private resetTapTwice() {
+  private static resetTapTwice() {
     didTapTwice = false;
   }
 
@@ -649,7 +653,7 @@ class App extends React.Component<any, AppState> {
       didTapTwice = true;
       clearTimeout(tappedTwiceTimer);
       tappedTwiceTimer = window.setTimeout(
-        this.resetTapTwice,
+        App.resetTapTwice,
         TAP_TWICE_TIMEOUT,
       );
       return;
@@ -932,7 +936,7 @@ class App extends React.Component<any, AppState> {
         // undo, a user makes a change, and then try to redo, your element(s) will be lost. However,
         // right now we think this is the right tradeoff.
         history.clear();
-        if (this.portal.socketInitialized === false) {
+        if (!this.portal.socketInitialized) {
           initialize();
         }
       };
@@ -2704,7 +2708,7 @@ class App extends React.Component<any, AppState> {
               action: this.copyToClipboardAsSvg,
             },
           ...this.actionManager.getContextMenuItems((action) =>
-            this.canvasOnlyActions.includes(action.name),
+            CANVAS_ONLY_ACTIONS.includes(action.name),
           ),
         ],
         top: event.clientY,
@@ -2736,7 +2740,7 @@ class App extends React.Component<any, AppState> {
           action: this.copyToClipboardAsSvg,
         },
         ...this.actionManager.getContextMenuItems(
-          (action) => !this.canvasOnlyActions.includes(action.name),
+          (action) => !CANVAS_ONLY_ACTIONS.includes(action.name),
         ),
       ],
       top: event.clientY,

src/constants.ts

@@ -65,3 +65,5 @@ export const FONT_FAMILY = {
   2: "Helvetica",
   3: "Cascadia",
 } as const;
+
+export const CANVAS_ONLY_ACTIONS = ["selectAll"];

src/time_constants.ts

@@ -1,3 +1,4 @@
 // time in milliseconds
 export const TAP_TWICE_TIMEOUT = 300;
 export const INITAL_SCENE_UPDATE_TIMEOUT = 5000;
+export const SYNC_FULL_SCENE_INTERVAL_MS = 20000;