CODEWITHSUNDEEP
javascriptjquery
darcher
darcher

Reputation: 3134

collapsing an accordion "li" when class is removed

I've been trying to sort through this for a while now, can't seem to get it working 100%. The current code reveals the .content area without any collapse. It also shifts the page down to center on the content (which in this case goes under the fold if you don't). 

$(document).ready(function() {
    $('.vl-option .vl-toggle-link').click(function(e) {
        e.preventDefault();
        $(this).closest('li').find('.content').not(':animated').slideToggle();
        $(this).closest('li').toggleClass('active');
    });
    $('.vl-option').bind('click',function(){
        var self = this;
        setTimeout(function() {
            theOffset = $(self).offset();
            $('body,html').animate({ scrollTop: theOffset.top - 20 });
        }, 310);
    });
});

I've attempted a few approaches to no avail. I'll list a few below (concept was to find any siblings and collapse):

$(this).siblings('li').find('.content').slideToggle();

which actually breaks the original functionality. Then I went with the following (to try an make anything without class="active" collapse):

if ( $(this).siblings('li').not('.active').find('.content').slideToggle(); ) //also tried .hide()

which doesn't seem to have any affect on anything.

The HTML is simple

<ul>
  <li class="lv-option active"><!-- when toggled, "active" class is applied... -->
    <a href="#" class="vl-toggle-link">Yada</a>
    <div class="content"></div>
  </li>
  <li class="lv-option"><!-- ...when untoggled, "active" class removed -->
    <a href="#" class="vl-toggle-link">Yada yada</a>
    <div class="content"></div>
  </li>
</ul>

.active is only applied for stylistic reasons. it has no effect on the functionality of anything. Just needed to be able to target the :before / :after when something was selected.

I just can't seem to wrap my head around jquery... argh.

Upvotes: 0

Views: 149

Answers (1)

Mark S
Mark S

Reputation: 3799

Only one content at a time should be opened in an accordion right, so applying .slideToggle() to all <li> contents will break this rule. I think it's okay with your markup since you only have two <li>, so they just slides alternately. But if you have more I think the active <li> only should be .slideToggle() other should be .slideUp() only.

You can just chain them all:

$('.vl-toggle-link').click(function(e) {
    e.preventDefault();
    //find the sibling content apply slideToggle, then move to their parent and add active
    $(this).siblings('.content').slideToggle().parent('li').addClass('active')
    // go through all the siblings of their parent and remove active, then slideUp their child content
    .siblings('li').removeClass('active').children('.content').slideUp();    
});

I also find your .animate({scrollTop}) not working properly because it's obtaining the old offset().top value. I think you should get the offset().top after the slideToggle() has finished. See this jsfiddle.

Or you can also calculate scrollTop: jsfiddle.

Upvotes: 1

Related Questions