Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(jqLite) - toggleClass & removeClass reflect documentation #12852

Closed
wants to merge 3 commits into from
Closed

fix(jqLite) - toggleClass & removeClass reflect documentation #12852

wants to merge 3 commits into from

Conversation

JordanMajd
Copy link

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

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') {
Copy link
Contributor

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') {
Copy link
Contributor

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.
@JordanMajd
Copy link
Author

@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.

@gkalpak
Copy link
Member

gkalpak commented Sep 17, 2015

There is still the inconsistency of the passed index. (It's always 0.)

@JordanMajd
Copy link
Author

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 jqLite and jQuery are intrinsically going to be different. This is because jQuery can work on multiple elements, while jqLite will only operate on a single element. Since jqLite is only operating on one element, the element will always be index 0.

@gkalpak
Copy link
Member

gkalpak commented Sep 17, 2015

Off the top of my head, I think jqLite operates on multiple elements as well.

@JordanMajd
Copy link
Author

@gkalpak thanks,

I just did some digging, you are correct! I will fix this.

@Narretz
Copy link
Contributor

Narretz commented May 23, 2016

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.
Feel free to reopen this PR or open a new one if you have remedied the issues pointed out.

@Narretz Narretz closed this May 23, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jqLite - removeClass cannot accept a function element.toggleClass does not accept a function
4 participants