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

Update Windows titlebar code to ignore AltGr #4628

Merged
merged 1 commit into from
Oct 10, 2016
Merged

Update Windows titlebar code to ignore AltGr #4628

merged 1 commit into from
Oct 10, 2016

Conversation

bsclifton
Copy link
Member

@bsclifton bsclifton commented Oct 8, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Update Windows titlebar code to ignore AltGr

  • Implemented by adding new keyLocations file and checking event location.
  • New file and existing keyCodes file were moved to new directory structure

Fixes #4626

Auditors: @bbondy

Test plan

  1. Use Brave on Windows and visit http://fsymbols.com/keyboard/windows/alt-codes/list/
  2. Ensure hide menu by default is enabled
  3. Send focus to the demo text field, hold AltGr and then hit 1 to type a smiley face ☺
  4. Notice menu is NOT toggled

@@ -0,0 +1,14 @@


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove newlines from start of file

* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

const KeyLocationCodes = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: keyLocationCodes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit2: Name it the same you call variables you require to for better grep-ability, so keyLocations

@@ -131,6 +132,11 @@ class Main extends ImmutableComponent {
const customTitlebar = this.customTitlebar
switch (e.which) {
case keyCodes.ALT:
// Ignore right alt (AltGr)
if (e.location === keyLocations.DOM_KEY_LOCATION_RIGHT) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neat I didn't even know this existed.

@bbondy
Copy link
Member

bbondy commented Oct 10, 2016

A few nits, but after that I don't need to see this again so just merge directly to master.

- Implemented by adding new keyLocations file and checking event location.
- New file and existing keyCodes file were moved to new directory structure

Fixes #4626

Auditors: @bbondy
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants