Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix nonprintable keys #199

Merged
merged 3 commits into from
Jun 17, 2019
Merged

Bugfix nonprintable keys #199

merged 3 commits into from
Jun 17, 2019

Conversation

greena13
Copy link
Owner

Context

react-hotkeys uses a whitelist of accepted keynames in order to fail loudly if a user of the library enters an invalid key name. This list is currently incomplete, resulting in #198.

This pull request

Takes the lead from the way that react itself handles keys, and consolidates all of the follow features into a single list:

  • Normalizing differences between browsers (down by React itself when events bubble up through React, or done by react-hotkeys for global hotkeys that occur outside the React application)
  • Establishing whether a key has a native keypress event, or whether it should be simulated (all non-printable keys do not have a keypress event)
  • Determining whether a (normalized) key name is a valid one, or whether the user of react-hotkeys may have made a mistake in defining their key sequence

@@ -1,27 +0,0 @@
/**
Copy link
Owner Author

Choose a reason for hiding this comment

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

🐥 Remove separate file previously used to record keys without keypress events - this is now just the list of non-printable keys.

@@ -0,0 +1,10 @@
/**
Copy link
Owner Author

Choose a reason for hiding this comment

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

🐥 New dictionary of non-printable keys, that takes its values from the same list that the React normalization algorithm uses.

@@ -1,13 +0,0 @@
import SpecialKeysDictionary from '../../const/SpecialKeysDictionary';
Copy link
Owner Author

Choose a reason for hiding this comment

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

🐥 Move the list of "special" keys, to be referred to as "non-printable"

return isSpecialKey(keyName) || String.fromCharCode(keyName.charCodeAt(0)) === keyName;
return isNonPrintableKeyName(keyName) ||
String.fromCharCode(keyName.charCodeAt(0)) === keyName ||
Configuration.option('customKeyCodes')[keyName];
Copy link
Owner Author

@greena13 greena13 Jun 17, 2019

Choose a reason for hiding this comment

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

✅🐛Fix this line - incorrect way of accepting custom keys (it's a map of key codes to key names and this lookup will always fail).

@@ -28,50 +29,6 @@ const normalizeKey = {
MozPrintableKey: 'Unidentified',
};

/**
Copy link
Owner Author

Choose a reason for hiding this comment

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

🐥☂️Abstract React's list of non-printable keys into a separate file, so it can be re-used by react-hotkeys.

@greena13 greena13 merged commit c363ba7 into master Jun 17, 2019
@greena13 greena13 deleted the bugfix_nonprintable_keys branch June 17, 2019 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant