views:

99

answers:

4

Here's my current if/else statement:

var current_class = $(this).closest("article").attr("class")

if (current_class == 'opened')
{
  $(this).closest("article").removeClass('opened');
  $(this).closest("article").addClass('closed');
}
else
{
  $(this).closest("article").removeClass('closed');
  $(this).closest("article").addClass('opened');
}

Is there a shorter way to write all of that? Just seems...overweight. :)

+6  A: 

Use

.toggleClass()

Here is an example http://api.jquery.com/toggleClass/

cps7
he has 2 classes tho
Dan
It seems that `.toggleClass()` works with two classes. i.e. `.toggleClass('opened closed')`. Example: http://jsfiddle.net/xaHTn/4/
Ryan Kinal
+2  A: 

IMO this is the cleanest and still clearest solution:

var current_class = $(this).closest("article");
if (current_class.hasClass('opened'))
{
  current_class.removeClass('opened').addClass('closed');
}
else
{
  current_class.removeClass('closed').addClass('opened');
}
Dan
the cleanest would be to use only 1 class (open by default?) then only have the class close. Then use toggle like cps7 suggested.
Dan
hasClass() is an important change. Just doing a string comparison of the class attribute will break if the element has other classes added.
Frank Schwieterman
I would assign a local variable for $(this).closest("article") to avoid doing the DOM lookup twice. That's not a style thing though, it's a speed thing.
telent
A: 

maybe something like:

var current = $(this).closest('article'); // no need to run the selector over and over
var current_class = current.attr('class');
current.removeClass('opened closed').addClass(current_class=='opened'?'closed':'opened');
jrdn
shorter but might be a little harder to read no?
Dan
A: 
$(this).closest("article").toggleClass('opened').toggleClass('closed');
pwb44
You can get a state problem if the item somehow contains both opened and closed.
Dan