Move Settings Window into Modal (#3007)

* Move Settings Window into modal

* Re-add for E2E tests

src/main/app/config.ts

@@ -11,7 +11,6 @@ import {setUnreadBadgeSetting} from 'main/badge';
 import Tray from 'main/tray/tray';
 import LoadingScreen from 'main/views/loadingScreen';
 import MainWindow from 'main/windows/mainWindow';
-import SettingsWindow from 'main/windows/settingsWindow';
 
 import type {CombinedConfig, Config as ConfigType} from 'types/config';
 
@@ -73,7 +72,7 @@ export function handleConfigUpdate(newConfig: CombinedConfig) {
 
     if (app.isReady()) {
         MainWindow.sendToRenderer(RELOAD_CONFIGURATION);
-        SettingsWindow.sendToRenderer(RELOAD_CONFIGURATION);
+        ipcMain.emit(EMIT_CONFIGURATION, true, Config.data);
     }
 
     setUnreadBadgeSetting(newConfig && newConfig.showUnreadBadge);
@@ -113,7 +112,6 @@ export function handleDarkModeChange(darkMode: boolean) {
 
     Tray.refreshImages(Config.trayIconTheme);
     MainWindow.sendToRenderer(DARK_MODE_CHANGE, darkMode);
-    SettingsWindow.sendToRenderer(DARK_MODE_CHANGE, darkMode);
     LoadingScreen.setDarkMode(darkMode);
 
     ipcMain.emit(EMIT_CONFIGURATION, true, Config.data);

src/main/app/initialize.test.js

@@ -166,9 +166,6 @@ jest.mock('main/views/viewManager', () => ({
     getViewByWebContentsId: jest.fn(),
     handleDeepLink: jest.fn(),
 }));
-jest.mock('main/windows/settingsWindow', () => ({
-    show: jest.fn(),
-}));
 jest.mock('main/windows/mainWindow', () => ({
     get: jest.fn(),
     show: jest.fn(),

src/main/app/initialize.ts

@@ -31,6 +31,7 @@ import {
     DOUBLE_CLICK_ON_WINDOW,
     TOGGLE_SECURE_INPUT,
     GET_APP_INFO,
+    SHOW_SETTINGS_WINDOW,
 } from 'common/communication';
 import Config from 'common/config';
 import {Logger} from 'common/log';
@@ -79,6 +80,7 @@ import {
     handleQuit,
     handlePingDomain,
     handleToggleSecureInput,
+    handleShowSettingsModal,
 } from './intercom';
 import {
     clearAppCache,
@@ -281,6 +283,10 @@ function initializeInterCommunicationEventListeners() {
     ipcMain.on(DOUBLE_CLICK_ON_WINDOW, handleDoubleClick);
 
     ipcMain.on(TOGGLE_SECURE_INPUT, handleToggleSecureInput);
+
+    if (process.env.NODE_ENV === 'test') {
+        ipcMain.on(SHOW_SETTINGS_WINDOW, handleShowSettingsModal);
+    }
 }
 
 async function initializeAfterAppReady() {

src/main/app/intercom.ts

@@ -160,3 +160,18 @@ export function handleToggleSecureInput(event: IpcMainEvent, secureInput: boolea
     log.debug('handleToggleSecureInput', secureInput);
     app.setSecureKeyboardEntryEnabled(secureInput);
 }
+
+export function handleShowSettingsModal() {
+    const mainWindow = MainWindow.get();
+    if (!mainWindow) {
+        return;
+    }
+
+    ModalManager.addModal(
+        'settingsModal',
+        getLocalURLString('settings.html'),
+        getLocalPreload('internalAPI.js'),
+        null,
+        mainWindow,
+    );
+}

src/main/menus/app.test.js

@@ -79,10 +79,8 @@ jest.mock('main/downloadsManager', () => ({
 }));
 jest.mock('main/views/viewManager', () => ({}));
 jest.mock('main/windows/mainWindow', () => ({
-    sendToRenderer: jest.fn(),
+    get: jest.fn(),
-    on: jest.fn(),
 }));
-jest.mock('main/windows/settingsWindow', () => ({}));
 jest.mock('common/views/View', () => ({
     getViewDisplayName: (name) => name,
 }));
@@ -93,6 +91,9 @@ jest.mock('main/AutoLauncher', () => ({
 jest.mock('main/windows/callsWidgetWindow', () => ({
     isOpen: jest.fn(),
 }));
+jest.mock('main/views/modalManager', () => ({
+    addModal: jest.fn(),
+}));
 
 describe('main/menus/app', () => {
     const config = {

src/main/menus/app.ts

@@ -18,9 +18,11 @@ import type {UpdateManager} from 'main/autoUpdater';
 import Diagnostics from 'main/diagnostics';
 import downloadsManager from 'main/downloadsManager';
 import {localizeMessage} from 'main/i18nManager';
+import {getLocalPreload, getLocalURLString} from 'main/utils';
+import ModalManager from 'main/views/modalManager';
 import ViewManager from 'main/views/viewManager';
 import CallsWidgetWindow from 'main/windows/callsWidgetWindow';
-import SettingsWindow from 'main/windows/settingsWindow';
+import MainWindow from 'main/windows/mainWindow';
 
 export function createTemplate(config: Config, updateManager: UpdateManager) {
     const separatorItem: MenuItemConstructorOptions = {
@@ -48,7 +50,18 @@ export function createTemplate(config: Config, updateManager: UpdateManager) {
         label: settingsLabel,
         accelerator: 'CmdOrCtrl+,',
         click() {
-            SettingsWindow.show();
+            const mainWindow = MainWindow.get();
+            if (!mainWindow) {
+                return;
+            }
+
+            ModalManager.addModal(
+                'settingsModal',
+                getLocalURLString('settings.html'),
+                getLocalPreload('internalAPI.js'),
+                null,
+                mainWindow,
+            );
         },
     });
 

src/main/menus/tray.test.js

@@ -17,7 +17,13 @@ jest.mock('common/servers/serverManager', () => ({
 jest.mock('app/serverViewState', () => ({
     switchServer: jest.fn(),
 }));
-jest.mock('main/windows/settingsWindow', () => ({}));
+jest.mock('main/windows/mainWindow', () => ({
+    sendToRenderer: jest.fn(),
+    on: jest.fn(),
+}));
+jest.mock('main/views/modalManager', () => ({
+    addModal: jest.fn(),
+}));
 
 describe('main/menus/tray', () => {
     it('should show the first 9 servers (using order)', () => {

src/main/menus/tray.ts

@@ -9,7 +9,9 @@ import {Menu} from 'electron';
 import ServerViewState from 'app/serverViewState';
 import ServerManager from 'common/servers/serverManager';
 import {localizeMessage} from 'main/i18nManager';
-import SettingsWindow from 'main/windows/settingsWindow';
+import {getLocalPreload, getLocalURLString} from 'main/utils';
+import ModalManager from 'main/views/modalManager';
+import MainWindow from 'main/windows/mainWindow';
 
 export function createTemplate() {
     const servers = ServerManager.getOrderedServers();
@@ -26,7 +28,18 @@ export function createTemplate() {
         }, {
             label: process.platform === 'darwin' ? localizeMessage('main.menus.tray.preferences', 'Preferences...') : localizeMessage('main.menus.tray.settings', 'Settings'),
             click: () => {
-                SettingsWindow.show();
+                const mainWindow = MainWindow.get();
+                if (!mainWindow) {
+                    return;
+                }
+
+                ModalManager.addModal(
+                    'settingsModal',
+                    getLocalURLString('settings.html'),
+                    getLocalPreload('internalAPI.js'),
+                    null,
+                    mainWindow,
+                );
             },
         }, {
             type: 'separator',

src/main/tray/tray.ts

@@ -10,7 +10,6 @@ import {UPDATE_APPSTATE_TOTALS} from 'common/communication';
 import {Logger} from 'common/log';
 import {localizeMessage} from 'main/i18nManager';
 import MainWindow from 'main/windows/mainWindow';
-import SettingsWindow from 'main/windows/settingsWindow';
 
 const assetsDir = path.resolve(app.getAppPath(), 'assets');
 const log = new Logger('Tray');
@@ -144,12 +143,7 @@ export class TrayIcon {
             mainWindow.show();
         }
 
-        const settingsWindow = SettingsWindow.get();
-        if (settingsWindow) {
-            settingsWindow.focus();
-        } else {
         mainWindow.focus();
-        }
     };
 
     private onAppStateUpdate = (anyExpired: boolean, anyMentions: number, anyUnreads: boolean) => {

src/main/views/modalManager.ts

@@ -15,6 +15,7 @@ import {
     DARK_MODE_CHANGE,
     GET_MODAL_UNCLOSEABLE,
     MAIN_WINDOW_RESIZED,
+    RELOAD_CONFIGURATION,
 } from 'common/communication';
 import {Logger} from 'common/log';
 import {getAdjustedWindowBoundaries} from 'main/utils';
@@ -152,6 +153,7 @@ export class ModalManager {
         }
 
         this.modalQueue.forEach((modal) => {
+            modal.view.webContents.send(RELOAD_CONFIGURATION);
             modal.view.webContents.send(DARK_MODE_CHANGE, config.darkMode);
         });
     };

src/main/windows/settingsWindow.ts

@@ -1,83 +0,0 @@
-// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
-// See LICENSE.txt for license information.
-
-import {BrowserWindow, ipcMain} from 'electron';
-
-import {SHOW_SETTINGS_WINDOW} from 'common/communication';
-import Config from 'common/config';
-import {Logger} from 'common/log';
-
-import MainWindow from './mainWindow';
-
-import ContextMenu from '../contextMenu';
-import {getLocalPreload, getLocalURLString} from '../utils';
-
-const log = new Logger('SettingsWindow');
-
-export class SettingsWindow {
-    private win?: BrowserWindow;
-
-    constructor() {
-        ipcMain.on(SHOW_SETTINGS_WINDOW, this.show);
-    }
-
-    show = () => {
-        if (this.win) {
-            this.win.show();
-        } else {
-            this.create();
-        }
-    };
-
-    get = () => {
-        return this.win;
-    };
-
-    sendToRenderer = (channel: string, ...args: any[]) => {
-        this.win?.webContents.send(channel, ...args);
-    };
-
-    private create = () => {
-        const mainWindow = MainWindow.get();
-        if (!mainWindow) {
-            return;
-        }
-
-        const preload = getLocalPreload('internalAPI.js');
-        const spellcheck = (typeof Config.useSpellChecker === 'undefined' ? true : Config.useSpellChecker);
-        this.win = new BrowserWindow({
-            title: 'Desktop App Settings',
-            fullscreen: false,
-            webPreferences: {
-                preload,
-                spellcheck,
-            }});
-
-        const contextMenu = new ContextMenu({}, this.win);
-        contextMenu.reload();
-
-        const localURL = getLocalURLString('settings.html');
-        this.win.setMenuBarVisibility(false);
-        this.win.loadURL(localURL).catch(
-            (reason) => {
-                log.error('failed to load', reason);
-            });
-        this.win.show();
-
-        if (Boolean(process.env.MM_DEBUG_SETTINGS) || false) {
-            this.win.webContents.openDevTools({mode: 'detach'});
-        }
-
-        this.win.on('closed', () => {
-            delete this.win;
-
-            // For some reason, on macOS, the app will hard crash when the settings window is closed
-            // It seems to be related to calling view.focus() and there's no log output unfortunately
-            // Adding this arbitrary delay seems to get rid of it (it happens very frequently)
-            setTimeout(() => MainWindow.get()?.focus(), 10);
-        });
-    };
-}
-
-const settingsWindow = new SettingsWindow();
-export default settingsWindow;

src/renderer/components/SettingsPage.tsx

@@ -7,7 +7,7 @@
 import 'renderer/css/settings.css';
 
 import React from 'react';
-import {FormCheck, Col, FormGroup, FormText, Container, Row, Button, FormControl} from 'react-bootstrap';
+import {FormCheck, Col, FormGroup, FormText, Container, Row, Button, FormControl, Modal} from 'react-bootstrap';
 import type {IntlShape} from 'react-intl';
 import {FormattedMessage, injectIntl} from 'react-intl';
 import type {ActionMeta, MultiValue} from 'react-select';
@@ -25,6 +25,8 @@ const CONFIG_TYPE_UPDATES = 'updates';
 const CONFIG_TYPE_APP_OPTIONS = 'appOptions';
 
 type Props = {
+    show: boolean;
+    onClose: () => void;
     intl: IntlShape;
 }
 
@@ -1228,12 +1230,21 @@ class SettingsPage extends React.PureComponent<Props, State> {
         }
 
         return (
-            <div
+            <Modal
-                className='container-fluid'
+                show={this.props.show}
-                style={{
+                id='settingsModal'
-                    height: '100%',
+                onHide={this.props.onClose}
-                }}
             >
+                <Modal.Header closeButton={true}>
+                    <Modal.Title>
+                        <FormattedMessage
+                            id='renderer.components.settingsPage.header'
+                            defaultMessage='Settings'
+                        />
+                    </Modal.Title>
+                </Modal.Header>
+
+                <Modal.Body>
                     <div
                         style={{
                             overflowY: 'auto',
@@ -1241,22 +1252,14 @@ class SettingsPage extends React.PureComponent<Props, State> {
                             margin: '0 -15px',
                         }}
                     >
-                    <div style={{position: 'relative'}}>
-                        <h1 style={settingsPage.heading}>
-                            <FormattedMessage
-                                id='renderer.components.settingsPage.header'
-                                defaultMessage='Settings'
-                            />
-                        </h1>
-                        <hr/>
-                    </div>
                         <Container
                             className='settingsPage'
                         >
                             {waitForIpc}
                         </Container>
                     </div>
-            </div>
+                </Modal.Body>
+            </Modal>
         );
     }
 }

src/renderer/css/lazy/modals-dark.lazy.css

@@ -3,3 +3,26 @@
 body {
     background-color: transparent;
 }
+
+.SettingsPage__spellCheckerLocalesDropdown .SettingsPage__spellCheckerLocalesDropdown__control {
+    background: #242a30;
+}
+
+.SettingsPage__spellCheckerLocalesDropdown .SettingsPage__spellCheckerLocalesDropdown__menu {
+    background: #242a30;
+}
+
+.SettingsPage__spellCheckerLocalesDropdown .SettingsPage__spellCheckerLocalesDropdown__option {
+    background: #242a30;
+}
+
+.SettingsPage__spellCheckerLocalesDropdown .SettingsPage__spellCheckerLocalesDropdown__option:hover {
+    background: rgba(255, 255, 255, 0.16);
+}
+
+#settingsModal .modal-body  {
+    overflow-y: auto;
+    overflow-x: hidden;
+    scrollbar-width: thin;
+    scrollbar-color: var(--light) rgba(255, 255, 255, 0);
+}

src/renderer/css/lazy/settings-dark.lazy.css

@@ -1,21 +0,0 @@
-@import '~bootstrap-dark/src/bootstrap-dark.css';
-
-.ServerListItem:hover {
-    background: #242a30;
-}
-
-.SettingsPage__spellCheckerLocalesDropdown .SettingsPage__spellCheckerLocalesDropdown__control {
-    background: #242a30;
-}
-
-.SettingsPage__spellCheckerLocalesDropdown .SettingsPage__spellCheckerLocalesDropdown__menu {
-    background: #242a30;
-}
-
-.SettingsPage__spellCheckerLocalesDropdown .SettingsPage__spellCheckerLocalesDropdown__option {
-    background: #242a30;
-}
-
-.SettingsPage__spellCheckerLocalesDropdown .SettingsPage__spellCheckerLocalesDropdown__option:hover {
-    background: rgba(255, 255, 255, 0.16);
-}

src/renderer/css/settings.css

@@ -41,3 +41,25 @@ body {
     margin-left: 16px;
     width: 50%;
 }
+
+#settingsModal .modal-content  {
+    padding: 16px;
+    max-height: calc(100vh - 50px);
+}
+
+#settingsModal .modal-body  {
+    overflow-y: auto;
+    overflow-x: hidden;
+    scrollbar-width: thin;
+    scrollbar-color: var(--dark) rgba(255, 255, 255, 0);
+}
+
+#settingsModal .modal-header  {
+    align-items: center;
+}
+
+@media (min-width: 862px) {
+    #settingsModal {
+        max-width: 786px;
+    }
+}

src/renderer/modals/settings/settings.tsx

@@ -1,4 +1,3 @@
-// Copyright (c) 2015-2016 Yuya Ochiai
 // Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
@@ -9,27 +8,24 @@ import 'renderer/css/settings.css';
 import React from 'react';
 import ReactDOM from 'react-dom';
 
-import darkStyles from 'renderer/css/lazy/settings-dark.lazy.css';
+import SettingsPage from '../../components/SettingsPage';
+import IntlProvider from '../../intl_provider';
+import setupDarkMode from '../darkMode';
 
-import SettingsPage from './components/SettingsPage';
+setupDarkMode();
-import IntlProvider from './intl_provider';
 
-const setDarkMode = (darkMode: boolean) => {
+const onClose = () => {
-    if (darkMode) {
+    window.desktop.modals.finishModal();
-        darkStyles.use();
-    } else {
-        darkStyles.unuse();
-    }
 };
 
-window.desktop.onDarkModeChange((darkMode) => setDarkMode(darkMode));
-window.desktop.getDarkMode().then(setDarkMode);
-
 const start = async () => {
     ReactDOM.render(
         (
             <IntlProvider>
-                <SettingsPage/>
+                <SettingsPage
+                    show={true}
+                    onClose={onClose}
+                />
             </IntlProvider>
         )
         ,

webpack.config.renderer.js

@@ -13,7 +13,7 @@ const base = require('./webpack.config.base');
 module.exports = merge(base, {
     entry: {
         index: './src/renderer/index.tsx',
-        settings: './src/renderer/settings.tsx',
+        settings: './src/renderer/modals/settings/settings.tsx',
         dropdown: './src/renderer/dropdown.tsx',
         downloadsDropdownMenu: './src/renderer/downloadsDropdownMenu.tsx',
         downloadsDropdown: './src/renderer/downloadsDropdown.tsx',