jQuery: Slide Toggle || Any optimizations to the code?

Question

Hi Im new to programming jQuery and i have a quick code snippet that I would love someone to test for me.

I think it works well.. but i dont know if its the best way to handle it.

working example: http://jsfiddle.net/zWnLv/29/

//hide wrapper at document ready
        $('#newsbox_content_wrapper').hide();

        //toggle visiblility of newsbox and slide down to scroll window to newsbox
        $('.newsbox_toggle').bind('click', function () {
            //define speed for effect
            var $speed = 400;

            //check to see if the class 'open' exists then run
            if ($('.newsbox_toggle').hasClass('open')) {
                //scroll to the top of the newsbox
                $('html, body').animate({scrollTop: $('#header_lower').offset().top}, $speed);
                $('#newsbox_content_wrapper').slideDown($speed);
                $('.newsbox_toggle').removeClass('open');
                //delay HTML replacement to sync with animation
                $('.newsbox_expand').delay($speed).queue(function(n) {
                    $(this).html('Click to Close News Feature.');
                    $(this).addClass('rotate');
                    n();
                });
            } else {
                //scroll back to top of body
                $('html, body').animate({ scrollTop: 0 }, $speed);
                $('#newsbox_content_wrapper').slideUp($speed);
                $('.newsbox_toggle').addClass('open');
                //delay HTML replacement to sync with animation
                $('.newsbox_expand').delay($speed).queue(function(n) {
                     $(this).html('Click to Read More.');
                     $(this).removeClass('rotate');
                     n();
                });
            }
        });

Answer 1

Here you go (with scroll): http://jsfiddle.net/zWnLv/43/

//hide wrapper at document ready and put in var for re-use
var newsbox = $('#newsbox_content_wrapper').hide();

//toggle visiblility of newsbox and slide down to scroll window to newsbox
$('.newsbox_toggle').bind('click', function() {   
    newsbox.slideToggle("slow",function() {
        $('html, body').animate({ scrollTop: newsbox.offset().top }, 'slow');
    });

});

As far as the "click to read more" you have plenty of options... you could toggle the text each time or, my preference, toggle a class with a +/- (or arrows) background image to let a user intuitively know they can open or close that section.

Answer 2

The only way you could "optimize" this is using callbacks instead of manually delaying functions. .slideUp() and .slideDown() accept callbacks to be executed after the animation finishes. Using chaining is a best practice, so you don't have to recreate objects (see the callback functions).

Also, I've changed the bind() function with the new on(), which was added in jQuery 1.7.

$('.newsbox_toggle').on('click', function () {
    //define speed for effect
    var $speed = 400;

    //check to see if the class 'open' exists then run
    if ($('.newsbox_toggle').hasClass('open')) {
        //scroll to the top of the newsbox
        $('html, body').animate({scrollTop: $('#header_lower').offset().top}, $speed);
        $('#newsbox_content_wrapper').slideDown($speed, function() {
            $('.newsbox_expand').html('Click to Close News Feature.').addClass('rotate');
        });
        $('.newsbox_toggle').removeClass('open');
    } else {
        //scroll back to top of body
        $('html, body').animate({ scrollTop: 0 }, $speed);
        $('#newsbox_content_wrapper').slideUp($speed, function() {
            $('.newsbox_expand').html('Click to Read More.').removeClass('rotate');
        });
        $('.newsbox_toggle').addClass('open');
    }
});

If you're on jQuery < 1.7, use .click(), which is a shorthand for .bind().

$('.newsbox_toggle').click(function () {
    // ...
});