-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[Drop-down-menu] Added multiple items selection support #844
Conversation
Merge from original base
Merge from original base
@@ -23,7 +23,8 @@ var DropDownMenu = React.createClass({ | |||
onChange: React.PropTypes.func, | |||
menuItems: React.PropTypes.array.isRequired, | |||
menuItemStyle: React.PropTypes.object, | |||
selectedIndex: React.PropTypes.number | |||
selectedIndex: React.PropTypes.number, | |||
isMultiple: React.PropTypes.array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool
@oliviertassinari thanks for your review. Mostly there are typos, fixed in next commit. |
@xmityaz If you can squash the 4 commits into one, it's event better |
_handleMultipleSelect: function (key) { | ||
var selectedItems = this.state.selectedItems, | ||
item = this.props.menuItems[key], | ||
index = selectedItems.indexOf(item); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion : https://github.com/airbnb/javascript#13.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it would be better to follow same approach as in the rest of code.
Thanks
@oliviertassinari unfortunately I can't squash first 2 commits. It were merge from original base. I think will be easier to fork repo again and push all changes in 1 commit, then just make new pull request. |
As part of the data table #890 I added the capability to select multiple entries. The multi-select works almost identically to Excel's. We should look at abstracting this logic so that it can be shared between these components and others (the new list component comes to mind). |
Thanks @xmityaz @oliviertassinari @jkruder multi item selection will be available on the new menus that will be released soon. It works much like how selects work, by passing an array of values into the value prop. |
User can set bool property "isMultiple" to make drop-down multiselectable.
Some screens attached bellow