Skip to content

Commit

Permalink
[MM-30713] Stop Linux and Windows app from minimizing/hiding without …
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@devinbinnie
devinbinnie authored Mar 1, 2022
1 parent 062ca92 commit fc13f87
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/common/config/defaultPreferences.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const defaultPreferences: ConfigV3 = {
teams: [],
showTrayIcon: true,
trayIconTheme: 'use_system',
minimizeToTray: true,
minimizeToTray: process.platform !== 'linux',
notifications: {
flashWindow: 2,
bounceIcon: true,
Expand Down
6 changes: 6 additions & 0 deletions src/common/config/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions src/main/Validator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ const configDataSchemaV3 = Joi.object<ConfigV3>({
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'};
Expand Down
101 changes: 96 additions & 5 deletions src/main/windows/mainWindow.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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: {
Expand All @@ -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(),
}));
Expand Down Expand Up @@ -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',
Expand All @@ -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', {
Expand All @@ -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',
Expand All @@ -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', () => {
Expand Down
36 changes: 31 additions & 5 deletions src/main/windows/mainWindow.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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':
Expand Down
4 changes: 2 additions & 2 deletions src/renderer/components/SettingsPage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ export default class SettingsPage extends React.PureComponent<Record<string, nev
}

handleChangeMinimizeToTray = () => {
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({
Expand Down Expand Up @@ -691,7 +691,7 @@ export default class SettingsPage extends React.PureComponent<Record<string, nev
type='checkbox'
id='inputMinimizeToTray'
ref={this.minimizeToTrayRef}
disabled={!this.state.showTrayIcon}
disabled={process.platform !== 'win32' && !this.state.showTrayIcon}
checked={this.state.minimizeToTray}
onChange={this.handleChangeMinimizeToTray}
/>
Expand Down
2 changes: 2 additions & 0 deletions src/types/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ export type ConfigV3 = {
downloadLocation: string;
spellCheckerURL?: string;
lastActiveTeam?: number;
alwaysMinimize?: boolean;
alwaysClose?: boolean;
}

export type ConfigV2 = {
Expand Down

0 comments on commit fc13f87

Please sign in to comment.