From 159f881e1f27f92a5f71670c093ecb731924836c Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 14 Sep 2016 10:13:44 -0600 Subject: [PATCH 1/6] :arrow_up: atom-keymap (prerelease) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 251a1bb02..0448a60db 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "electronVersion": "1.3.5", "dependencies": { "async": "0.2.6", - "atom-keymap": "6.3.2", + "atom-keymap": "6.3.3-beta1", "atom-ui": "0.4.1", "babel-core": "5.8.38", "cached-run-in-this-context": "0.4.1", From a6094d2ed0e28aa4c0d4c81d563d1c7a2fcf6f76 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 15 Sep 2016 14:08:35 -0600 Subject: [PATCH 2/6] Don't allow menu shortcuts that could conflict with AltGraph characters Signed-off-by: Max Brunsfeld --- spec/menu-manager-spec.coffee | 44 +++++++++++++++++++++++++++++++++++ src/menu-manager.coffee | 15 ++++++------ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/spec/menu-manager-spec.coffee b/spec/menu-manager-spec.coffee index a2d76c1d0..5de5ecf92 100644 --- a/spec/menu-manager-spec.coffee +++ b/spec/menu-manager-spec.coffee @@ -53,6 +53,9 @@ describe "MenuManager", -> expect(menu.template[originalItemCount]).toEqual {label: "A", submenu: [{label: "B", command: "b"}]} describe "::update()", -> + originalPlatform = process.platform + afterEach -> Object.defineProperty process, 'platform', value: originalPlatform + it "sends the current menu template and associated key bindings to the browser process", -> spyOn(menu, 'sendToBrowserProcess') menu.add [{label: "A", submenu: [{label: "B", command: "b"}]}] @@ -75,6 +78,47 @@ describe "MenuManager", -> runs -> expect(menu.sendToBrowserProcess.argsForCall[0][1]['b']).toBeUndefined() + it "omits key bindings that could conflict with AltGraph characters on macOS", -> + spyOn(menu, 'sendToBrowserProcess') + menu.add [{label: "A", submenu: [ + {label: "B", command: "b"}, + {label: "C", command: "c"} + {label: "D", command: "d"} + ]}] + + atom.keymaps.add 'test', 'atom-workspace': + 'alt-b': 'b' + 'alt-shift-C': 'c' + 'alt-cmd-d': 'd' + + waits 50 + + runs -> + expect(menu.sendToBrowserProcess.argsForCall[0][1]['b']).toBeUndefined() + expect(menu.sendToBrowserProcess.argsForCall[0][1]['c']).toBeUndefined() + expect(menu.sendToBrowserProcess.argsForCall[0][1]['d']).toEqual(['alt-cmd-d']) + + it "omits key bindings that could conflict with AltGraph characters on Windows", -> + Object.defineProperty process, 'platform', value: 'win32' + spyOn(menu, 'sendToBrowserProcess') + menu.add [{label: "A", submenu: [ + {label: "B", command: "b"}, + {label: "C", command: "c"} + {label: "D", command: "d"} + ]}] + + atom.keymaps.add 'test', 'atom-workspace': + 'ctrl-alt-b': 'b' + 'ctrl-alt-shift-C': 'c' + 'ctrl-alt-cmd-d': 'd' + + waits 50 + + runs -> + expect(menu.sendToBrowserProcess.argsForCall[0][1]['b']).toBeUndefined() + expect(menu.sendToBrowserProcess.argsForCall[0][1]['c']).toBeUndefined() + expect(menu.sendToBrowserProcess.argsForCall[0][1]['d']).toEqual(['ctrl-alt-cmd-d']) + it "updates the application menu when a keymap is reloaded", -> spyOn(menu, 'update') keymapPath = path.join(__dirname, 'fixtures', 'packages', 'package-with-keymaps', 'keymaps', 'keymap-1.cson') diff --git a/src/menu-manager.coffee b/src/menu-manager.coffee index ebebfa3f3..cc0a5386b 100644 --- a/src/menu-manager.coffee +++ b/src/menu-manager.coffee @@ -144,16 +144,15 @@ class MenuManager update: -> clearImmediate(@pendingUpdateOperation) if @pendingUpdateOperation? @pendingUpdateOperation = setImmediate => - includedBindings = [] - unsetKeystrokes = new Set - - for binding in @keymapManager.getKeyBindings() when @includeSelector(binding.selector) - includedBindings.push(binding) - if binding.command is 'unset!' - unsetKeystrokes.add(binding.keystrokes) + includedBindings = @keymapManager.getKeyBindings().filter (binding) => + return false unless @includeSelector(binding.selector) + return false if binding.command is 'unset!' + return false if process.platform is 'darwin' and /^alt-(shift-)?.$/.test(binding.keystrokes) + return false if process.platform is 'win32' and /^ctrl-alt-(shift-)?.$/.test(binding.keystrokes) + true keystrokesByCommand = {} - for binding in includedBindings when not unsetKeystrokes.has(binding.keystrokes) + for binding in includedBindings keystrokesByCommand[binding.command] ?= [] keystrokesByCommand[binding.command].unshift binding.keystrokes From fe7a9ed419d2d4b7b79887e53df686d88270bba8 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 15 Sep 2016 14:15:41 -0600 Subject: [PATCH 3/6] Fix unset keystroke handling, :art: --- src/menu-manager.coffee | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/src/menu-manager.coffee b/src/menu-manager.coffee index cc0a5386b..b6ed7fd1a 100644 --- a/src/menu-manager.coffee +++ b/src/menu-manager.coffee @@ -143,16 +143,20 @@ class MenuManager # Public: Refreshes the currently visible menu. update: -> clearImmediate(@pendingUpdateOperation) if @pendingUpdateOperation? + @pendingUpdateOperation = setImmediate => - includedBindings = @keymapManager.getKeyBindings().filter (binding) => - return false unless @includeSelector(binding.selector) - return false if binding.command is 'unset!' - return false if process.platform is 'darwin' and /^alt-(shift-)?.$/.test(binding.keystrokes) - return false if process.platform is 'win32' and /^ctrl-alt-(shift-)?.$/.test(binding.keystrokes) - true + unsetKeystrokes = new Set + for binding in @keymapManager.getKeyBindings() + if binding.command is 'unset!' + unsetKeystrokes.add(binding.keystrokes) keystrokesByCommand = {} - for binding in includedBindings + 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] ?= [] keystrokesByCommand[binding.command].unshift binding.keystrokes @@ -175,21 +179,7 @@ class MenuManager unmerge: (menu, item) -> MenuHelpers.unmerge(menu, item) - # macOS can't handle displaying accelerators for multiple keystrokes. - # If they are sent across, it will stop processing accelerators for the rest - # of the menu items. - filterMultipleKeystroke: (keystrokesByCommand) -> - filtered = {} - for key, bindings of keystrokesByCommand - for binding in bindings - continue if binding.indexOf(' ') isnt -1 - - filtered[key] ?= [] - filtered[key].push(binding) - filtered - sendToBrowserProcess: (template, keystrokesByCommand) -> - keystrokesByCommand = @filterMultipleKeystroke(keystrokesByCommand) ipcRenderer.send 'update-application-menu', template, keystrokesByCommand # Get an {Array} of {String} classes for the given element. From 88f47990d08ce79ba2e8b9fe1c19b64f4b635a91 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 15 Sep 2016 14:23:03 -0600 Subject: [PATCH 4/6] :arrow_up: atom-keymap --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 0448a60db..bf0e3f306 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "electronVersion": "1.3.5", "dependencies": { "async": "0.2.6", - "atom-keymap": "6.3.3-beta1", + "atom-keymap": "6.3.3", "atom-ui": "0.4.1", "babel-core": "5.8.38", "cached-run-in-this-context": "0.4.1", From 290c4ecefd1520880e3739dad4414390fc634a02 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Fri, 16 Sep 2016 12:19:14 -0600 Subject: [PATCH 5/6] :arrow_up: atom-keymap --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index bf0e3f306..182a7e2a3 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "electronVersion": "1.3.5", "dependencies": { "async": "0.2.6", - "atom-keymap": "6.3.3", + "atom-keymap": "6.3.4", "atom-ui": "0.4.1", "babel-core": "5.8.38", "cached-run-in-this-context": "0.4.1", From dbb8dec74838f74606a8fbaac9a793e4a5bf1e0c Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Fri, 16 Sep 2016 16:49:18 -0600 Subject: [PATCH 6/6] :arrow_up: atom-keymap --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 182a7e2a3..94b2faafc 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "electronVersion": "1.3.5", "dependencies": { "async": "0.2.6", - "atom-keymap": "6.3.4", + "atom-keymap": "6.3.5", "atom-ui": "0.4.1", "babel-core": "5.8.38", "cached-run-in-this-context": "0.4.1",