From 0ab6a1f80f0933cfbed3d3cf9bbebb9fbf0e9360 Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Mon, 7 Mar 2022 09:24:26 -0500 Subject: [PATCH] [MM-42072] Fix issues with loading screen animations (#2010) Co-authored-by: Mattermod --- src/main/views/MattermostView.ts | 6 ++- src/main/views/viewManager.test.js | 20 +++++--- src/main/views/viewManager.ts | 47 ++++++++++++++----- .../LoadingAnimation/LoadingAnimation.tsx | 9 ++-- 4 files changed, 58 insertions(+), 24 deletions(-) diff --git a/src/main/views/MattermostView.ts b/src/main/views/MattermostView.ts index 6c928c24..7d7d2425 100644 --- a/src/main/views/MattermostView.ts +++ b/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 = () => { diff --git a/src/main/views/viewManager.test.js b/src/main/views/viewManager.test.js index f17ebb59..08fde9a7 100644 --- a/src/main/views/viewManager.test.js +++ b/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(); diff --git a/src/main/views/viewManager.ts b/src/main/views/viewManager.ts index 29f6ebe4..d69ead07 100644 --- a/src/main/views/viewManager.ts +++ b/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.needsLoadingScreen()) { - this.showLoadingScreen(); + 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.loadingScreen!.webContents.send(TOGGLE_LOADING_SCREEN_VISIBILITY, true); + 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); } } diff --git a/src/renderer/components/LoadingAnimation/LoadingAnimation.tsx b/src/renderer/components/LoadingAnimation/LoadingAnimation.tsx index cdfcf061..4a5bb3c2 100644 --- a/src/renderer/components/LoadingAnimation/LoadingAnimation.tsx +++ b/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--loading': animationState === LOADING_STATE.LOADING && animationState !== LOADING_STATE.COMPLETE, - 'LoadingAnimation--loaded': animationState === LOADING_STATE.LOADED && animationState !== LOADING_STATE.COMPLETE, + 'LoadingAnimation--spinning': animationState !== LOADING_STATE.INITIALIZING, + 'LoadingAnimation--loading': animationState === LOADING_STATE.LOADING, + 'LoadingAnimation--loaded': animationState === LOADING_STATE.LOADED, })} >