jQuery click(function()) does not use variable on first click

Question

I have this tiny script:

var $totalHeight;

$(document).ready(function() {
    $totalHeight = $('#sidebar-container').innerHeight();
    $('.sidebar-heading').each(function() {
        $totalHeight = ($totalHeight - $(this).outerHeight());
    });

    $('.sidebar-contents').each(function() { 
        if ($(this).hasClass('active')) {
            $(this).css('height',$totalHeight);
        }
    });

    $('.sidebar-heading').click(function() {
        $('.sidebar-contents').slideUp().removeClass('active').addClass('inactive')/*.css('height','').removeAttr('style')*/;
        $('.sidebar-heading').removeClass('active')/*.next().slideUp()*/;
        $(this).addClass('active').next().addClass('active').removeClass('inactive').css('height',$totalHeight).slideDown();
    });
});

I am pretty sure it's obvious what it is supposed to do.

My problem is: The first click on a ('.sidebar-heading') does not apply the desired $totalHeight to its sibling. The following clicks, it does. What am I doing wrong?

I have a draft-HTML with this code in action. I commented out those parts above to check, where the bug is, but couldn't figure it out anyway.

Seems to work fine for me. Maybe you do need to be a little more explicit in what you expect it to do and what it is actually doing. — Matt Burland
– Matt Burland, Commented Jan 18, 2013 at 17:45
Check out this FIDDLE .. the first click doesn't expand the full height.. but every other one does — wirey00
– wirey00, Commented Jan 18, 2013 at 17:47
the content-area should always be at full height. "Inactive" heading elements are supposed to either be at top or at bottom. not in between. With the first click, the following heading-elements are somewhere in the middle, not at the bottom of the container. — rob_st
– rob_st, Commented Jan 18, 2013 at 17:47
Any reason to name an integer variable $totalHeight? This is not PHP — John Dvorak
– John Dvorak, Commented Jan 18, 2013 at 17:49
I'm unable to reproduce this behavior using Chrome (v24). What browser are you seeing this behavior in? — pete
– pete, Commented Jan 18, 2013 at 17:50

John Dvorak · Accepted Answer · 2013-01-18 18:30:50Z

3

The slideUp() animation takes a while to run and cleans up all the heights and display styles after it gets done. This erases the setting you made in the next two lines.

Try waiting until slideUp() is done to do the other ones.

   $('.sidebar-heading').click(function() {
        var self = this;
        $('.sidebar-contents').slideUp(undefined, function() {
            $('.sidebar-heading').removeClass('active')/*.next().slideUp()*/;
            $(self).addClass('active')
                .next().addClass('active').removeClass('inactive')
                .css('height',$totalHeight).slideDown();
        }).removeClass('active').addClass('inactive')
        /*.css('height','').removeAttr('style')*/;
    });

I figured this out by running in Chrome with the debugger and noticing that the styles were changing in a wierd way. The first click on the 2nd heading would leave the height set on the 1st heading even though the code was clearing it. That means something was, maybe, setting it wrong. So I tried the above code on the jsFiddle someone set up. Lo and behold ...

edited Jan 18, 2013 at 18:30

John Dvorak

27.4k13 gold badges73 silver badges86 bronze badges

answered Jan 18, 2013 at 18:22

Lee Meador

13k2 gold badges38 silver badges43 bronze badges

Sign up to request clarification or add additional context in comments.

8 Comments

Lee Meador Over a year ago

Probably copied something wrong from the fiddle. Too many lines copied. Ill fix it.

John Dvorak Over a year ago

I guess I was just confused by the lack of indentation. FTFY.

John Dvorak Over a year ago

Can you provide the fixed fiddle? I would like to see your fix live.

John Dvorak Over a year ago

Oh. The first animation was removing the height. I was trying to catch the second one red-handed.

Lee Meador Over a year ago

Fixed fiddle showing the code change and thanks @JanDvorak for the reformatting.

|

Community · Accepted Answer · 2017-05-23 12:12:16Z

2

You can also let the fx function set the height by itself to avoid conflict mentioned in @LeeMeador's answer.

Since the slideDown() doesn't accept any css value, we can use the animate() function directly instead.

$('.sidebar-heading').click(function () {
    $('.sidebar-heading').removeClass('active');
    $('.sidebar-contents').removeClass('active').addClass('inactive').stop().animate({ height: '0px' });
    $(this).addClass('active').next().addClass('active').removeClass('inactive').stop().animate({ height: $totalHeight + 'px' });
});

In addition, use stop() before perform any animation to prevent weird behavior if you click the heading again while the previous animation haven't finished.

Modified jsfiddle: http://jsfiddle.net/z62tr/4/

edited May 23, 2017 at 12:12

CommunityBot

11 silver badge

answered Jan 18, 2013 at 20:09

runTarm

11.6k1 gold badge39 silver badges37 bronze badges

1 Comment

rob_st Over a year ago

I also thought about an approach using animate(). Thanks for this solution. On the first glance, it works for me :-)

Collectives™ on Stack Overflow

jQuery click(function()) does not use variable on first click

2 Answers 2

8 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

8 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related