From 3df22026abe069790ea1213b18c80c05b2530ed0 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Fri, 2 Mar 2018 15:59:39 -0500 Subject: [PATCH] UX Improvements: Global Menu & Copy Changes (#2078) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - [x] Removed ‘Restart Signal’ global menu item - [x] Change _Click to create contact…_ to _Start conversation…_ - [x] Move global menu (top-left kebab) into OS menu bar, i.e. **Settings** > **Preferences…** - [x] Add tests for OS menu bar templates - [x] Fix bug with **Window** menu on macOS when showing setup options - [x] Use _Title Case_ for all OS menu bar menu items for consistency commit dedf7c9af0de90980388559659df0d92a77b864c Author: Daniel Gasienica Date: Tue Feb 27 16:53:42 2018 -0500 Use ‘Title Case’ to be consistent with OS menus References: - Apple: - https://developer.apple.com/macos/human-interface-guidelines/menus/menu-anatomy/#menu-and-menu-item-titles - https://developer.apple.com/library/content/documentation/FinalCutProX/Conceptual/FxPlugHIG/TextStyleGuidelines/TextStyleGuidelines.html#//apple_ref/doc/uid/TP40013782-CH6-SW1 - https://titlecaseconverter.com/ commit 3286da29b334bd4526c587b17707c2f230cec8f5 Author: Daniel Gasienica Date: Tue Feb 27 16:36:50 2018 -0500 Fix bug for macOS ‘Window’ menu with setup options commit 236a23d1eafe2a16073394a27b9013298b682a25 Author: Daniel Gasienica Date: Tue Feb 27 16:27:46 2018 -0500 Test menus with included setup options commit c5d5f5abb8d7f52d6a4aa182a86c92f7ddceade0 Author: Daniel Gasienica Date: Tue Feb 27 16:10:27 2018 -0500 Move settings (‘Preferences’) into OS-level menu This reduces our reliance on custom UI until we have more design resources. commit 027803f8f4983cffa443f0beff1854dcf541689b Author: Daniel Gasienica Date: Tue Feb 27 16:02:56 2018 -0500 Prepare tests for menu with/without included setup commit 9e2f006924b85eb249a8a1261c1c4dd1a706afa6 Author: Daniel Gasienica Date: Tue Feb 27 15:55:46 2018 -0500 Destructure `includeSetup` commit 6b2a1eccdf724fd722e58415d2700da73942d9e8 Author: Daniel Gasienica Date: Tue Feb 27 15:55:14 2018 -0500 :abc: `createTemplate` `options` commit c2fecba34b153fed106f414ed3347d46299f6fe5 Author: Daniel Gasienica Date: Tue Feb 27 12:49:55 2018 -0500 Test menu for Windows and Linux commit 60281b1af9ad1f022cdbc40711ebd0b688a7355d Author: Daniel Gasienica Date: Tue Feb 27 12:40:39 2018 -0500 Add `yarn run test-app` command commit 1a0489919c0a97b03fe88196260fef894fb3d9e4 Author: Daniel Gasienica Date: Tue Feb 27 12:40:29 2018 -0500 Add test for `SignalMenu.createTemplate` on macOS commit 9638b86c0f00f231e44562a5aa01626f0e5fdd8b Author: Daniel Gasienica Date: Tue Feb 27 12:34:46 2018 -0500 Make `createTemplate` pure Extracting `options.platform` makes it easier to test without having to stub `process.platform`. commit 9c26404892d7c9a7bd0199a9e8367a165a3b365c Author: Daniel Gasienica Date: Tue Feb 27 11:47:39 2018 -0500 Extract `locale.load` `appLocale` & `logger` for testability This allows us to run this code in a non-Electron environment, e.g. Node.js Mocha test suite. commit 710b22438df25c8d5e8431845a035c55ec8fc0b7 Author: Daniel Gasienica Date: Tue Feb 27 11:46:13 2018 -0500 :abc: npm scripts commit 9ae22937fbce078f91443023b560b3c0468c1380 Author: Daniel Gasienica Date: Tue Feb 27 11:45:30 2018 -0500 Use 2-space indendation for `app` module tests commit 22c26baf6159bd2e1f5a787c10e2260f09395329 Author: Daniel Gasienica Date: Tue Feb 27 11:22:55 2018 -0500 Prefer named exports commit 9c9526195266ac77ac2ca04135a1e675f617dfd2 Author: Daniel Gasienica Date: Tue Feb 27 11:22:46 2018 -0500 :abc: Organize `require`s commit 2f144d24d9e9a9ef72fe418996e3c911b304b00a Author: Daniel Gasienica Date: Tue Feb 27 11:13:50 2018 -0500 Remove existing global hamburger menu This will be replaced by a OS-level ‘Preferences’ menu. commit f5adb374cb742e5f319ececda8ab6d8adee88d7e Author: Daniel Gasienica Date: Mon Feb 26 18:40:54 2018 -0500 Remove ‘Restart Signal’ menu from settings Apparently, this is a remnant from the Chrome web application. commit d7a206bc8e67ef44022085e804ca040ed1b219f7 Author: Daniel Gasienica Date: Mon Feb 26 17:16:49 2018 -0500 Clarify label for starting a new conversation When user a enters a number that is not a contact, we prompt them to start a new conversation. commit 715a4064367fb61d85c1a4f9d48261b2ce002435 Author: Daniel Gasienica Date: Mon Feb 26 16:46:26 2018 -0500 Use ‘Enter name or number’ as prompt’ This follows implementation of Android and recommendation from Alissa. --- .editorconfig | 2 +- _locales/en/messages.json | 32 ++-- app/locale.js | 18 +- app/menu.js | 49 ++++- background.html | 11 -- js/background.js | 10 + js/views/conversation_search_view.js | 2 +- js/views/inbox_view.js | 15 -- main.js | 20 +- package.json | 5 +- preload.js | 4 + test/app/fixtures/menu-mac-os-setup.json | 180 ++++++++++++++++++ test/app/fixtures/menu-mac-os.json | 167 ++++++++++++++++ .../fixtures/menu-windows-linux-setup.json | 135 +++++++++++++ test/app/fixtures/menu-windows-linux.json | 124 ++++++++++++ test/app/menu_test.js | 78 ++++++++ test/index.html | 11 -- 17 files changed, 783 insertions(+), 80 deletions(-) create mode 100644 test/app/fixtures/menu-mac-os-setup.json create mode 100644 test/app/fixtures/menu-mac-os.json create mode 100644 test/app/fixtures/menu-windows-linux-setup.json create mode 100644 test/app/fixtures/menu-windows-linux.json create mode 100644 test/app/menu_test.js diff --git a/.editorconfig b/.editorconfig index 5878c72c8..5bfccc902 100644 --- a/.editorconfig +++ b/.editorconfig @@ -10,5 +10,5 @@ indent_style = space insert_final_newline = true trim_trailing_whitespace = true -[{js/modules/**/*.js, test/modules/**/*.js}] +[{js/modules/**/*.js, test/app/**/*.js, test/modules/**/*.js}] indent_size = 2 diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 814fff7be..8b27ca197 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -19,16 +19,20 @@ "message": "&Help", "description": "The label that is used for the Help menu in the program main menu. The '&' indicates that the following letter will be used as the keyboard 'shortcut letter' for accessing the menu with the Alt- combination." }, + "mainMenuSettings": { + "message": "Preferences…", + "description": "The label that is used for the Preferences menu in the program main menu. This should be consistent with the standard naming for ‘Preferences’ on the operating system." + }, "menuSetupWithImport": { - "message": "Set up with import", + "message": "Set Up with Import", "description": "When the application is not yet set up, menu option to start up the import sequence" }, "menuSetupAsNewDevice": { - "message": "Set up as new device", + "message": "Set Up as New Device", "description": "When the application is not yet set up, menu option to start up the set up as fresh device" }, "menuSetupAsStandalone": { - "message": "Set up as standalone device", + "message": "Set Up as Standalone Device", "description": "Only available on development modes, menu option to open up the standalone device setup sequence" }, "loading": { @@ -368,22 +372,22 @@ }, "debugLog": { "message": "Debug Log", - "description": "View menu item to open the debug log, capitalized" + "description": "View menu item to open the debug log (title case)" }, "goToReleaseNotes": { - "message": "Go to release notes", + "message": "Go to Release Notes", "description": "" }, "goToForums": { - "message": "Go to forums", + "message": "Go to Forums", "description": "Item under the Help menu, takes you to the forums" }, "goToSupportPage": { - "message": "Go to support page", + "message": "Go to Support Page", "description": "Item under the Help menu, takes you to the support page" }, "fileABug": { - "message": "File a bug", + "message": "File a Bug", "description": "Item under the Help menu, takes you to GitHub new issue form" }, "aboutSignalDesktop": { @@ -411,7 +415,7 @@ "description": "Tooltip for the tray icon" }, "searchForPeopleOrGroups": { - "message": "Search...", + "message": "Enter name or number", "description": "Placeholder text in the search input" }, "welcomeToSignal": { @@ -595,10 +599,6 @@ "message": "New Messages", "description": "Displayed in notifications for multiple messages" }, - "restartSignal": { - "message": "Restart Signal", - "description": "Menu item for restarting the program." - }, "messageNotSent": { "message": "Message not sent.", "description": "Informational label, appears on messages that failed to send" @@ -835,9 +835,9 @@ "message": "Hide menu bar", "description": "Label text for menu bar visibility setting" }, - "newContact": { - "message": "Click to create new contact", - "description": "" + "startConversation": { + "message": "Start conversation…", + "description": "Label underneath number a user enters that is not an existing contact" }, "newPhoneNumber": { "message": "Enter a phone number to add a contact.", diff --git a/app/locale.js b/app/locale.js index fddce1f27..b2df2fe00 100644 --- a/app/locale.js +++ b/app/locale.js @@ -1,9 +1,7 @@ const path = require('path'); const fs = require('fs'); -const { app } = require('electron'); const _ = require('lodash'); -const logging = require('./logging'); function normalizeLocaleName(locale) { if (/^en-/.test(locale)) { @@ -27,15 +25,17 @@ function getLocaleMessages(locale) { return JSON.parse(fs.readFileSync(targetFile, 'utf-8')); } -function load() { - const logger = logging.getLogger(); - const english = getLocaleMessages('en'); - let appLocale = app.getLocale(); - - if (process.env.NODE_ENV === 'test') { - appLocale = 'en'; +function load({ appLocale, logger } = {}) { + if (!appLocale) { + throw new TypeError('`appLocale` is required'); } + if (!logger || !logger.error) { + throw new TypeError('`logger.error` is required'); + } + + const english = getLocaleMessages('en'); + // Load locale - if we can't load messages for the current locale, we // default to 'en' // diff --git a/app/menu.js b/app/menu.js index 5ed1b86aa..11912e202 100644 --- a/app/menu.js +++ b/app/menu.js @@ -1,19 +1,37 @@ -function createTemplate(options, messages) { +const isString = require('lodash/isString'); + + +exports.createTemplate = (options, messages) => { + if (!isString(options.platform)) { + throw new TypeError('`options.platform` must be a string'); + } + const { + includeSetup, openForums, openNewBugForm, openReleaseNotes, openSupportPage, + platform, setupAsNewDevice, setupAsStandalone, setupWithImport, showAbout, showDebugLog, + showSettings, } = options; const template = [{ label: messages.mainMenuFile.message, submenu: [ + { + label: messages.mainMenuSettings.message, + accelerator: 'CommandOrControl+,', + click: showSettings, + }, + { + type: 'separator', + }, { role: 'quit', }, @@ -126,7 +144,7 @@ function createTemplate(options, messages) { ], }]; - if (options.includeSetup) { + if (includeSetup) { const fileMenu = template[0]; // These are in reverse order, since we're prepending them one at a time @@ -137,6 +155,9 @@ function createTemplate(options, messages) { }); } + fileMenu.submenu.unshift({ + type: 'separator', + }); fileMenu.submenu.unshift({ label: messages.menuSetupAsNewDevice.message, click: setupAsNewDevice, @@ -147,19 +168,21 @@ function createTemplate(options, messages) { }); } - if (process.platform === 'darwin') { + if (platform === 'darwin') { return updateForMac(template, messages, options); } return template; -} +}; function updateForMac(template, messages, options) { const { + includeSetup, setupAsNewDevice, setupAsStandalone, setupWithImport, showAbout, + showSettings, showWindow, } = options; @@ -170,7 +193,7 @@ function updateForMac(template, messages, options) { // Remove File menu template.shift(); - if (options.includeSetup) { + if (includeSetup) { // Add a File menu just for these setup options. Because we're using unshift(), we add // the file menu first, though it ends up to the right of the Signal Desktop menu. const fileMenu = { @@ -207,6 +230,14 @@ function updateForMac(template, messages, options) { { type: 'separator', }, + { + label: messages.mainMenuSettings.message, + accelerator: 'CommandOrControl+,', + click: showSettings, + }, + { + type: 'separator', + }, { role: 'hide', }, @@ -226,7 +257,7 @@ function updateForMac(template, messages, options) { }); // Add to Edit menu - const editIndex = options.includeSetup ? 2 : 1; + const editIndex = includeSetup ? 2 : 1; template[editIndex].submenu.push( { type: 'separator', @@ -245,9 +276,9 @@ function updateForMac(template, messages, options) { ); // Replace Window menu - const windowIndex = options.includeSetup ? 4 : 3; + const windowMenuTemplateIndex = includeSetup ? 4 : 3; // eslint-disable-next-line no-param-reassign - template[windowIndex].submenu = [ + template[windowMenuTemplateIndex].submenu = [ { accelerator: 'CmdOrCtrl+W', role: 'close', @@ -273,5 +304,3 @@ function updateForMac(template, messages, options) { return template; } - -module.exports = createTemplate; diff --git a/background.html b/background.html index 10233e918..388dd921b 100644 --- a/background.html +++ b/background.html @@ -69,17 +69,6 @@