[MM-52625] Rework tray icon code into a class, make the behaviour of the tray icon consistent with the OS it's running on (#2708)

* Rework tray into a class, make click behaviour consistent

* Fix issue where app wouldn't switch to workspace where the app was visible

* Fixed an issue where the app would show the window with hideOnStart enabled

* Add comment about StatusIconLinuxDbus

* Fix tests

src/common/communication.ts

@@ -169,3 +169,4 @@ export const UPDATE_APPSTATE_FOR_VIEW_ID = 'update-appstate-for-view-id';
 
 export const MAIN_WINDOW_CREATED = 'main-window-created';
 export const MAIN_WINDOW_RESIZED = 'main-window-resized';
+export const MAIN_WINDOW_FOCUSED = 'main-window-focused';

src/main/app/app.ts

@@ -9,7 +9,7 @@ import {parseURL} from 'common/utils/url';
 import updateManager from 'main/autoUpdater';
 import CertificateStore from 'main/certificateStore';
 import {localizeMessage} from 'main/i18nManager';
-import {destroyTray} from 'main/tray/tray';
+import Tray from 'main/tray/tray';
 import ViewManager from 'main/views/viewManager';
 import MainWindow from 'main/windows/mainWindow';
 
@@ -72,7 +72,7 @@ export function handleAppBeforeQuit() {
     log.debug('handleAppBeforeQuit');
 
     // Make sure tray icon gets removed if the user exits via CTRL-Q
-    destroyTray();
+    Tray.destroy();
     global.willAppQuit = true;
     updateManager.handleOnQuit();
 }

src/main/app/config.ts

@@ -11,7 +11,7 @@ import {Logger, setLoggingLevel} from 'common/log';
 
 import AutoLauncher from 'main/AutoLauncher';
 import {setUnreadBadgeSetting} from 'main/badge';
-import {refreshTrayImages} from 'main/tray/tray';
+import Tray from 'main/tray/tray';
 import LoadingScreen from 'main/views/loadingScreen';
 import MainWindow from 'main/windows/mainWindow';
 import SettingsWindow from 'main/windows/settingsWindow';
@@ -103,7 +103,7 @@ export function handleConfigUpdate(newConfig: CombinedConfig) {
 
     handleUpdateMenuEvent();
     if (newConfig.trayIconTheme) {
-        refreshTrayImages(newConfig.trayIconTheme);
+        Tray.refreshImages(newConfig.trayIconTheme);
     }
 
     ipcMain.emit(EMIT_CONFIGURATION, true, newConfig);
@@ -112,7 +112,7 @@ export function handleConfigUpdate(newConfig: CombinedConfig) {
 export function handleDarkModeChange(darkMode: boolean) {
     log.debug('handleDarkModeChange', darkMode);
 
-    refreshTrayImages(Config.trayIconTheme);
+    Tray.refreshImages(Config.trayIconTheme);
     MainWindow.sendToRenderer(DARK_MODE_CHANGE, darkMode);
     SettingsWindow.sendToRenderer(DARK_MODE_CHANGE, darkMode);
     LoadingScreen.setDarkMode(darkMode);

src/main/app/initialize.test.js

@@ -157,8 +157,8 @@ jest.mock('common/servers/serverManager', () => ({
     on: jest.fn(),
 }));
 jest.mock('main/tray/tray', () => ({
-    refreshTrayImages: jest.fn(),
+    refreshImages: jest.fn(),
-    setupTray: jest.fn(),
+    setup: jest.fn(),
 }));
 jest.mock('main/trustedOrigins', () => ({
     load: jest.fn(),

src/main/app/initialize.ts

@@ -60,7 +60,7 @@ import i18nManager from 'main/i18nManager';
 import parseArgs from 'main/ParseArgs';
 import ServerManager from 'common/servers/serverManager';
 import TrustedOriginsStore from 'main/trustedOrigins';
-import {refreshTrayImages, setupTray} from 'main/tray/tray';
+import Tray from 'main/tray/tray';
 import UserActivityMonitor from 'main/UserActivityMonitor';
 import ViewManager from 'main/views/viewManager';
 import CallsWidgetWindow from 'main/windows/callsWidgetWindow';
@@ -239,7 +239,7 @@ function initializeBeforeAppReady() {
         process.chdir(expectedPath);
     }
 
-    refreshTrayImages(Config.trayIconTheme);
+    Tray.refreshImages(Config.trayIconTheme);
 
     // If there is already an instance, quit this one
     // eslint-disable-next-line no-undef
@@ -401,7 +401,7 @@ async function initializeAfterAppReady() {
     UserActivityMonitor.startMonitoring();
 
     if (shouldShowTrayIcon()) {
-        setupTray(Config.trayIconTheme);
+        Tray.init(Config.trayIconTheme);
     }
     setupBadge();
 

src/main/app/utils.ts

@@ -25,7 +25,7 @@ import {localizeMessage} from 'main/i18nManager';
 import {createMenu as createAppMenu} from 'main/menus/app';
 import {createMenu as createTrayMenu} from 'main/menus/tray';
 import {ServerInfo} from 'main/server/serverInfo';
-import {setTrayMenu} from 'main/tray/tray';
+import Tray from 'main/tray/tray';
 import ViewManager from 'main/views/viewManager';
 import MainWindow from 'main/windows/mainWindow';
 
@@ -64,7 +64,7 @@ export function handleUpdateMenuEvent() {
     // set up context menu for tray icon
     if (shouldShowTrayIcon()) {
         const tMenu = createTrayMenu();
-        setTrayMenu(tMenu);
+        Tray.setMenu(tMenu);
     }
 }
 

src/main/tray/tray.test.js

@@ -5,7 +5,7 @@ import {handleConfigUpdate} from 'main/app/config';
 
 import AutoLauncher from 'main/AutoLauncher';
 
-import * as tray from './tray';
+import Tray from './tray';
 
 jest.mock('path', () => ({
     join: (a, b) => b,
@@ -72,7 +72,7 @@ describe('main/tray', () => {
     describe('config changes', () => {
         let spy;
         beforeAll(() => {
-            spy = jest.spyOn(tray, 'refreshTrayImages').mockImplementation();
+            spy = jest.spyOn(Tray, 'refreshImages').mockImplementation();
         });
         afterAll(() => {
             spy.mockRestore();
@@ -81,13 +81,13 @@ describe('main/tray', () => {
             handleConfigUpdate({
                 trayIconTheme: 'light',
             });
-            expect(tray.refreshTrayImages).toHaveBeenCalledWith('light');
+            expect(Tray.refreshImages).toHaveBeenCalledWith('light');
         });
         it('should update the tray icon color immediately when the config is updated', () => {
             handleConfigUpdate({
                 trayIconTheme: 'dark',
             });
-            expect(tray.refreshTrayImages).toHaveBeenCalledWith('dark');
+            expect(Tray.refreshImages).toHaveBeenCalledWith('dark');
         });
     });
 
@@ -101,7 +101,7 @@ describe('main/tray', () => {
         Object.defineProperty(process, 'platform', {
             value: 'darwin',
         });
-        const result = tray.refreshTrayImages('light');
+        const result = Tray.refreshImages('light');
         it.each(Object.keys(result))('match "%s"', (a) => {
             expect(result[a].image).toBe(darwinResultAllThemes[a]);
         });
@@ -121,7 +121,7 @@ describe('main/tray', () => {
         Object.defineProperty(process, 'platform', {
             value: 'win32',
         });
-        const result = tray.refreshTrayImages('light');
+        const result = Tray.refreshImages('light');
         it.each(Object.keys(result))('match "%s"', (a) => {
             expect(result[a].image).toBe(winResultLight[a]);
         });
@@ -141,7 +141,7 @@ describe('main/tray', () => {
         Object.defineProperty(process, 'platform', {
             value: 'win32',
         });
-        const result = tray.refreshTrayImages('dark');
+        const result = Tray.refreshImages('dark');
         it.each(Object.keys(result))('match "%s"', (a) => {
             expect(result[a].image).toBe(winResultDark[a]);
         });
@@ -161,7 +161,7 @@ describe('main/tray', () => {
         Object.defineProperty(process, 'platform', {
             value: 'linux',
         });
-        const result = tray.refreshTrayImages('light');
+        const result = Tray.refreshImages('light');
         it.each(Object.keys(result))('match "%s"', (a) => {
             expect(result[a].image).toBe(linuxResultLight[a]);
         });
@@ -181,7 +181,7 @@ describe('main/tray', () => {
         Object.defineProperty(process, 'platform', {
             value: 'linux',
         });
-        const result = tray.refreshTrayImages('dark');
+        const result = Tray.refreshImages('dark');
         it.each(Object.keys(result))('match "%s"', (a) => {
             expect(result[a].image).toBe(linuxResultDark[a]);
         });

src/main/tray/tray.ts

@@ -16,19 +16,43 @@ import SettingsWindow from 'main/windows/settingsWindow';
 const assetsDir = path.resolve(app.getAppPath(), 'assets');
 const log = new Logger('Tray');
 
-let trayImages: Record<string, Electron.NativeImage>;
+export class TrayIcon {
-let trayIcon: Tray;
+    private tray?: Tray;
-let lastStatus = 'normal';
+    private images: Record<string, Electron.NativeImage>;
-let lastMessage = app.name;
+    private status: string;
+    private message: string;
 
-/* istanbul ignore next */
+    constructor() {
-export function refreshTrayImages(trayIconTheme: string) {
+        this.status = 'normal';
+        this.message = app.name;
+        this.images = {};
+
+        AppState.on(UPDATE_APPSTATE_TOTALS, this.onAppStateUpdate);
+    }
+
+    init = (iconTheme: string) => {
+        this.refreshImages(iconTheme);
+        this.tray = new Tray(this.images.normal);
+
+        if (process.platform === 'darwin') {
+            systemPreferences.subscribeNotification('AppleInterfaceThemeChangedNotification', () => {
+                this.tray?.setImage(this.images.normal);
+            });
+        }
+
+        this.tray.setToolTip(app.name);
+        this.tray.on('click', this.onClick);
+        this.tray.on('right-click', () => this.tray?.popUpContextMenu());
+        this.tray.on('balloon-click', this.onClick);
+    }
+
+    refreshImages = (trayIconTheme: string) => {
         const systemTheme = nativeTheme.shouldUseDarkColors ? 'light' : 'dark';
         const winTheme = trayIconTheme === 'use_system' ? systemTheme : trayIconTheme;
 
         switch (process.platform) {
         case 'win32':
-        trayImages = {
+            this.images = {
                 normal: nativeImage.createFromPath(path.resolve(assetsDir, `windows/tray_${winTheme}.ico`)),
                 unread: nativeImage.createFromPath(path.resolve(assetsDir, `windows/tray_${winTheme}_unread.ico`)),
                 mention: nativeImage.createFromPath(path.resolve(assetsDir, `windows/tray_${winTheme}_mention.ico`)),
@@ -41,7 +65,7 @@ export function refreshTrayImages(trayIconTheme: string) {
             osxNormal.setTemplateImage(true);
             osxUnread.setTemplateImage(true);
 
-        trayImages = {
+            this.images = {
                 normal: osxNormal,
                 unread: osxUnread,
                 mention: osxUnread,
@@ -52,14 +76,14 @@ export function refreshTrayImages(trayIconTheme: string) {
         case 'linux':
         {
             if (trayIconTheme === 'dark') {
-            trayImages = {
+                this.images = {
                     normal: nativeImage.createFromPath(path.resolve(assetsDir, 'linux', 'top_bar_dark_16.png')),
                     unread: nativeImage.createFromPath(path.resolve(assetsDir, 'linux', 'top_bar_dark_unread_16.png')),
                     mention: nativeImage.createFromPath(path.resolve(assetsDir, 'linux', 'top_bar_dark_mention_16.png')),
                 };
             } else {
                 //Fallback for invalid theme setting
-            trayImages = {
+                this.images = {
                     normal: nativeImage.createFromPath(path.resolve(assetsDir, 'linux', 'top_bar_light_16.png')),
                     unread: nativeImage.createFromPath(path.resolve(assetsDir, 'linux', 'top_bar_light_unread_16.png')),
                     mention: nativeImage.createFromPath(path.resolve(assetsDir, 'linux', 'top_bar_light_mention_16.png')),
@@ -68,99 +92,79 @@ export function refreshTrayImages(trayIconTheme: string) {
             break;
         }
         default:
-        trayImages = {};
+            this.images = {};
         }
-    if (trayIcon) {
+        if (this.tray) {
-        setTray(lastStatus, lastMessage);
+            this.update(this.status, this.message);
+        }
+        return this.images;
     }
-    return trayImages;
-}
 
-export function setupTray(iconTheme: string) {
+    destroy = () => {
-    refreshTrayImages(iconTheme);
+        if (process.platform === 'win32') {
-    trayIcon = new Tray(trayImages.normal);
+            this.tray?.destroy();
+        }
+    }
+
+    setMenu = (tMenu: Electron.Menu) => this.tray?.setContextMenu(tMenu);
+
+    private update = (status: string, message: string) => {
+        if (!this.tray || this.tray.isDestroyed()) {
+            return;
+        }
+
+        this.status = status;
+        this.message = message;
+        this.tray.setImage(this.images[status]);
+        this.tray.setToolTip(message);
+    }
+
+    // Linux note: the click event was fixed in Electron v23, but only fires when the OS supports StatusIconLinuxDbus
+    // There is a fallback case that will make sure the icon is displayed, but will only support the context menu
+    // See here: https://github.com/electron/electron/pull/36333
+    private onClick = () => {
+        log.verbose('onClick');
+
+        // Special case for macOS since that's how most tray icons behave there
         if (process.platform === 'darwin') {
-        systemPreferences.subscribeNotification('AppleInterfaceThemeChangedNotification', () => {
+            this.tray?.popUpContextMenu();
-            trayIcon.setImage(trayImages.normal);
+            return;
-        });
         }
 
-    trayIcon.setToolTip(app.name);
+        // At minimum show the main window
-    trayIcon.on('click', () => {
-        const mainWindow = MainWindow.get();
-        if (mainWindow && mainWindow.isVisible()) {
-            mainWindow.blur(); // To move focus to the next top-level window in Windows
-            mainWindow.hide();
-        } else {
-            restoreMain();
-        }
-    });
-
-    trayIcon.on('right-click', () => {
-        trayIcon.popUpContextMenu();
-    });
-    trayIcon.on('balloon-click', () => {
-        restoreMain();
-    });
-
-    AppState.on(UPDATE_APPSTATE_TOTALS, (anyExpired: boolean, anyMentions: number, anyUnreads: boolean) => {
-        if (anyMentions > 0) {
-            setTray('mention', localizeMessage('main.tray.tray.mention', 'You have been mentioned'));
-        } else if (anyUnreads) {
-            setTray('unread', localizeMessage('main.tray.tray.unread', 'You have unread channels'));
-        } else if (anyExpired) {
-            setTray('mention', localizeMessage('main.tray.tray.expired', 'Session Expired: Please sign in to continue receiving notifications.'));
-        } else {
-            setTray('normal', app.name);
-        }
-    });
-}
-
-const restoreMain = () => {
-    log.info('restoreMain');
         MainWindow.show();
+
         const mainWindow = MainWindow.get();
         if (!mainWindow) {
             throw new Error('Main window does not exist');
         }
-    if (!mainWindow.isVisible() || mainWindow.isMinimized()) {
+
+        // Restore if minimized
         if (mainWindow.isMinimized()) {
             mainWindow.restore();
-        } else {
             mainWindow.show();
         }
+
         const settingsWindow = SettingsWindow.get();
         if (settingsWindow) {
             settingsWindow.focus();
         } else {
             mainWindow.focus();
         }
-    } else if (SettingsWindow.get()) {
+    };
-        SettingsWindow.get()?.focus();
+
+    private onAppStateUpdate = (anyExpired: boolean, anyMentions: number, anyUnreads: boolean) => {
+        if (anyMentions > 0) {
+            this.update('mention', localizeMessage('main.tray.tray.mention', 'You have been mentioned'));
+        } else if (anyUnreads) {
+            this.update('unread', localizeMessage('main.tray.tray.unread', 'You have unread channels'));
+        } else if (anyExpired) {
+            this.update('mention', localizeMessage('main.tray.tray.expired', 'Session Expired: Please sign in to continue receiving notifications.'));
         } else {
-        mainWindow.focus();
+            this.update('normal', app.name);
         }
-};
-
-function setTray(status: string, message: string) {
-    if (trayIcon.isDestroyed()) {
-        return;
-    }
-
-    lastStatus = status;
-    lastMessage = message;
-    trayIcon.setImage(trayImages[status]);
-    trayIcon.setToolTip(message);
-}
-
-export function destroyTray() {
-    if (trayIcon && process.platform === 'win32') {
-        trayIcon.destroy();
     }
 }
 
-export function setTrayMenu(tMenu: Electron.Menu) {
+const tray = new TrayIcon();
-    if (trayIcon) {
+export default tray;
-        trayIcon.setContextMenu(tMenu);
-    }
-}

src/main/views/viewManager.ts

@@ -26,6 +26,7 @@ import {
     SESSION_EXPIRED,
     MAIN_WINDOW_CREATED,
     MAIN_WINDOW_RESIZED,
+    MAIN_WINDOW_FOCUSED,
 } from 'common/communication';
 import Config from 'common/config';
 import {Logger} from 'common/log';
@@ -61,6 +62,7 @@ export class ViewManager {
 
         MainWindow.on(MAIN_WINDOW_CREATED, this.init);
         MainWindow.on(MAIN_WINDOW_RESIZED, this.handleSetCurrentViewBounds);
+        MainWindow.on(MAIN_WINDOW_FOCUSED, this.focusCurrentView);
         ipcMain.handle(GET_VIEW_INFO_FOR_TEST, this.handleGetViewInfoForTest);
         ipcMain.on(HISTORY, this.handleHistory);
         ipcMain.on(REACT_APP_INITIALIZED, this.handleReactAppInitialized);
@@ -76,8 +78,6 @@ export class ViewManager {
     }
 
     private init = () => {
-        MainWindow.onBrowserWindow?.('focus', this.focusCurrentView);
-
         LoadingScreen.show();
         ServerManager.getAllServers().forEach((server) => this.loadServer(server));
         this.showInitial();

src/main/windows/mainWindow.test.js

@@ -485,6 +485,7 @@ describe('main/windows/mainWindow', () => {
                 setWindowOpenHandler: jest.fn(),
             },
         };
+        mainWindow.init = jest.fn();
 
         beforeEach(() => {
             mainWindow.win.show.mockImplementation(() => {
@@ -493,16 +494,21 @@ describe('main/windows/mainWindow', () => {
         });
 
         afterEach(() => {
+            mainWindow.ready = false;
             jest.resetAllMocks();
         });
 
-        it('should show main window if it exists and focus it if it is already visible', () => {
+        it('should show main window and focus it if it is exists', () => {
+            mainWindow.ready = true;
             mainWindow.show();
             expect(mainWindow.win.show).toHaveBeenCalled();
-
-            mainWindow.show();
             expect(mainWindow.win.focus).toHaveBeenCalled();
         });
+
+        it('should init if the main window does not exist', () => {
+            mainWindow.show();
+            expect(mainWindow.init).toHaveBeenCalled();
+        });
     });
 
     describe('onUnresponsive', () => {

src/main/windows/mainWindow.ts

@@ -25,6 +25,7 @@ import {
     MAXIMIZE_CHANGE,
     MAIN_WINDOW_CREATED,
     MAIN_WINDOW_RESIZED,
+    MAIN_WINDOW_FOCUSED,
     VIEW_FINISHED_RESIZING,
 } from 'common/communication';
 import Config from 'common/config';
@@ -99,11 +100,6 @@ export class MainWindow extends EventEmitter {
         this.win.setAutoHideMenuBar(true);
         this.win.setMenuBarVisibility(false);
 
-        const localURL = getLocalURLString('index.html');
-        this.win.loadURL(localURL).catch(
-            (reason) => {
-                log.error('failed to load', reason);
-            });
         this.win.once('ready-to-show', () => {
             if (!this.win) {
                 return;
@@ -123,7 +119,6 @@ export class MainWindow extends EventEmitter {
         this.win.once('restore', () => {
             this.win?.restore();
         });
-
         this.win.on('close', this.onClose);
         this.win.on('closed', this.onClosed);
         this.win.on('focus', this.onFocus);
@@ -143,7 +138,6 @@ export class MainWindow extends EventEmitter {
         if (process.platform === 'linux') {
             this.win.on('resize', this.onResize);
         }
-
         this.win.webContents.on('before-input-event', this.onBeforeInputEvent);
 
         // Should not allow the main window to generate a window of its own
@@ -155,6 +149,12 @@ export class MainWindow extends EventEmitter {
         const contextMenu = new ContextMenu({}, this.win);
         contextMenu.reload();
 
+        const localURL = getLocalURLString('index.html');
+        this.win.loadURL(localURL).catch(
+            (reason) => {
+                log.error('failed to load', reason);
+            });
+
         this.emit(MAIN_WINDOW_CREATED);
     }
 
@@ -167,15 +167,11 @@ export class MainWindow extends EventEmitter {
     }
 
     show = () => {
-        if (this.win) {
+        if (this.win && this.isReady) {
-            if (this.win.isVisible()) {
+            this.win.show();
             this.win.focus();
-            } else {
-                this.win.show();
-            }
         } else {
             this.init();
-            this.show();
         }
     }
 
@@ -202,8 +198,6 @@ export class MainWindow extends EventEmitter {
         }
     }
 
-    onBrowserWindow = this.win?.on;
-
     sendToRenderer = (channel: string, ...args: unknown[]) => {
         this.sendToRendererWithRetry(3, channel, ...args);
     }
@@ -295,6 +289,7 @@ export class MainWindow extends EventEmitter {
         }
 
         this.emit(MAIN_WINDOW_RESIZED, this.getBounds());
+        this.emit(MAIN_WINDOW_FOCUSED);
     }
 
     private onBlur = () => {