From 7727dc093e4aafda3b0f8ff7481fae1d20913434 Mon Sep 17 00:00:00 2001 From: Derek Meer Date: Mon, 8 Apr 2019 17:25:14 +0000 Subject: [PATCH] Hides the "Hide menu bar" option on MacOS (#2903) The "Hide menu bar" option is only applicable to Windows and some Linux distros, where the menu bar is attached to the Signal window. Therefore, this commit ensures that it doesn't show up on MacOS. It includes a setting, isHideMenuBarSupported(), to control the option's appearance. This commit also includes the tests to make sure isHideMenuBarSupported() works correctly. Fixes #2705 --- js/views/settings_view.js | 15 +++++---- settings.html | 2 ++ ts/test/types/Settings_test.ts | 61 ++++++++++++++++++++++++++++++++++ ts/types/Settings.ts | 3 ++ 4 files changed, 75 insertions(+), 6 deletions(-) diff --git a/js/views/settings_view.js b/js/views/settings_view.js index 58818531e97b..e17304a294d7 100644 --- a/js/views/settings_view.js +++ b/js/views/settings_view.js @@ -106,12 +106,14 @@ value: window.initialData.spellCheck, setFn: window.setSpellCheck, }); - new CheckboxView({ - el: this.$('.menu-bar-setting'), - name: 'menu-bar-setting', - value: window.initialData.hideMenuBar, - setFn: window.setHideMenuBar, - }); + if (Settings.isHideMenuBarSupported()) { + new CheckboxView({ + el: this.$('.menu-bar-setting'), + name: 'menu-bar-setting', + value: window.initialData.hideMenuBar, + setFn: window.setHideMenuBar, + }); + } new MediaPermissionsSettingView({ el: this.$('.media-permissions'), value: window.initialData.mediaPermissions, @@ -140,6 +142,7 @@ nameOnly: i18n('nameOnly'), audioNotificationDescription: i18n('audioNotificationDescription'), isAudioNotificationSupported: Settings.isAudioNotificationSupported(), + isHideMenuBarSupported: Settings.isHideMenuBarSupported(), themeLight: i18n('themeLight'), themeDark: i18n('themeDark'), hideMenuBar: i18n('hideMenuBar'), diff --git a/settings.html b/settings.html index f74ccb55547e..bf14f2444012 100644 --- a/settings.html +++ b/settings.html @@ -54,10 +54,12 @@
+ {{ #isHideMenuBarSupported }} + {{ /isHideMenuBarSupported }}

{{ notifications }}

diff --git a/ts/test/types/Settings_test.ts b/ts/test/types/Settings_test.ts index d54f9107b420..714b1342ed39 100644 --- a/ts/test/types/Settings_test.ts +++ b/ts/test/types/Settings_test.ts @@ -130,4 +130,65 @@ describe('Settings', () => { }); }); }); + describe('isHideMenuBarSupported', () => { + context('on macOS', () => { + beforeEach(() => { + sandbox.stub(process, 'platform').value('darwin'); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it('should return false', () => { + assert.isFalse(Settings.isHideMenuBarSupported()); + }); + }); + + context('on Windows', () => { + context('version 7', () => { + beforeEach(() => { + sandbox.stub(process, 'platform').value('win32'); + sandbox.stub(os, 'release').returns('7.0.0'); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it('should return true', () => { + assert.isTrue(Settings.isHideMenuBarSupported()); + }); + }); + + context('version 8+', () => { + beforeEach(() => { + sandbox.stub(process, 'platform').value('win32'); + sandbox.stub(os, 'release').returns('8.0.0'); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it('should return true', () => { + assert.isTrue(Settings.isHideMenuBarSupported()); + }); + }); + }); + + context('on Linux', () => { + beforeEach(() => { + sandbox.stub(process, 'platform').value('linux'); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it('should return true', () => { + assert.isTrue(Settings.isHideMenuBarSupported()); + }); + }); + }); }); diff --git a/ts/types/Settings.ts b/ts/types/Settings.ts index 737fad3d3e24..5231bdf0fa80 100644 --- a/ts/types/Settings.ts +++ b/ts/types/Settings.ts @@ -9,3 +9,6 @@ export const isAudioNotificationSupported = () => // https://github.com/electron/electron/issues/11189 export const isNotificationGroupingSupported = () => !OS.isWindows() || OS.isWindows(MIN_WINDOWS_VERSION); + +// the "hide menu bar" option is specific to Windows and Linux +export const isHideMenuBarSupported = () => !OS.isMacOS();