ayrton / react-key-handler

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

Adds new `handledKeys` prop to handle more than one key #91

Closed aarosil closed 6 years ago

aarosil commented 7 years ago

Thanks for the nice library!

I saw in the issues a lot of requests to let this library handle multiple keys, so I took a stab at it.

Assuming you had some component with following methods:

handleKeyEvent(e) {
  switch(e.key) {
    case 'Escape':
      return console.log('hit escape');

    case 'ArrowLeft':
      return console.log('navigated direction');
  }
}

get handledKeys() {
  return [
    { keyValue: 'Escape', allowInputTarget: true },
    { keyCode: 37 }
  ];
}

You could use it like this:

<KeyHandler
  keyEventName={KEYDOWN}
  handledKeys={this.handledKeys}
  onKeyHandle={this.handleKeyEvent} />

Should also work the same using as a HOC.

Let me know if this looks good to you and I can try adding some unit tests for this behavior.

ayrton commented 7 years ago

First of all I wanna say this is awesome, I somehow missed this PR so sorry for the late reply.

I like the direction where this is heading, but I think the ideal API looks something like this:

<KeyHandler
  keyValues={['Escape', 'ArrowLeft']}
  onKeyHandle={this.handleKeyEvent} />

Correct me if I'm wrong but I don't think we need to intro allowInputTarget if you specify the right keyEventName (in this case the default should do)

aarosil commented 7 years ago

I too apologize for late reply. Glad to hear you like this suggestion!

I had mentioned previous issues in the repo about multiple keys, but upon closer reading, I noticed those were about binding to multiple keys at the same time (cmd + whatever for example), so I just wanted to clarify that this PR isn't doing that, but just lets you configure a single KeyHandler to handle multiple keys.

Anyway, that is why key values was array of objects instead of just the keyValue string names, to keep the same config options as someone had before. But I can update so it is just an array of the keyValue string names also if you preferred that. Also, I think you're right that allowInputTarget not needed.

ayrton commented 7 years ago

@aarosil in that case what do you think if keyValue can either be a string or an array of strings to avoid confusion. That way we don't introduce a new prop, and we can still use keyValues for combinations of keys

gforge commented 6 years ago

I agree that keyCode/keyValue could be either string or array. Having additional options could cause confusion when using both.

gforge commented 6 years ago

I took the liberty of writing my own array handler + adding the code key to the package: https://github.com/gforge/react-key-handler/tree/add_code_and_array

I also took the liberty of refactoring a little - the index.js is more informative and the decorator has moved into a separate file.

There is also 1 major change and that is that I removed the priority of inputs. It seems to me that either you throw an error when mounting the component with both keyValue and keyCode specified or you look and see if any of them match.

The README and demo's are also updated.

Let me know if you want this as a PR, I'll probably release a scoped package version otherwise for my own convenience :grin:. Thanks again for this awesome package!

ayrton commented 6 years ago

@gforge feel free to open a PR! I can have a look this week

ayrton commented 6 years ago

Closing in favour of #148 (which is closed to being merged in)