Setting up checkbox#3
Conversation
| super(props); | ||
| if (props.checked === 'true') { | ||
| this.state = { | ||
| pressed: props.checked, |
There was a problem hiding this comment.
Consider only using checked in all places throughout this component -- we may not need the string value for state.pressed, and it would be good for this data to live in one place.
| const props = { ...this.props }; | ||
|
|
||
| return ( | ||
| <form> |
There was a problem hiding this comment.
I would move this <form> containing element into the story for this component to avoid unnecessary DOM elements within the component itself.
| aria-describedby={props.describedby} | ||
| aria-checked={this.state.pressed} | ||
| tabIndex="0" | ||
| onClick={() => this.handleClick()} |
There was a problem hiding this comment.
This could be rewritten as just this.handleClick -- but you'll have to bind the context in the constructor, something like
this.handleDocumentClick = this.handleClick.bind(this);
| aria-checked={this.state.pressed} | ||
| tabIndex="0" | ||
| onClick={() => this.handleClick()} | ||
| disabled={props.disable} |
There was a problem hiding this comment.
I would use disabled instead since that prop already exists in inputProps.
|
|
||
| CheckBox.propTypes = { | ||
| ...inputProps, | ||
| checkLabel: PropTypes.string.isRequired, |
There was a problem hiding this comment.
Similarly, you can just use label which exists within inputProps -- you don't need to add an additional custom property for the checkbox.
| }); | ||
| } | ||
| if (this.props.fun) { | ||
| this.props.fun(); |
There was a problem hiding this comment.
I think you can just use this.props.onChange for this -- also, make sure to add a story to show this callback in action :)
|
Awesome work @sarahkf. Ship it!! |
2 participants
No description provided.