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

KeyboardEvent.keyCode is deprecated #7

Closed
leocavalcante opened this Issue Mar 3, 2016 · 8 comments

Comments

2 participants
@leocavalcante
Contributor

leocavalcante commented Mar 3, 2016

keyCode isn't a web standard anymore.
Maybe, should it be avoided?
https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode

@ayrton

This comment has been minimized.

Owner

ayrton commented Mar 4, 2016

Thanks for bringing this up, when working on the initial implementation I saw this in the docs and was quite surprised by this. Afaik key codes are currently still supported in every single browser, I will however investigate this more into detail.

FYI in addition to a keyCode you can also pass on keyName (internally this is being translated to a key code). A full list of possible values is available in the tests of the keycodes project.

@ayrton

This comment has been minimized.

Owner

ayrton commented Mar 4, 2016

KeyboardEvent.key is the successor of KeyboardEvent.keyCode, however browser currently do not widely support it yet.

For us ideally I think this means, we accept all possible KeyboardEvent.key values and translate them to keyCode under the hood, once it is fully supported we just make the switch internally.

@leocavalcante

This comment has been minimized.

Contributor

leocavalcante commented Mar 4, 2016

My research leads me to KeyboardEvent.keyIdentifier when KeyboardEvent.key isn't present. It is deprecated too, but is how non-key browsers are working right now.

As key is the standard, I would go for it as default implementation, then add internally hacks for browser support, like using keyIdentifier when key isn't present and translating keyCode to key when netheir key or keyIdentifier are present on the event.

@leocavalcante

This comment has been minimized.

Contributor

leocavalcante commented Mar 4, 2016

Just realized that mapping keyCode to key is a project a part, but this:

const key = event.key || event.keyIdentifier || keyVal(event.keyCode);

is present-future proof.

@ayrton

This comment has been minimized.

Owner

ayrton commented Mar 4, 2016

Love that you investigated this, is this something you'd like to contribute in code? If so I'm happy to assign you to this feature, if not I can probably put this theory to practice.

One final note, we can't use the prop key because it's a reserved prop name.

@leocavalcante

This comment has been minimized.

Contributor

leocavalcante commented Mar 4, 2016

Sure, I can send a PR if you like to. Some extra fun for the weekend 😆
Can we use keyId as short for keyIdentifier referencing KeyboardEvent.key?

const keyId = this.props.keyId || keyIds(this.props.keyCode) || keyIds(keyNames(this.props.keyName));
const eventKeyId = event.key || event.keyIdentifier || keyIds(event.keyCode);
if (keyId !== eventKeyId) return;
@ayrton

This comment has been minimized.

Owner

ayrton commented Mar 4, 2016

Awesome, feel free to experiment with the code as how you see best. Will give a detailed opinion on this inside the PR. In the meantime will thinker about this too 🙌

@ayrton

This comment has been minimized.

Owner

ayrton commented Mar 13, 2016

PR #10 is merged in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment