8000 Drop `toggleClass(boolean)` and `toggleClass(undefined)` signatures · Issue #3388 · jquery/jquery · GitHub
[go: up one dir, main page]
Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Drop toggleClass(boolean) and toggleClass(undefined) signatures #3388

Closed
silverwind opened this issue Nov 5, 2016 · 7 comments · Fixed by #4766
Closed

Drop toggleClass(boolean) and toggleClass(undefined) signatures #3388

silverwind opened this issue Nov 5, 2016 · 7 comments · Fixed by #4766
Labels
Attributes Behavior Change
Milestone
4.0.0

Comments

@silverwind
Copy link
Contributor

Can we evaluate if these signatures can be dropped? Here's a few arguments:

  • It can mask bugs when the first argument inadvertently evaluates to undefined, which results in all classes being removed the first time it is called.
  • It's badly documented (Misleading documentation for toggleClass() method. #1696) and because of that, probably not well-known.
  • The way it saves a __className__ data seems like a bad practice.
  • For custom builds, it creates a otherwise unnecessary dependency for attributes/classes towards data.
  • It would allow the following savings for full builds:
   raw     gz Compared to master @ 5b4cb0d33731a58384e02ad51e703e7dcb00e424
  -701   -230 dist/jquery.js
  -168    -61 dist/jquery.min.js
@silverwind
Copy link
Contributor Author
silverwind commented Nov 5, 2016 •
edited
Loading

Looks like I missed that the boolean signature was already deprecated in 3.0. I assume this means the undocumented undefined is also included?

Oh, and what's the target timeframe for removal? 4.0? I'd gladly submit a PR for its removal if the time is right.

@timmywil
Copy link
Member
timmywil commented Nov 7, 2016

@silverwind yes, removal would be in 4.0

@timmywil
Copy link
Member
timmywil commented Nov 7, 2016

And thanks for offering!

@silverwind
Copy link
Contributor Author

Ok, I'll close this as I'm sure you have that planned. If you want to keep it as a reminder for removal, feel free to reopen.

@mgol
Copy link
Member
mgol commented Nov 7, 2016

I think it's good to have it as a reminder, we already have some issues created for 4.0.0 (https://github.com/jquery/jquery/milestone/7) but this one was missing.

@mgol mgol reopened this Nov 7, 2016
@mgol mgol added this to the 4.0.0 milestone Nov 7, 2016
@mgol
Copy link
Member
mgol commented Nov 7, 2016

Thanks for the report!

@dmethvin dmethvin mentioned this issue Jan 29, 2017
Closed
mgol added a commit to mgol/jquery that referenced this issue Jul 29, 2020
@mgol
Attributes: Drop the toggleClass(boolean|undefined) signature
8f01d92 
The behavior of this signature is not intuitive, especially if classes are
manipulated via other ways between `toggleClass` calls.

Fixes jquerygh-3388
@mgol mgol mentioned this issue Jul 29, 2020
Merged
3 tasks
@mgol
Copy link
Member
mgol commented Jul 29, 2020

PR: #4766

@mgol mgol closed this as completed in #4766 Sep 1, 2020
mgol added a commit that referenced this issue Sep 1, 2020
@mgol
Attributes: Drop the toggleClass(boolean|undefined) signature
a442110 
The behavior of this signature is not intuitive, especially if classes are
manipulated via other ways between `toggleClass` calls.

Fixes gh-3388
Closes gh-4766
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Attributes Behavior Change
Milestone
4.0.0
Development

Successfully merging a pull request may close this issue.

Attributes: Drop the toggleClass(boolean|undefined) signature mgol/jquery
3 participants
@silverwind @timmywil @mgol
0