feat: disable input #188
feat: disable input #188
Conversation
ghost
changed the title
There was a problem hiding this comment.
Thank you for submitting this PR. It is very well thought out, and updating the docs examples and updating bundle.js is a nice touch.
The package.json changes are fine. I recently spent a good deal of time upgrading to babel-7 and latest of ava and what not but I had multiple issues that were insurmountable. So until babel-7 gets mature, I think it's a good idea to pin these.
I have some suggestions. Please review and let me know your thoughts.
src/index.js
Outdated
| @@ -175,7 +179,7 @@ class DropdownTreeSelect extends Component { | |||
| render() { | |||
| const dropdownTriggerClassname = cx({ | |||
| 'dropdown-trigger': true, | |||
| arrow: true, | |||
| arrow: !this.props.disabled, | |||
There was a problem hiding this comment.
I think we should leave the arrow as is but add another class disabled for disabled case. This accomplishes few things:
- keeps the current behavior as is
- the arrow should not disappear, rather appear as disabled for a better UX
- allows users to extend or style differently when the control is disabled (since it's not a native control, they cannot use CSS attribute selector)
Also, I think we need some disabled styling for arrow like greying it out and setting cursor to not-allowed.
src/input/index.js
Outdated
| @@ -47,6 +48,7 @@ class Input extends PureComponent { | |||
| <li className={cx('tag-item')}> | |||
| <input | |||
| type="text" | |||
| disabled={this.props.disabled} | |||
There was a problem hiding this comment.
Can you move it to the props destructuring statement at the beginning - to keep things consistent?
const { ..., disabled, ...} = this.props
...
disabled={disabled}
...
src/index.js
Outdated
| @@ -84,6 +85,9 @@ class DropdownTreeSelect extends Component { | |||
|
|
|||
| handleClick = () => { | |||
| this.setState(prevState => { | |||
| if (this.props.disabled) { | |||
| return { showDropdown: false } | |||
| } | |||
There was a problem hiding this comment.
I think we can save some unnecessary render cycles if we simply add a no-op click handler for disabled
<a className={dropdownTriggerClassname} onClick={!this.props.disabled && this.handleClick}>
ghost
dismissed
|
Thanks for the feedback. Thanks |
|
Awesome! Changes look good on paper. I'll run this branch locally and if everything checks out, will merge it. Stay tuned! |
…tree-select into feat/disableInput
* chore: Update package.json dependencies * feat: Add disabled prop to Input and DropdownTreeSelect * chore: Rebuild bundle.js * style: Show arrow as disabled, update disabled logic * chore: Update docs * chore: Make demo script easier to use
|
🎉 This PR is included in version 1.14.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes #187
I've had to update package.json to get the specific versions ava and babel, otherwise I couldn't get the tests to run properly. I've actually just checked this from PR186
disable search input and dropdown (no dropdown on click)
tests to cover new features
update storybook options
If there's anything i missed or need to do, please let me know.