[MM-42072] Fix issues with loading screen animations (#2010)

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

src/main/views/MattermostView.ts

@@ -263,7 +263,11 @@ export class MattermostView extends EventEmitter {
     }
 
     isReady = () => {
-        return this.status !== Status.LOADING;
+        return this.status === Status.READY;
+    }
+
+    isErrored = () => {
+        return this.status === Status.ERROR;
     }
 
     needsLoadingScreen = () => {

src/main/views/viewManager.test.js

@@ -98,13 +98,12 @@ describe('main/views/viewManager', () => {
             expect(viewManager.closedViews.has('server1-tab1')).toBe(false);
         });
 
-        it('should add view to views map, add listeners and show the view', () => {
+        it('should add view to views map and add listeners', () => {
             viewManager.loadView({name: 'server1'}, {}, {name: 'tab1', isOpen: true}, 'http://server-1.com/subpath');
             expect(viewManager.views.has('server1-tab1')).toBe(true);
             expect(viewManager.createLoadingScreen).toHaveBeenCalled();
             expect(onceFn).toHaveBeenCalledWith(LOAD_SUCCESS, viewManager.activateView);
             expect(loadFn).toHaveBeenCalledWith('http://server-1.com/subpath');
-            expect(viewManager.showByName).toHaveBeenCalledWith('server1-tab1');
         });
     });
 
@@ -504,6 +503,7 @@ describe('main/views/viewManager', () => {
         const viewManager = new ViewManager({});
         const baseView = {
             isReady: jest.fn(),
+            isErrored: jest.fn(),
             show: jest.fn(),
             hide: jest.fn(),
             needsLoadingScreen: jest.fn(),
@@ -567,23 +567,31 @@ describe('main/views/viewManager', () => {
             expect(oldView.hide).toHaveBeenCalled();
         });
 
+        it('should not show the view when it is in error state', () => {
+            const view = {...baseView};
+            view.isErrored.mockReturnValue(true);
+            viewManager.views.set('view1', view);
+            viewManager.showByName('view1');
+            expect(view.show).not.toHaveBeenCalled();
+        });
+
         it('should show loading screen when the view needs it', () => {
             const view = {...baseView};
+            view.isErrored.mockReturnValue(false);
             view.needsLoadingScreen.mockImplementation(() => true);
             viewManager.views.set('view1', view);
             viewManager.showByName('view1');
             expect(viewManager.showLoadingScreen).toHaveBeenCalled();
         });
 
-        it('should show the view when ready', () => {
+        it('should show the view when not errored', () => {
             const view = {...baseView};
             view.needsLoadingScreen.mockImplementation(() => false);
-            view.isReady.mockImplementation(() => true);
+            view.isErrored.mockReturnValue(false);
             viewManager.views.set('view1', view);
             viewManager.showByName('view1');
             expect(viewManager.currentView).toBe('view1');
             expect(view.show).toHaveBeenCalled();
-            expect(viewManager.fadeLoadingScreen).toHaveBeenCalled();
         });
     });
 
@@ -594,7 +602,7 @@ describe('main/views/viewManager', () => {
             addBrowserView: jest.fn(),
         };
         const viewManager = new ViewManager(window);
-        const loadingScreen = {webContents: {send: jest.fn()}};
+        const loadingScreen = {webContents: {send: jest.fn(), isLoading: () => false}};
 
         beforeEach(() => {
             viewManager.createLoadingScreen = jest.fn();

src/main/views/viewManager.ts

@@ -37,6 +37,12 @@ import WebContentsEventManager from './webContentEvents';
 const URL_VIEW_DURATION = 10 * SECOND;
 const URL_VIEW_HEIGHT = 36;
 
+enum LoadingScreenState {
+    VISIBLE = 1,
+    FADING = 2,
+    HIDDEN = 3,
+}
+
 export class ViewManager {
     configServers: TeamWithTabs[];
     lastActiveServer?: number;
@@ -48,6 +54,7 @@ export class ViewManager {
     urlViewCancel?: () => void;
     mainWindow: BrowserWindow;
     loadingScreen?: BrowserView;
+    loadingScreenState: LoadingScreenState;
 
     constructor(mainWindow: BrowserWindow) {
         this.configServers = Config.teams.concat();
@@ -56,6 +63,7 @@ export class ViewManager {
         this.views = new Map(); // keep in mind that this doesn't need to hold server order, only tabs on the renderer need that.
         this.mainWindow = mainWindow;
         this.closedViews = new Map();
+        this.loadingScreenState = LoadingScreenState.HIDDEN;
     }
 
     updateMainWindow = (mainWindow: BrowserWindow) => {
@@ -83,7 +91,6 @@ export class ViewManager {
         }
         const view = new MattermostView(tabView, serverInfo, this.mainWindow, this.viewOptions);
         this.views.set(tabView.name, view);
-        this.showByName(tabView.name);
         if (!this.loadingScreen) {
             this.createLoadingScreen();
         }
@@ -179,18 +186,16 @@ export class ViewManager {
             }
 
             this.currentView = name;
+            if (!newView.isErrored()) {
+                newView.show();
                 if (newView.needsLoadingScreen()) {
                     this.showLoadingScreen();
                 }
+            }
             newView.window.webContents.send(SET_ACTIVE_VIEW, newView.tab.server.name, newView.tab.type);
             ipcMain.emit(SET_ACTIVE_VIEW, true, newView.tab.server.name, newView.tab.type);
             if (newView.isReady()) {
-                // if view is not ready, the renderer will have something to display instead.
-                newView.show();
                 ipcMain.emit(UPDATE_LAST_ACTIVE, true, newView.tab.server.name, newView.tab.type);
-                if (!newView.needsLoadingScreen()) {
-                    this.fadeLoadingScreen();
-                }
             } else {
                 log.warn(`couldn't show ${name}, not ready`);
             }
@@ -249,8 +254,11 @@ export class ViewManager {
         ipcMain.emit(OPEN_TAB, null, srv.name, tab.name);
     }
 
-    failLoading = () => {
+    failLoading = (tabName: string) => {
         this.fadeLoadingScreen();
+        if (this.currentView === tabName) {
+            this.getCurrentView()?.hide();
+        }
     }
 
     getCurrentView() {
@@ -351,6 +359,11 @@ export class ViewManager {
         this.loadingScreen = new BrowserView({webPreferences: {
             nativeWindowOpen: true,
             preload,
+
+            // Workaround for this issue: https://github.com/electron/electron/issues/30993
+            // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+            // @ts-ignore
+            transparent: true,
         }});
         const localURL = getLocalURLString('loadingScreen.html');
         this.loadingScreen.webContents.loadURL(localURL);
@@ -361,7 +374,15 @@ export class ViewManager {
             this.createLoadingScreen();
         }
 
+        this.loadingScreenState = LoadingScreenState.VISIBLE;
+
+        if (this.loadingScreen?.webContents.isLoading()) {
+            this.loadingScreen.webContents.once('did-finish-load', () => {
                 this.loadingScreen!.webContents.send(TOGGLE_LOADING_SCREEN_VISIBILITY, true);
+            });
+        } else {
+            this.loadingScreen!.webContents.send(TOGGLE_LOADING_SCREEN_VISIBILITY, true);
+        }
 
         if (this.mainWindow.getBrowserViews().includes(this.loadingScreen!)) {
             this.mainWindow.setTopBrowserView(this.loadingScreen!);
@@ -373,13 +394,15 @@ export class ViewManager {
     }
 
     fadeLoadingScreen = () => {
-        if (this.loadingScreen) {
+        if (this.loadingScreen && this.loadingScreenState === LoadingScreenState.VISIBLE) {
+            this.loadingScreenState = LoadingScreenState.FADING;
             this.loadingScreen.webContents.send(TOGGLE_LOADING_SCREEN_VISIBILITY, false);
         }
     }
 
     hideLoadingScreen = () => {
-        if (this.loadingScreen) {
+        if (this.loadingScreen && this.loadingScreenState !== LoadingScreenState.HIDDEN) {
+            this.loadingScreenState = LoadingScreenState.HIDDEN;
             this.mainWindow.removeBrowserView(this.loadingScreen);
         }
     }

src/renderer/components/LoadingAnimation/LoadingAnimation.tsx

@@ -12,7 +12,6 @@ const LOADING_STATE = {
     INITIALIZING: 'initializing', // animation graphics are hidden
     LOADING: 'loading', // animation graphics fade in and animate
     LOADED: 'loaded', // animation graphics fade out
-    COMPLETE: 'complete', // animation graphics are removed from the DOM
 };
 
 const ANIMATION_COMPLETION_DELAY = 500;
@@ -71,7 +70,7 @@ function LoadingAnimation({
         if (onLoadAnimationComplete) {
             onLoadAnimationComplete();
         }
-        setAnimationState(LOADING_STATE.COMPLETE);
+        setAnimationState(LOADING_STATE.INITIALIZING);
     }, 'LoadingAnimation__shrink');
 
     return (
@@ -79,9 +78,9 @@ function LoadingAnimation({
             ref={loadingIconContainerRef}
             className={classNames('LoadingAnimation', {
                 'LoadingAnimation--darkMode': darkMode,
-                'LoadingAnimation--spinning': animationState !== LOADING_STATE.INITIALIZING && animationState !== LOADING_STATE.COMPLETE,
+                'LoadingAnimation--spinning': animationState !== LOADING_STATE.INITIALIZING,
-                'LoadingAnimation--loading': animationState === LOADING_STATE.LOADING && animationState !== LOADING_STATE.COMPLETE,
+                'LoadingAnimation--loading': animationState === LOADING_STATE.LOADING,
-                'LoadingAnimation--loaded': animationState === LOADING_STATE.LOADED && animationState !== LOADING_STATE.COMPLETE,
+                'LoadingAnimation--loaded': animationState === LOADING_STATE.LOADED,
             })}
         >
             <LoadingIcon/>