Bug Tracker

Modify

Ticket #6889 (closed enhancement: wontfix)

Opened 4 years ago

Last modified 3 years ago

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
Blocking: Blocked by:

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.

Change History

comment:1 Changed 4 years ago by dmethvin

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.

comment:2 Changed 4 years ago by scott.gonzalez

Allowing non-booleans would break jQuery UI's .toggleClass() implementation which allows animating between two classes.

comment:3 Changed 4 years ago by waltertross

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.

comment:4 Changed 3 years ago by SlexAxton

  • Cc kswedberg, paulirish, SlexAxton added
  • Priority set to low
  • Status changed from new to open
  • Milestone changed from 1.4.3 to 1.5

+1 for stronger docs on this. Agree that changing it would break too much potential stuff.

ccing docs team for updates.

comment:5 Changed 3 years ago by john

  • Keywords needsdocs added
  • Status changed from open to closed
  • Resolution set to wontfix

comment:6 Changed 3 years ago by dmethvin

  • Keywords toggleClass boolean parameter needsdocs removed

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.