Eliminate replacement _toggleChecked code in multiselect.filter.js #745
Conversation
Replaces the separate _toggleChecked function code in multiselect.filter.js with instead using a parameter that changes the context for _toggleChecked in multiselect.js . Also, a few housekeeping updates. Note: The events.js file that I pulled has tabs in it. What I am pushing back has spaces instead. (consider this fair warning lol)
src/jquery.multiselect.filter.js
Outdated
| // is fired, only the currently displayed filtered inputs are checked | ||
| var $instance = this.instance; | ||
| $instance._oldToggleChecked = $instance._toggleChecked; | ||
| $instance._toggleChecked = function(flag, group, filteredInputs) { |
There was a problem hiding this comment.
It looks like the filteredInputs param isn't necessary since you're fixing it to true below.
There was a problem hiding this comment.
Yeah, it could probably be dropped.
There was a problem hiding this comment.
Ok just did another commit that removes it.
I'm tempted to just hit every file with a code formatter before we tag this just to finally get it all consistent. It looks like you changed |
I think the other thing to note is that the filtering depends on knowing / tracking the hidden state of the inputs in the list. I am wondering if it may be better to apply/remove a new class name w/ CSS rule display:none rather than directly determining the state of an intrinsic characteristic? (Also, PR #674 comes to mind... it would likely cause an interaction w/ the filtering.) If you want me to do a new class name like I am alluding to, I could do that in another PR. If you are ok w/ this one, then I would just suggest to merge it. |
|
Heh, I meant in that file. This PR looks good, if you want to change the underlying behavior on how the filtering is done that can be another discussion and PR. |
2 participants
Handles #733
Replaces the separate _toggleChecked function code in multiselect.filter.js with instead using a parameter that changes the context for _toggleChecked in multiselect.js .
Also, a few housekeeping updates.
_Note: The events.js file that I pulled has tabs in it. What I am pushing back has spaces instead. (consider this fair warning...)