Side navigation
#6889 closed enhancement (wontfix)
Opened August 09, 2010 04:50PM UTC
Closed April 17, 2011 12:51AM UTC
Last modified May 20, 2011 04:27PM UTC
the second parameter of toggleClass() should be allowed to be non-boolean
Reported by: | waltertross | Owned by: | |
---|---|---|---|
Priority: | low | Milestone: | |
Component: | manipulation | Version: | 1.4.2 |
Keywords: | Cc: | kswedberg, paulirish, SlexAxton | |
Blocked by: | Blocking: |
Description
var modif = $('#x').hasClass('xModif') OR $('#y').find('.yModif').length;
$('#z).toggleClass('zModif', modif);
(please replace the OR with a double vertical bar, which the tracking system refuses to display)
This example doesn't work currently, unless one replaces length with length > 0, because modif will be true if hasClass() returns true, but it will be an integer otherwise, and toggleClass() currently only uses the second parameter if it is of boolean type.
This example is particularly tricky, because many people tend to think that the OR condition always produces a boolean, and because it will not fail in all cases.
The suggested enhancement is to ignore the second parameter of toggleClass() only if it is undefined, converting it to boolean otherwise.
Alternatively, a big warning in the documentation is suggested.
Attachments (0)
Change History (6)
Changed August 09, 2010 11:46PM UTC by comment:1
Changed August 10, 2010 02:13AM UTC by comment:2
Allowing non-booleans would break jQuery UI's .toggleClass() implementation which allows animating between two classes.
Changed August 10, 2010 09:18AM UTC by comment:3
I hadn't noticed toggleClass() in the UI because I'm new to jQuery. In my eyes, this is a small case of bad design.
A warning in the documentation, maybe exceptionally mentioning the UI, seems to be the only viable solution.
Changed October 25, 2010 07:39AM UTC by comment:4
cc: | → kswedberg, paulirish, SlexAxton |
---|---|
milestone: | 1.4.3 → 1.5 |
priority: | → low |
status: | new → open |
+1 for stronger docs on this. Agree that changing it would break too much potential stuff.
ccing docs team for updates.
Changed April 17, 2011 12:51AM UTC by comment:5
keywords: | toggleClass boolean parameter → toggleClass boolean parameter needsdocs |
---|---|
resolution: | → wontfix |
status: | open → closed |
Changed May 20, 2011 04:27PM UTC by comment:6
keywords: | toggleClass boolean parameter needsdocs |
---|
I've been bitten by this before as well so now it's burned into my memory. Given the current signature of .toggleClass() it seems like it would be possible to treat the switch arg as truthy/falsy as long as it is not undefined, or check for arguments.length>1.