Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Menu on Windows now properly responds to ALT.
Browse files Browse the repository at this point in the history
Menu is now toggled only under these conditions:
- the left ALT key is pressed (ignore AltGr)
- last key pressed was ALT (typing ALT codes should not toggle menu)
- no other key is being pushed simultaneously
- since initial keydown, ALT has been the only key pressed

NOTE: key state is reset when window blurs (in case you used a shortcut that opened a new window, which prevents the keyup handler from firing on release)

Auditors: @jonathansampson, @bbondy

Fixes #5775
  • Loading branch information
bsclifton committed Mar 4, 2017
1 parent 644e46d commit c789de1
Showing 1 changed file with 36 additions and 16 deletions.
52 changes: 36 additions & 16 deletions js/components/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class Main extends ImmutableComponent {
this.onFind = this.onFind.bind(this)
this.onFindHide = this.onFindHide.bind(this)
this.checkForTitleMode = debounce(this.checkForTitleMode.bind(this), 20)
this.lastKeyPressed = undefined
this.resetAltMenuProcessing()
}
registerWindowLevelShortcuts () {
// For window level shortcuts that don't work as local shortcuts
Expand All @@ -131,36 +131,47 @@ class Main extends ImmutableComponent {
}
break
}

this.keydown[e.which] = true
this.lastKeyPressed = e.which
})
}

resetAltMenuProcessing () {
this.lastKeyPressed = undefined
this.keydown = {}
this.keydownHistory = []
}

registerCustomTitlebarHandlers () {
if (this.customTitlebar.enabled) {
document.addEventListener('keyup', (e) => {
const customTitlebar = this.customTitlebar
switch (e.which) {
case keyCodes.ALT:
// Ignore right alt (AltGr)
if (e.location === keyLocations.DOM_KEY_LOCATION_RIGHT) {
/*
Only show/hide the menu if:
- the left ALT key is pressed (ignore AltGr)
- last key pressed was ALT (typing ALT codes should not toggle menu)
- no other key is being pushed simultaneously
- since initial keydown, ALT has been the only key pressed
*/
if (e.location === keyLocations.DOM_KEY_LOCATION_RIGHT ||
this.lastKeyPressed !== keyCodes.ALT ||
Object.keys(this.keydown).length > 1 ||
this.keydownHistory.length > 0) {
break
}

// Only show/hide the menu if last key pressed was ALT
// (typing ALT codes should not toggle menu)
if (this.lastKeyPressed === keyCodes.ALT) {
e.preventDefault()
e.preventDefault()

if (getSetting(settings.AUTO_HIDE_MENU)) {
windowActions.toggleMenubarVisible(null)
if (getSetting(settings.AUTO_HIDE_MENU)) {
windowActions.toggleMenubarVisible(null)
} else {
if (customTitlebar.menubarSelectedIndex) {
windowActions.setMenuBarSelectedIndex()
windowActions.setContextMenuDetail()
} else {
if (customTitlebar.menubarSelectedIndex) {
windowActions.setMenuBarSelectedIndex()
windowActions.setContextMenuDetail()
} else {
windowActions.setMenuBarSelectedIndex(0)
}
windowActions.setMenuBarSelectedIndex(0)
}
}
break
Expand All @@ -177,6 +188,14 @@ class Main extends ImmutableComponent {
}
break
}

// For ALT menu processing
if (Object.keys(this.keydown).length > 1) {
this.keydownHistory.push(e.which)
} else {
this.keydownHistory = []
}
delete this.keydown[e.which]
})

document.addEventListener('focus', (e) => {
Expand Down Expand Up @@ -528,6 +547,7 @@ class Main extends ImmutableComponent {
windowActions.onFocusChanged(true)
})
currentWindow.on('blur', function () {
self.resetAltMenuProcessing()
appActions.windowBlurred(currentWindow.id)
windowActions.onFocusChanged(false)
})
Expand Down

0 comments on commit c789de1

Please sign in to comment.