Merge pull request #19423 from atom/as/multi-stroke-accelerators

Display multi-keystroke key bindings in menu item's label

spec/context-menu-manager-spec.coffee

@@ -334,6 +334,30 @@ describe "ContextMenuManager", ->
           ]
         ])
 
+    it "does not add accelerators for multi-keystroke key bindings", ->
+      atom.keymaps.add('source', {
+        '.child': {
+          'ctrl-a ctrl-b': 'test:multi-keystroke-command'
+        }
+      })
+      contextMenu.clear()
+      contextMenu.add('.parent': [{
+        label: 'Multi-keystroke command',
+        command: 'test:multi-keystroke-command',
+      }])
+
+      child.focus()
+
+      label =
+        if process.platform is 'darwin'
+          '⌃A ⌃B'
+        else
+          'Ctrl+A Ctrl+B'
+      expect(contextMenu.templateForEvent({target: child})).toEqual([{
+        label: "Multi-keystroke command [#{label}]",
+        command: 'test:multi-keystroke-command',
+      }])
+
   describe "::templateForEvent(target) (sorting)", ->
     it "applies simple sorting rules", ->
       contextMenu.add('.parent': [{

src/context-menu-manager.coffee

@@ -6,6 +6,7 @@ fs = require 'fs-plus'
 {remote} = require 'electron'
 MenuHelpers = require './menu-helpers'
 {sortMenuItems} = require './menu-sort-helpers'
+_ = require 'underscore-plus'
 
 platformContextMenu = require('../package.json')?._atomMenu?['context-menu']
 
@@ -158,8 +159,15 @@ class ContextMenuManager
     for id, item of template
       if item.command
         keymaps = @keymapManager.findKeyBindings({command: item.command, target: document.activeElement})
-        accelerator = MenuHelpers.acceleratorForKeystroke(keymaps?[0]?.keystrokes)
-        item.accelerator = accelerator if accelerator
+        keystrokes = keymaps?[0]?.keystrokes
+        if keystrokes
+          # Electron does not support multi-keystroke accelerators. Therefore,
+          # when the command maps to a multi-stroke key binding, show the
+          # keystrokes next to the item's label.
+          if keystrokes.includes(' ')
+            item.label += " [#{_.humanizeKeystroke(keystrokes)}]"
+          else
+            item.accelerator = MenuHelpers.acceleratorForKeystroke(keystrokes)
       if Array.isArray(item.submenu)
         @addAccelerators(item.submenu)
 

src/main-process/application-menu.js

@@ -222,10 +222,18 @@ module.exports = class ApplicationMenu {
     template.forEach(item => {
       if (item.metadata == null) item.metadata = {};
       if (item.command) {
-        item.accelerator = this.acceleratorForCommand(
-          item.command,
-          keystrokesByCommand
-        );
+        const keystrokes = keystrokesByCommand[item.command];
+        if (keystrokes && keystrokes.length > 0) {
+          const keystroke = keystrokes[0];
+          // Electron does not support multi-keystroke accelerators. Therefore,
+          // when the command maps to a multi-stroke key binding, show the
+          // keystrokes next to the item's label.
+          if (keystroke.includes(' ')) {
+            item.label += ` [${_.humanizeKeystroke(keystroke)}]`;
+          } else {
+            item.accelerator = MenuHelpers.acceleratorForKeystroke(keystroke);
+          }
+        }
         item.click = () =>
           global.atomApplication.sendCommand(item.command, item.commandDetail);
         if (!/^application:/.test(item.command)) {
@@ -237,18 +245,4 @@ module.exports = class ApplicationMenu {
     });
     return template;
   }
-
-  // Determine the accelerator for a given command.
-  //
-  // command - The name of the command.
-  // keystrokesByCommand - An Object where the keys are commands and the values
-  //                       are Arrays containing the keystroke.
-  //
-  // Returns a String containing the keystroke in a format that can be interpreted
-  //   by Electron to provide nice icons where available.
-  acceleratorForCommand(command, keystrokesByCommand) {
-    const firstKeystroke =
-      keystrokesByCommand[command] && keystrokesByCommand[command][0];
-    return MenuHelpers.acceleratorForKeystroke(firstKeystroke);
-  }
 };

src/menu-manager.coffee

@@ -161,7 +161,6 @@ class MenuManager
       for binding in @keymapManager.getKeyBindings()
         continue unless @includeSelector(binding.selector)
         continue if unsetKeystrokes.has(binding.keystrokes)
-        continue if binding.keystrokes.includes(' ')
         continue if process.platform is 'darwin' and /^alt-(shift-)?.$/.test(binding.keystrokes)
         continue if process.platform is 'win32' and /^ctrl-alt-(shift-)?.$/.test(binding.keystrokes)
         keystrokesByCommand[binding.command] ?= []