Max selected and misc updates #747
Conversation
1. Fix/finish maxSelected functionality. 2. Fix related test. 3. Fix namespace period issue. 4. Add new event triggers requested by open PR.
src/jquery.multiselect.js
Outdated
| @@ -122,7 +122,7 @@ | |||
| // create a unique namespace for events that the widget | |||
| // factory cannot unbind automatically. Use eventNamespace if on | |||
| // jQuery UI 1.9+, and otherwise fallback to a custom string. | |||
| this._namespaceID = this.eventNamespace || ('multiselect' + multiselectID); | |||
| this._namespaceID = this.eventNamespace.slice(1) || ('multiselect' + multiselectID); | |||
There was a problem hiding this comment.
This mentions that the second condition is for jQuery UI prior to 1.9, can probably just delete it since the widget now requires 1.11.
src/jquery.multiselect.js
Outdated
| this._trigger('beforeCheckAll'); | ||
|
|
||
| var maxSelected = this.options.maxSelected; | ||
| if (maxSelected === null || maxSelected > this.$inputs.length) { |
There was a problem hiding this comment.
I think it might be better to set the checkAllText option to null if maxSelected is set. That way the user just can't press it. It would be a clearer interface for the user interacting with the widget, remove the need for this second check, and we wouldn't need that Check All Not Permitted message.
|
@mlh758 changes committed as requested. |
|
I haven't had a chance to review the whole file yet. I was baking a mountain of cookies last night. I'll try to finish up the review today and get it merged. |
|
@mlh758 Please let me know if additional changes are required to let you merge this. Thx. |
|
I checked out your code this morning and it looks like I can no longer click option group labels to select all the items under that group. |
|
@mlh758 Oops! Sorry. Fixed. |
|
Looks good now, merged it. |
Addresses #717, Closes #737 and PR #584.