From fc13f877f2a4fdb4988b568737dac645ceea2a0e Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Tue, 1 Mar 2022 12:35:27 -0500 Subject: [PATCH] [MM-30713] Stop Linux and Windows app from minimizing/hiding without user warning (#1988) * [MM-30713] Stop Linux app from minimizing/hiding without user warning * Added same behaviour for Windows * Update messages * Change wording * Fix for accidentally disabled setting --- src/common/config/defaultPreferences.ts | 2 +- src/common/config/index.ts | 6 ++ src/main/Validator.ts | 2 + src/main/windows/mainWindow.test.js | 101 +++++++++++++++++++++-- src/main/windows/mainWindow.ts | 36 ++++++-- src/renderer/components/SettingsPage.tsx | 4 +- src/types/config.ts | 2 + 7 files changed, 140 insertions(+), 13 deletions(-) diff --git a/src/common/config/defaultPreferences.ts b/src/common/config/defaultPreferences.ts index 3e318493d28..63f982d69b2 100644 --- a/src/common/config/defaultPreferences.ts +++ b/src/common/config/defaultPreferences.ts @@ -21,7 +21,7 @@ const defaultPreferences: ConfigV3 = { teams: [], showTrayIcon: true, trayIconTheme: 'use_system', - minimizeToTray: true, + minimizeToTray: process.platform !== 'linux', notifications: { flashWindow: 2, bounceIcon: true, diff --git a/src/common/config/index.ts b/src/common/config/index.ts index c27a047a840..35aa400e91b 100644 --- a/src/common/config/index.ts +++ b/src/common/config/index.ts @@ -275,6 +275,12 @@ export class Config extends EventEmitter { get lastActiveTeam() { return this.combinedData?.lastActiveTeam; } + get alwaysClose() { + return this.combinedData?.alwaysClose; + } + get alwaysMinimize() { + return this.combinedData?.alwaysMinimize; + } // initialization/processing methods diff --git a/src/main/Validator.ts b/src/main/Validator.ts index 40e691c21cc..7ffa8e19b73 100644 --- a/src/main/Validator.ts +++ b/src/main/Validator.ts @@ -125,6 +125,8 @@ const configDataSchemaV3 = Joi.object({ darkMode: Joi.boolean().default(false), downloadLocation: Joi.string(), lastActiveTeam: Joi.number().integer().min(0).default(0), + alwaysMinimize: Joi.boolean(), + alwaysClose: Joi.boolean(), }); // eg. data['community.mattermost.com'] = { data: 'certificate data', issuerName: 'COMODO RSA Domain Validation Secure Server CA'}; diff --git a/src/main/windows/mainWindow.test.js b/src/main/windows/mainWindow.test.js index 73f519948cf..3b3d174e859 100644 --- a/src/main/windows/mainWindow.test.js +++ b/src/main/windows/mainWindow.test.js @@ -5,7 +5,7 @@ import fs from 'fs'; import path from 'path'; -import {BrowserWindow, screen, app, globalShortcut} from 'electron'; +import {BrowserWindow, screen, app, globalShortcut, dialog} from 'electron'; import {SELECT_NEXT_TAB, SELECT_PREVIOUS_TAB} from 'common/communication'; import Config from 'common/config'; @@ -24,6 +24,10 @@ jest.mock('electron', () => ({ app: { getPath: jest.fn(), hide: jest.fn(), + quit: jest.fn(), + }, + dialog: { + showMessageBox: jest.fn(), }, BrowserWindow: jest.fn(), ipcMain: { @@ -38,7 +42,9 @@ jest.mock('electron', () => ({ }, })); -jest.mock('common/config', () => ({})); +jest.mock('common/config', () => ({ + set: jest.fn(), +})); jest.mock('common/utils/util', () => ({ isVersionGreaterThanOrEqualTo: jest.fn(), })); @@ -200,7 +206,7 @@ describe('main/windows/mainWindow', () => { expect(fs.writeFileSync).toHaveBeenCalled(); }); - it('should hide window on close for Windows if app wont quit', () => { + it('should hide window on close for Windows if app wont quit and config item is set', () => { const originalPlatform = process.platform; Object.defineProperty(process, 'platform', { value: 'win32', @@ -214,13 +220,67 @@ describe('main/windows/mainWindow', () => { }), }; BrowserWindow.mockImplementation(() => window); + Config.minimizeToTray = true; + Config.alwaysMinimize = true; createMainWindow({}); + Config.minimizeToTray = false; + Config.alwaysMinimize = false; Object.defineProperty(process, 'platform', { value: originalPlatform, }); expect(window.hide).toHaveBeenCalled(); }); + it('should close app on close window for Windows if app wont quit and config item is not set', () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { + value: 'win32', + }); + const window = { + ...baseWindow, + on: jest.fn().mockImplementation((event, cb) => { + if (event === 'close') { + cb({preventDefault: jest.fn()}); + } + }), + }; + BrowserWindow.mockImplementation(() => window); + Config.alwaysClose = true; + createMainWindow({}); + Config.alwaysClose = false; + Object.defineProperty(process, 'platform', { + value: originalPlatform, + }); + expect(app.quit).toHaveBeenCalled(); + }); + + it('should close app on Windows if window closed depending on user input', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { + value: 'win32', + }); + const window = { + ...baseWindow, + on: jest.fn().mockImplementation((event, cb) => { + if (event === 'close') { + cb({preventDefault: jest.fn()}); + } + }), + }; + BrowserWindow.mockImplementation(() => window); + dialog.showMessageBox.mockResolvedValue({response: 1}); + createMainWindow({}); + expect(app.quit).not.toHaveBeenCalled(); + const promise = Promise.resolve({response: 0}); + dialog.showMessageBox.mockImplementation(() => promise); + createMainWindow({}); + Object.defineProperty(process, 'platform', { + value: originalPlatform, + }); + await promise; + expect(app.quit).toHaveBeenCalled(); + }); + it('should hide window on close for Linux if app wont quit and config item is set', () => { const originalPlatform = process.platform; Object.defineProperty(process, 'platform', { @@ -236,15 +296,17 @@ describe('main/windows/mainWindow', () => { }; BrowserWindow.mockImplementation(() => window); Config.minimizeToTray = true; + Config.alwaysMinimize = true; createMainWindow({}); Config.minimizeToTray = false; + Config.alwaysMinimize = false; Object.defineProperty(process, 'platform', { value: originalPlatform, }); expect(window.hide).toHaveBeenCalled(); }); - it('should minimize window on close for Linux if app wont quit and config item is not set', () => { + it('should close app on close window for Linux if app wont quit and config item is not set', () => { const originalPlatform = process.platform; Object.defineProperty(process, 'platform', { value: 'linux', @@ -258,11 +320,40 @@ describe('main/windows/mainWindow', () => { }), }; BrowserWindow.mockImplementation(() => window); + Config.alwaysClose = true; + createMainWindow({}); + Config.alwaysClose = false; + Object.defineProperty(process, 'platform', { + value: originalPlatform, + }); + expect(app.quit).toHaveBeenCalled(); + }); + + it('should close app on linux if window closed depending on user input', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { + value: 'linux', + }); + const window = { + ...baseWindow, + on: jest.fn().mockImplementation((event, cb) => { + if (event === 'close') { + cb({preventDefault: jest.fn()}); + } + }), + }; + BrowserWindow.mockImplementation(() => window); + dialog.showMessageBox.mockResolvedValue({response: 1}); + createMainWindow({}); + expect(app.quit).not.toHaveBeenCalled(); + const promise = Promise.resolve({response: 0}); + dialog.showMessageBox.mockImplementation(() => promise); createMainWindow({}); Object.defineProperty(process, 'platform', { value: originalPlatform, }); - expect(window.minimize).toHaveBeenCalled(); + await promise; + expect(app.quit).toHaveBeenCalled(); }); it('should hide window on close for Mac if app wont quit and window is not full screen', () => { diff --git a/src/main/windows/mainWindow.ts b/src/main/windows/mainWindow.ts index d97576af546..2073a0f481d 100644 --- a/src/main/windows/mainWindow.ts +++ b/src/main/windows/mainWindow.ts @@ -5,7 +5,7 @@ import fs from 'fs'; import os from 'os'; -import {app, BrowserWindow, BrowserWindowConstructorOptions, globalShortcut, ipcMain, screen} from 'electron'; +import {app, BrowserWindow, BrowserWindowConstructorOptions, dialog, globalShortcut, ipcMain, screen} from 'electron'; import log from 'electron-log'; import {SavedWindowState} from 'types/mainWindow'; @@ -143,13 +143,39 @@ function createMainWindow(options: {linuxAppIcon: string}) { } switch (process.platform) { case 'win32': - hideWindow(mainWindow); - break; case 'linux': if (Config.minimizeToTray) { - hideWindow(mainWindow); + if (Config.alwaysMinimize) { + hideWindow(mainWindow); + } else { + dialog.showMessageBox(mainWindow, { + title: 'Minimize to Tray', + message: 'Mattermost will continue to run in the system tray. This can be disabled in Settings.', + type: 'info', + checkboxChecked: true, + checkboxLabel: 'Don\'t show again', + }).then((result: {response: number; checkboxChecked: boolean}) => { + Config.set('alwaysMinimize', result.checkboxChecked); + hideWindow(mainWindow); + }); + } + } else if (Config.alwaysClose) { + app.quit(); } else { - mainWindow.minimize(); + dialog.showMessageBox(mainWindow, { + title: 'Close Application', + message: 'Are you sure you want to quit?', + detail: 'You will no longer receive notifications for messages. If you want to leave Mattermost running in the system tray, you can enable this in Settings.', + type: 'question', + buttons: ['Yes', 'No'], + checkboxChecked: true, + checkboxLabel: 'Don\'t ask again', + }).then((result: {response: number; checkboxChecked: boolean}) => { + Config.set('alwaysClose', result.checkboxChecked && result.response === 0); + if (result.response === 0) { + app.quit(); + } + }); } break; case 'darwin': diff --git a/src/renderer/components/SettingsPage.tsx b/src/renderer/components/SettingsPage.tsx index 434e5988183..f28506bad5a 100644 --- a/src/renderer/components/SettingsPage.tsx +++ b/src/renderer/components/SettingsPage.tsx @@ -214,7 +214,7 @@ export default class SettingsPage extends React.PureComponent { - const shouldMinimizeToTray = this.state.showTrayIcon && this.minimizeToTrayRef.current?.checked; + const shouldMinimizeToTray = (process.platform === 'win32' || this.state.showTrayIcon) && this.minimizeToTrayRef.current?.checked; window.timers.setImmediate(this.saveSetting, CONFIG_TYPE_APP_OPTIONS, {key: 'minimizeToTray', data: shouldMinimizeToTray}); this.setState({ @@ -691,7 +691,7 @@ export default class SettingsPage extends React.PureComponent diff --git a/src/types/config.ts b/src/types/config.ts index fb6bcdd6162..92929ba69bf 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -40,6 +40,8 @@ export type ConfigV3 = { downloadLocation: string; spellCheckerURL?: string; lastActiveTeam?: number; + alwaysMinimize?: boolean; + alwaysClose?: boolean; } export type ConfigV2 = {