ayrton / react-key-handler

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

Support for multiple key codes #4

Open ayrton opened 8 years ago

ayrton commented 8 years ago

We need to add support to listen to more than one key code, e.g.:

const A = 65;
const B = 66;

keyHandler({ keyCodes: [A, B] })(MyComponent);

could probably decorate a property keyCodes to MyComponent:

{
  alt: true,
  ctrl: false,
  meta: false,
  shift: false,
  65: true,
  66: false,
}

Alternatively, the following projects looks promising:

If you're interesting in working on this, let me know :)

princed commented 8 years ago

It could be nice to use https://github.com/facebook/react/blob/master/src/renderers/dom/client/utils/getEventKey.js for consistency with React.

ayrton commented 8 years ago

@princed completely agree, this needs to be 1:1, @leocavalcante started amazing work on this #10, see also the original issue #7.

princed commented 8 years ago

Good news, thank you!

ayrton commented 8 years ago

@princed started working on this in #17 let's continue the conversation there :)

villesau commented 8 years ago

@ayrton is the check for keys even necessary to have in the library it self? Couldn't the library user handle the checks and library could just pass all the events to the user?

I think by removing https://github.com/ayrton/react-key-handler/blob/master/lib/components/key-handler.js#L87 the responsibility of matching the key events could easily be moved to user. This could also be optional: If none of keyValue, keyCode and keyName is defined, it could hand the responsibility to handle all of the events to user. What do you think?

e: ok it seems to be a bit more complicated than that, at least when focusing on inputs but idea remains the same.

ayrton commented 8 years ago

@villesau I'm open to improvements, what is it you want to achieve with this?

villesau commented 8 years ago

@ayrton to be able to get all the keyboard events to my component and do what ever i want to do with them :) In my case i would use only keyup/down events (but i do not have usecase where i would listen to only one event/key) but i think it's just fine to let user handle the selection on which events to act on. The approach what i suggested was just an idea when i went quickly trough the code and seemed to be simple. I tried it quickly on built code but for some reason some times it still "freezed" to only give some specific letter so removing just the condition most likely does not work by it self, it seems to need some other changes too.

For example this library has nice and flexible api where you can decide if you want to listen to just specific events or catch em all: https://github.com/glortho/react-keydown . The only downside with that is how it does not play well with inputs, that's why i kept digging and came across your library.

ayrton commented 8 years ago

@villesau I think I like that - ideally the library would be able to let the user handle the event handling but also have a baked in version of handling multiple key events. PR's are very much welcome for both.

meandavejustice commented 7 years ago

What is the current status of this issue? Is there a suggested way to handle multiple keys now that #17 has been merged?

ayrton commented 7 years ago

@meandavejustice #17 makes sure you can expect the same event behaviour in this library as in react. It doesn't add support for multiple key codes. #91 is adding support to trigger the callback for multiple single keys, support for multiple key combinations is currently not ongoing. (PRs would be greatly appreciated)

meandavejustice commented 7 years ago

@ayrton Thanks for the clarification, I ended up going with a different (less integrated) solution as I have a deadline for this project. If I end up getting time to integrate this library down the road I would be more than happy to submit a pr. Thank you for the quick reply :)

sjorsvanheuveln commented 7 years ago

What's the status now on handling multiple keys? Would be awesome if this would be incorporated in this repo!