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

Support for multiple key codes #4

Open
ayrton opened this issue Mar 1, 2016 · 12 comments
Open

Support for multiple key codes #4

ayrton opened this issue Mar 1, 2016 · 12 comments

Comments

@ayrton
Copy link
Owner

ayrton commented Mar 1, 2016

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
Copy link

princed commented Mar 14, 2016

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
Copy link
Owner Author

ayrton commented Mar 15, 2016

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

@princed
Copy link

princed commented Mar 15, 2016

Good news, thank you!

@ayrton
Copy link
Owner Author

ayrton commented Mar 20, 2016

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

@villesau
Copy link

villesau commented Jul 19, 2016

@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
Copy link
Owner Author

ayrton commented Jul 19, 2016

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

@villesau
Copy link

villesau commented Jul 19, 2016

@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
Copy link
Owner Author

ayrton commented Jul 20, 2016

@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
Copy link

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

@ayrton
Copy link
Owner Author

ayrton commented Mar 7, 2017

@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
Copy link

@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
Copy link

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

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

No branches or pull requests

5 participants