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

Add W3C KeyboardEvent.key values support. #10

Closed
wants to merge 9 commits into from
Closed

Add W3C KeyboardEvent.key values support. #10

wants to merge 9 commits into from

Conversation

leocavalcante
Copy link
Contributor

As discussed at #7

KeyboardEvent.keyCode is deprecated, also MDN recommends to avoid it for new projects in favor of KeyboardEvent.key.

This PR is a result of following this recommendations without dropping the support for keyCode since not every browser has implemented the new standard yet.

The usage of keyValue instead of keyId as discussed at #7 is to match an ubiquitous terminology with current standards.

Now, despite letter case, the arrow keys values are the only difference I notice from keycodes naming to actual key values so keyNameVals might seams dummy, but it allows to map any another differences.

Hope it helps!
Cheers

@leocavalcante
Copy link
Contributor Author

Sorry about the flow mess. Just setup a Linux VM to properly test the project.

@ayrton
Copy link
Owner

ayrton commented Mar 6, 2016

Awesome work Leo, this is epic stuff 🙌

/**
* Helpers
*/
function valFromCode(keyCode: ?number): ?string {
Copy link
Owner

Choose a reason for hiding this comment

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

I would name this keyValueFromCode and move it to lib/utils.js

@leocavalcante
Copy link
Contributor Author

@ayrton, thanks and I do agree with all of your appointments, so here are them.
Glad to help! Learned a lot playing with this, I appreciate, loved this Flow thing, static analyses FTW!

@ayrton
Copy link
Owner

ayrton commented Mar 7, 2016

Glad you enjoyed it and you were able to learn something from this.

@ayrton
Copy link
Owner

ayrton commented Mar 13, 2016

Merged this in, will try to release a new version in the next couple days, thank you for your contribution!

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

2 participants