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

Have the higer-order component accept an onKeyHandle parameter #54

Closed
petejodo opened this issue Jul 22, 2016 · 3 comments
Closed

Have the higer-order component accept an onKeyHandle parameter #54

petejodo opened this issue Jul 22, 2016 · 3 comments

Comments

@petejodo
Copy link

petejodo commented Jul 22, 2016

As of right now a component that gets wrapped by keyHandler has to expect keyValue, etc being passed down. That causes the component to be coupled with keyHandler if all you want to do is fire a callback that is on the props object. A way around this would be to allow an onKeyHandle parameter be passed to keyHandler that could look like (ownProps, keyValue, keyEventName) => void or something akin to that.

Just want to open the discussion of any pros/cons of this approach

Here's an example:

import React, { Component } from 'react';
import { keyHandler, KEYPRESS } from 'react-key-handler';

const CloseButton = ({ closeHandler }) => <button onClick={closeHandler}>Close</button>;

const CloseOnEnterKeyOrButton = keyHandler(
  { keyEventName: KEYPRESS, keyValue: 'enter' },
  ownProps => {
    if (ownProps.closeHandler) ownProps.closeHandler();
  }
)(CloseButton);

class Parent extends Component {
  constructor(props) {
    super(props);
    this.state = { isOpened: true };
  }
  render() {
    if (this.state.isOpened) {
      return (
        <div>
          Some stuff
          <CloseOnEnterKeyOrButton closeHandler={this.setState.bind(this, { isOpened: false })} />
        </div>
      );
    }
    return null;
  }
}

This 2nd parameter to keyHandler would only be fired when enter would be pressed

@ayrton
Copy link
Owner

ayrton commented Jul 26, 2016

This feels like this can be the same possible solution that @villesau would like (described in #4 (comment)). As mentioned in that issue I'm open to improvements and more than happy to accept contributions

@ayrton
Copy link
Owner

ayrton commented Sep 13, 2016

Looks like I'm in need of this myself, I'll be working on a solution for this sometime soon 😊

@petejodo
Copy link
Author

Awesome. I've been wanting to help out and work on this but I've been super busy (+ a little vacation). I've actually had this tab open since I opened this issue to remind myself about it haha

@petejodo petejodo closed this as not planned Won't fix, can't repro, duplicate, stale Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants