-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(jqLite) - toggleClass & removeClass reflect documentation #12852
Conversation
toggleClass & removeClass changes make functions reflect documentation and jQuery 1.4 compatible. toggleClass now can accept a function as a parameter, a function and array of states as a parameter as well as empty parameters. removeClass now can accept a function as a parameter as well as empty parameters. closes #12793 closes #12851
function jqLiteRemoveClass(element, cssClassesOrFunc) { | ||
if (element.setAttribute) { | ||
if (cssClassesOrFunc) { | ||
if (typeof cssClassesOrFunc === 'function') { |
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.
can use isFunction
}); | ||
|
||
// the class name to be toggled can be determined by passing in a function. | ||
if (typeof selector === 'function') { |
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.
-> isFunction
remove class is matches documentation and is jQuery 1.4 compatible. Changes all function and array checks to use isArray and isFunction.
Updates removeClass to accept function in spec with 1.9 (current) jQuery docs. Updates removeClass to accept no parameters in spec with 1.9 (current) jQuery docs. Updates toggleClass to accept function in spec with 1.9 (current) jQuery docs. Updates toggleClass to accept no parameters in spec with 1.9 (current) jQuery docs. Provides tests for all changes.
@Narretz , thanks for the reviews. I've fixed the items you had mentioned as well as a few other things I had noticed that could use improvement and am ready for another review. |
There is still the inconsistency of the passed index. (It's always 0.) |
Thanks for the input. However, I think passing anything but 0 would be inconsistent. Please correct me if I'm mistaken, but it is my understanding that |
Off the top of my head, I think |
@gkalpak thanks, I just did some digging, you are correct! I will fix this. |
I'm going to close this PR because we cannot merge it in its current state, as we are either waiting for more information, tests are missing, or required code changes haven't been made. |
Updates removeClass to accept function in spec with 1.9 (current) jQuery docs.
Updates removeClass to accept no parameters in spec with 1.9 (current) jQuery docs.
Updates toggleClass to accept function in spec with 1.9 (current) jQuery docs.
Updates toggleClass to accept no parameters in spec with 1.9 (current) jQuery docs.
Provides tests for all changes.
fixes #12793
fixes #12851