feat(Dropdown): add closeOnChange prop #1252
Conversation
apiv
force-pushed
the
feature/Dropdown-close-on-change
branch
from
c41c2eb to
a6443f8
Compare
| @@ -17,6 +17,12 @@ const DropdownUsageExamples = () => ( | |||
| /> | |||
|
|
|||
| <ComponentExample | |||
| title='Close On Change' | |||
| description='A multiselect can close when the user changes its value.' | |||
There was a problem hiding this comment.
Nit: for consistency with the props, can we go with A multiple selection dropdown... here?
| @@ -85,6 +85,9 @@ export default class Dropdown extends Component { | |||
| /** Whether or not the menu should close when the dropdown is blurred. */ | |||
| closeOnBlur: PropTypes.bool, | |||
|
|
|||
| /** Whether or not the menu should close when a value is selected from the dropdown */ | |||
There was a problem hiding this comment.
Since the default is hidden from the user (logic based in the method) perhaps we should note the default here. Let's add the period to the end of this sentence while we're at it.
| const { closeOnChange, multiple } = this.props | ||
| const shouldClose = _.isUndefined(closeOnChange) | ||
| ? !multiple | ||
| : closeOnChange |
There was a problem hiding this comment.
The false branch of this logic is untested, this is why Codecov is failing the PR:
tip: the coverage data overlayed here in GH is added by the Codecov extension
Even though the other branches are executed during tests, we don't actually have assertions for this new feature. We'll need a new describe('closeOnChange', ... ) block in the Dropdown-test.js where we can test this new prop.
There was a problem hiding this comment.
Thanks for the heads up, specs added, changes made and re-pushed
apiv
force-pushed
the
feature/Dropdown-close-on-change
branch
from
a6443f8 to
34165f8
Compare
apiv
force-pushed
the
feature/Dropdown-close-on-change
branch
from
34165f8 to
bf6b0ba
Compare
|
Superb, thanks much! I'll release this within the hour. |
|
As promised, released in |
fulfills issue #1244, adding a
closeOnChangeprop to Dropdown, with usage example added to the docs.