Using .offset().top with .slideUp

Question

I'm trying to make the window scroll to an element on click. It is working, except when the slideUp animation of one of the previous elements fires, the clicked element scrolls up way past the top.

Here's the link: http://jtwebfolio.com/mobi

NOTE: Size your browser down to mobile width to best see the issue. Also, this is taking place on the portfolio projects.

Here's the code:

$('article.project header').click(function(){

    if($(this).parent().hasClass('clicked')){
        // If the clicked project has already been clicked, remove the class and hide the content
        $(this).parent().removeClass('clicked');
        $(this).parent().find('.proj_content').slideUp('fast');
    }else{
        // Remove the class and slide the content up on all projects
        $('article.project').removeClass('clicked');
        $('article.project .proj_content').slideUp('fast');

        // Add the class and show the content on the selected project
        $(this).parent().addClass('clicked');
        $(this).parent().find('.proj_content').slideDown('fast');

        // Slide the project to the top after clicking on it
        $('html, body').delay(500).animate({
            scrollTop: $(this).parent().offset().top
        }, 500);
    }

});

If someone could help me produce the desired effect, that would be amazing. Thanks in advance!

Answer 1

A couple of things:

• Your .slideDown()s use the speed fast, but you're only delaying by 500ms. fast is synonymous with 600ms. In general, I think it's a bad idea to use both as it's very confusing to someone reading your code what fast is.
• Rather than using .delay() method, I'd use the complete argument on the slideDown or slideUps, so that once they're complete, you do your scrolling. This makes more sense, as you'd then not have to worry about conflicting timings.
• My guess would be that your problem is caused by your transitions taking 600ms, but your scroll only waiting 500ms. At 500ms, it's getting the wrong offset values and scrolling to those.

• Perhaps you should consider changing your markup to something like:

  $(this).parent().find('.proj_content').slideDown('600', function() {
    // Slide the project to the top after clicking on it
    $('html, body').animate({
        scrollTop: $(this).parent().offset().top
    }, 500);
  });
  

* Note: I've changed fast to 600 for clarity's sake.

Answer 2

try this :

$('article.project').on('click','header',function(){
    _parent = $(this).parent();
    if(_parent.hasClass('clicked')){
        // If the clicked project has already been clicked, remove the class and hide the content
        _parent.removeClass('clicked');
               .find('.proj_content')
               .slideUp('fast');

    }else{
        // Remove the class and slide the content up on all projects
        $('article.project').removeClass('clicked')
                            .children('.proj_content')
                            .slideUp('fast');
        // Add the class and show the content on the selected project
        // Slide the project to the top after clicking on it
        _parent.addClass('clicked')
               .find('.proj_content')
               .slideDown('fast',function(){
                   // callback
                   $('html, body').animate({ scrollTop: +(_parent.offset().top)+'px' }, 500);
               });
    }

});