ayrton / react-key-handler

React component to handle keyboard events :key:
http://ayrton.be/react-key-handler
388 stars 29 forks source link

KeyboardEvent.keyCode is deprecated #7

Closed leocavalcante closed 8 years ago

leocavalcante commented 8 years ago

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

ayrton commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

Sure, I can send a PR if you like to. Some extra fun for the weekend :laughing: 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 commented 8 years ago

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 commented 8 years ago

PR #10 is merged in