Merge pull request #6190 from electron/default-label-and-accelerator

Add default label and accelerator for menu item roles

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

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
+}

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)
     }
   }
 }