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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants