diff --git a/default_app/main.js b/default_app/main.js index 8b2fb1bbf..dd887e0de 100644 --- a/default_app/main.js +++ b/default_app/main.js @@ -52,46 +52,30 @@ app.once('ready', () => { label: 'Edit', submenu: [ { - label: 'Undo', - accelerator: 'CmdOrCtrl+Z', role: 'undo' }, { - label: 'Redo', - accelerator: 'Shift+CmdOrCtrl+Z', role: 'redo' }, { type: 'separator' }, { - label: 'Cut', - accelerator: 'CmdOrCtrl+X', role: 'cut' }, { - label: 'Copy', - accelerator: 'CmdOrCtrl+C', role: 'copy' }, { - label: 'Paste', - accelerator: 'CmdOrCtrl+V', role: 'paste' }, { - label: 'Paste and Match Style', - accelerator: 'Shift+Command+V', role: 'pasteandmatchstyle' }, { - label: 'Delete', - accelerator: 'Delete', role: 'delete' }, { - label: 'Select All', - accelerator: 'CmdOrCtrl+A', role: 'selectall' } ] @@ -107,9 +91,7 @@ app.once('ready', () => { } }, { - label: 'Toggle Full Screen', - role: 'togglefullscreen', - accelerator: process.platform === 'darwin' ? 'Ctrl+Command+F' : 'F11' + role: 'togglefullscreen' }, { label: 'Toggle Developer Tools', @@ -121,23 +103,17 @@ app.once('ready', () => { ] }, { - label: 'Window', role: 'window', submenu: [ { - label: 'Minimize', - accelerator: 'CmdOrCtrl+M', role: 'minimize' }, { - label: 'Close', - accelerator: 'CmdOrCtrl+W', role: 'close' } ] }, { - label: 'Help', role: 'help', submenu: [ { @@ -175,14 +151,12 @@ app.once('ready', () => { label: 'Electron', submenu: [ { - label: 'About Electron', role: 'about' }, { type: 'separator' }, { - label: 'Services', role: 'services', submenu: [] }, @@ -190,49 +164,36 @@ app.once('ready', () => { type: 'separator' }, { - label: 'Hide Electron', - accelerator: 'Command+H', role: 'hide' }, { - label: 'Hide Others', - accelerator: 'Command+Alt+H', role: 'hideothers' }, { - label: 'Show All', role: 'unhide' }, { type: 'separator' }, { - label: 'Quit ' + app.getName(), - accelerator: 'Command+Q', role: 'quit' } ] }) template[3].submenu = [ { - label: 'Close', - accelerator: 'CmdOrCtrl+W', role: 'close' }, { - label: 'Minimize', - accelerator: 'CmdOrCtrl+M', role: 'minimize' }, { - label: 'Zoom', role: 'zoom' }, { type: 'separator' }, { - label: 'Bring All to Front', role: 'front' } ] @@ -243,7 +204,6 @@ app.once('ready', () => { label: 'File', submenu: [ { - label: 'Exit', role: 'quit' } ] diff --git a/docs/api/menu-item.md b/docs/api/menu-item.md index 0d05d81bc..3a9a565a9 100644 --- a/docs/api/menu-item.md +++ b/docs/api/menu-item.md @@ -39,6 +39,9 @@ It is best to specify `role` for any menu item that matches a standard role, rather than trying to manually implement the behavior in a `click` function. The built-in `role` behavior will give the best native experience. +The `label` and `accelerator` are optional when using a `role` and will default +to appropriate values for each platform. + The `role` property can have following values: * `undo` diff --git a/docs/api/menu.md b/docs/api/menu.md index 7dea2c6b4..d54469545 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -39,45 +39,30 @@ const template = [ label: 'Edit', submenu: [ { - label: 'Undo', - accelerator: 'CmdOrCtrl+Z', role: 'undo' }, { - label: 'Redo', - accelerator: 'Shift+CmdOrCtrl+Z', role: 'redo' }, { type: 'separator' }, { - label: 'Cut', - accelerator: 'CmdOrCtrl+X', role: 'cut' }, { - label: 'Copy', - accelerator: 'CmdOrCtrl+C', role: 'copy' }, { - label: 'Paste', - accelerator: 'CmdOrCtrl+V', role: 'paste' }, { - label: 'Paste and Match Style', - accelerator: 'Shift+Command+V', role: 'pasteandmatchstyle' }, { - label: 'Delete', role: 'delete' }, { - label: 'Select All', - accelerator: 'CmdOrCtrl+A', role: 'selectall' }, ] @@ -93,12 +78,7 @@ const template = [ } }, { - label: 'Toggle Full Screen', - accelerator: process.platform === 'darwin' ? 'Ctrl+Command+F' : 'F11', - click(item, focusedWindow) { - if (focusedWindow) - focusedWindow.setFullScreen(!focusedWindow.isFullScreen()); - } + role: 'togglefullscreen' }, { label: 'Toggle Developer Tools', @@ -111,23 +91,17 @@ const template = [ ] }, { - label: 'Window', role: 'window', submenu: [ { - label: 'Minimize', - accelerator: 'CmdOrCtrl+M', role: 'minimize' }, { - label: 'Close', - accelerator: 'CmdOrCtrl+W', role: 'close' }, ] }, { - label: 'Help', role: 'help', submenu: [ { @@ -144,14 +118,12 @@ if (process.platform === 'darwin') { label: name, submenu: [ { - label: 'About ' + name, role: 'about' }, { type: 'separator' }, { - label: 'Services', role: 'services', submenu: [] }, @@ -159,26 +131,19 @@ if (process.platform === 'darwin') { type: 'separator' }, { - label: 'Hide ' + name, - accelerator: 'Command+H', role: 'hide' }, { - label: 'Hide Others', - accelerator: 'Command+Alt+H', role: 'hideothers' }, { - label: 'Show All', role: 'unhide' }, { type: 'separator' }, { - label: 'Quit', - accelerator: 'Command+Q', - click() { app.quit(); } + role: 'quit' }, ] }); diff --git a/filenames.gypi b/filenames.gypi index af7080821..6b086e1f8 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -21,6 +21,7 @@ 'lib/browser/api/ipc-main.js', 'lib/browser/api/menu.js', 'lib/browser/api/menu-item.js', + 'lib/browser/api/menu-item-roles.js', 'lib/browser/api/navigation-controller.js', 'lib/browser/api/power-monitor.js', 'lib/browser/api/power-save-blocker.js', diff --git a/lib/browser/api/app.js b/lib/browser/api/app.js index 09835a792..34210f420 100644 --- a/lib/browser/api/app.js +++ b/lib/browser/api/app.js @@ -1,12 +1,15 @@ 'use strict' +const bindings = process.atomBinding('app') +const {app} = bindings + +// Only one app object permitted. +module.exports = app + const electron = require('electron') const {deprecate, Menu} = electron const {EventEmitter} = require('events') -const bindings = process.atomBinding('app') -const {app} = bindings - Object.setPrototypeOf(app, EventEmitter.prototype) let appPath = null @@ -67,6 +70,3 @@ process.atomBinding('download_item')._setWrapDownloadItem((downloadItem) => { // downloadItem is an EventEmitter. Object.setPrototypeOf(downloadItem, EventEmitter.prototype) }) - -// Only one App object pemitted. -module.exports = app diff --git a/lib/browser/api/menu-item-roles.js b/lib/browser/api/menu-item-roles.js new file mode 100644 index 000000000..082099818 --- /dev/null +++ b/lib/browser/api/menu-item-roles.js @@ -0,0 +1,147 @@ +const {app} = require('electron') + +const roles = { + about: { + get label () { + return process.platform === 'linux' ? 'About' : `About ${app.getName()}` + } + }, + close: { + label: 'Close', + accelerator: 'CommandOrControl+W', + windowMethod: 'close' + }, + copy: { + label: 'Copy', + accelerator: 'CommandOrControl+C', + webContentsMethod: 'copy' + }, + cut: { + label: 'Cut', + accelerator: 'CommandOrControl+X', + webContentsMethod: 'cut' + }, + delete: { + label: 'Delete', + webContentsMethod: 'delete' + }, + front: { + label: 'Bring All to Front' + }, + help: { + label: 'Help' + }, + hide: { + get label () { + return `Hide ${app.getName()}` + }, + accelerator: 'Command+H' + }, + hideothers: { + label: 'Hide Others', + accelerator: 'Command+Alt+H' + }, + minimize: { + label: 'Minimize', + accelerator: 'CommandOrControl+M', + windowMethod: 'minimize' + }, + paste: { + label: 'Paste', + accelerator: 'CommandOrControl+V', + webContentsMethod: 'paste' + }, + pasteandmatchstyle: { + label: 'Paste and Match Style', + accelerator: 'Shift+CommandOrControl+V', + webContentsMethod: 'pasteAndMatchStyle' + }, + quit: { + get label () { + switch (process.platform) { + case 'darwin': return `Quit ${app.getName()}` + case 'win32': return 'Exit' + default: return 'Quit' + } + }, + accelerator: process.platform === 'win32' ? null : 'Command+Q', + appMethod: 'quit' + }, + redo: { + label: 'Redo', + accelerator: 'Shift+CommandOrControl+Z', + webContentsMethod: 'redo' + }, + selectall: { + label: 'Select All', + accelerator: 'CommandOrControl+A', + webContentsMethod: 'selectAll' + }, + services: { + label: 'Services' + }, + togglefullscreen: { + label: 'Toggle Full Screen', + accelerator: process.platform === 'darwin' ? 'Control+Command+F' : 'F11', + windowMethod: function (window) { + window.setFullScreen(!window.isFullScreen()) + } + }, + undo: { + label: 'Undo', + accelerator: 'CommandOrControl+Z', + webContentsMethod: 'undo' + }, + unhide: { + label: 'Show All' + }, + window: { + label: 'Window' + }, + zoom: { + label: 'Zoom' + } +} + +exports.getDefaultLabel = (role) => { + if (roles.hasOwnProperty(role)) { + return roles[role].label + } else { + return '' + } +} + +exports.getDefaultAccelerator = (role) => { + if (roles.hasOwnProperty(role)) return roles[role].accelerator +} + +exports.execute = (role, focusedWindow) => { + if (!roles.hasOwnProperty(role)) return false + if (process.platform === 'darwin') return false + + const {appMethod, webContentsMethod, windowMethod} = roles[role] + + if (appMethod) { + app[appMethod]() + return true + } + + if (windowMethod && focusedWindow != null) { + if (typeof windowMethod === 'function') { + windowMethod(focusedWindow) + } else { + focusedWindow[windowMethod]() + } + return true + } + + if (webContentsMethod && focusedWindow != null) { + const {webContents} = focusedWindow + if (webContents) { + webContents[webContentsMethod]() + } + return true + } + + return false +} diff --git a/lib/browser/api/menu-item.js b/lib/browser/api/menu-item.js index 264be8179..8fb1b51d1 100644 --- a/lib/browser/api/menu-item.js +++ b/lib/browser/api/menu-item.js @@ -1,38 +1,11 @@ 'use strict' +const roles = require('./menu-item-roles') + let nextCommandId = 0 -// Maps role to methods of webContents -const rolesMap = { - undo: 'undo', - redo: 'redo', - cut: 'cut', - copy: 'copy', - paste: 'paste', - pasteandmatchstyle: 'pasteAndMatchStyle', - selectall: 'selectAll', - minimize: 'minimize', - close: 'close', - delete: 'delete', - quit: 'quit', - togglefullscreen: 'toggleFullScreen' -} - -// Maps methods that should be called directly on the BrowserWindow instance -const methodInBrowserWindow = { - minimize: true, - close: true, - toggleFullScreen: function (window) { - window.setFullScreen(!window.isFullScreen()) - } -} - -const methodInApp = { - quit: true -} - const MenuItem = function (options) { - const {app, Menu} = require('electron') + const {Menu} = require('electron') this.selector = options.selector this.type = options.type @@ -58,11 +31,11 @@ const MenuItem = function (options) { this.overrideReadOnlyProperty('type', 'normal') this.overrideReadOnlyProperty('role') - this.overrideReadOnlyProperty('accelerator') + this.overrideReadOnlyProperty('accelerator', roles.getDefaultAccelerator(this.role)) this.overrideReadOnlyProperty('icon') this.overrideReadOnlyProperty('submenu') - this.overrideProperty('label', '') + this.overrideProperty('label', roles.getDefaultLabel(this.role)) this.overrideProperty('sublabel', '') this.overrideProperty('enabled', true) this.overrideProperty('visible', true) @@ -81,22 +54,12 @@ const MenuItem = function (options) { this.checked = !this.checked } - if (this.role && rolesMap[this.role] && process.platform !== 'darwin' && focusedWindow != null) { - const methodName = rolesMap[this.role] - if (methodInApp[methodName]) { - return app[methodName]() - } else if (typeof methodInBrowserWindow[methodName] === 'function') { - return methodInBrowserWindow[methodName](focusedWindow) - } else if (methodInBrowserWindow[methodName]) { - return focusedWindow[methodName]() - } else { - const {webContents} = focusedWindow - return webContents != null ? webContents[methodName]() : void 0 + if (!roles.execute(this.role, focusedWindow)) { + if (typeof click === 'function') { + click(this, focusedWindow, event) + } else if (typeof this.selector === 'string' && process.platform === 'darwin') { + Menu.sendActionToFirstResponder(this.selector) } - } else if (typeof click === 'function') { - return click(this, focusedWindow, event) - } else if (typeof this.selector === 'string' && process.platform === 'darwin') { - return Menu.sendActionToFirstResponder(this.selector) } } } diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index d6ac56475..d78b5b4e3 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -395,4 +395,32 @@ describe('menu module', function () { }, /Invalid submenu/) }) }) + + describe('MenuItem role', function () { + it('includes a default label and accelerator', function () { + var item = new MenuItem({role: 'close'}) + assert.equal(item.label, 'Close') + assert.equal(item.accelerator, 'CommandOrControl+W') + + var item = new MenuItem({role: 'close', label: 'Other'}) + assert.equal(item.label, 'Other') + assert.equal(item.accelerator, 'CommandOrControl+W') + + var item = new MenuItem({role: 'close', accelerator: 'D'}) + assert.equal(item.label, 'Close') + assert.equal(item.accelerator, 'D') + + var item = new MenuItem({role: 'close', label: 'C', accelerator: 'D'}) + assert.equal(item.label, 'C') + assert.equal(item.accelerator, 'D') + + var item = new MenuItem({role: 'help'}) + assert.equal(item.label, 'Help') + assert.equal(item.accelerator, undefined) + + var item = new MenuItem({role: 'hide'}) + assert.equal(item.label, 'Hide Electron Test') + assert.equal(item.accelerator, 'Command+H') + }) + }) })