Bug Tracker

Opened 7 years ago

Closed 7 years ago

#12639 closed feature (wontfix)

Make toggleClass('active', function() { return bool; }) work

Reported by: oyasumi@… Owned by: oyasumi@…
Priority: low Milestone: None
Component: attributes Version: 1.8.0
Keywords: 1.9-discuss Cc:
Blocked by: Blocking:

Description

toggleClass('active', bool) already works, setting or removing the "active" class dependent on the value of the passed boolean.

Please allow the boolean to be a boolean-returning function, deferring the decision to code. Thanks!

Change History (11)

comment:1 Changed 7 years ago by Rick Waldron

Owner: set to oyasumi@…
Status: newpending

No use case? Well then... what's the use?

comment:2 Changed 7 years ago by oyasumi@…

Status: pendingnew

Principle of least surprise and largest utility. Being able to write this:

$buttons = $('ul.buttons li a');
$buttons.click(function() {
  var clicked = this;
  $buttons.toggleClass('active', function() { return this === clicked });
});

instead of (as you currently need to) this:

$buttons = $('ul.buttons li a');
$buttons.click(function() {
  var clicked = this;
  $buttons.each(function() {
    $(this).toggleClass('active', this === clicked);
  });
});

What is particularly unfortunate about the current behaviour is that first code example doesn't throw any error - it just behaves as if the function passed wasn't even there, plain toggling the class for all elements in the jQuery selection.

comment:3 Changed 7 years ago by scottgonzalez

That use case is usually handled with something along the lines of:

$buttons.not( this ).removeClass( "active" );
$( this ).addClass( "active" );

comment:4 Changed 7 years ago by oyasumi@…

To me the point of using toggleClass in the first place is to write DRY code.

Anyway, I am not here to argue or press my point – I thought it was just an accidental bug or missed corner case, especially as I've been using the great d3.js idiom selection.classed('active', bool_or_function_returning_bool) long enough to have that hard-wired, rather than breaking it down into multiple discrete manual steps in separate chained commands, or using a more complex chain, as in the fallback I opted for above.

It would just delight me if jQuery matched intuition.

comment:5 Changed 7 years ago by oyasumi@…

comment:6 Changed 7 years ago by sindresorhus

Component: unfiledcss
Priority: undecidedlow

or

$buttons.removeClass('active').siblings().addClass('active');

comment:7 Changed 7 years ago by timmywil

Component: cssattributes
Keywords: 1.9-discuss added
Status: newopen
Type: bugfeature

comment:8 Changed 7 years ago by mikesherov

+1

comment:9 Changed 7 years ago by gnarf

+0, What gets passed into the function... index, hasClass ???

comment:10 Changed 7 years ago by dmethvin

-1, not needed

comment:11 Changed 7 years ago by mikesherov

Resolution: wontfix
Status: openclosed
Note: See TracTickets for help on using tickets.