1

I am designing a responsive layout with a carousel. The carousel has a control that allows the user to toggle the entire box, hiding and showing the carousel. You can see this fully working here: http://pixelcakecreative.com/cimlife/responsive2/

It seems like it is functioning very sluggishly. I was thinking that my jquery was written poorly, and it might be slowing the performance down. My jquery is as follows:

$("#closeBox a").click(function(){
            if ($(this).find('span').hasClass('minus')) {
               $(this).find('span').removeClass('minus').addClass('plus');
               $(".padCar").css("padding-bottom","0");

            } else if ($(this).find('span').hasClass('plus')) {
                 $(this).find('span').removeClass('plus').addClass('minus');
                 $(".padCar").css("padding-bottom","20px");
            }
            $('#carousel').slideToggle('slow');

        return false;
        });

Any ideas why it is so jumpy? Perhaps my jquery should be better written, or maybe it is something else on the page that causes this?

JCHASE11
  • 3,901
  • 19
  • 69
  • 132

3 Answers3

2

Well first off you need to cache your selectors, so you should use

var span = $(this).find('span');

And then just use span.someFunction(); - this reduces DOM queries and will speed it up.

Also what about adding a context to your initial selector? So if you know the links you are targeting are within a div with a class of .myBox, use:

$('#closebox a', '.myBox')

Or even better, use delegate():

$('.myBox').delegate('#closebox a', 'click', function(){ ... });

Update

As John Hartsock and RightSaidFred pointed out, if using v1.7+, you should use on() rather than delegate(), as so:

$('.myBox').on('click', '#closebox a', function(){ ... });
Tom Walters
  • 15,366
  • 7
  • 57
  • 74
  • 1
    If using jQuery 1.7 or higher on() and off() should be used instead of delegate. – John Hartsock Dec 04 '11 at 16:57
  • Tom, Im not sure I understand the latter part of your answer. Why would I use this delegate/on method? – JCHASE11 Dec 04 '11 at 16:59
  • Very true, requiring the simple change of 'delegate' to 'on' in the above code. – Tom Walters Dec 04 '11 at 16:59
  • @JCHASE11 The final part is a better way of binding event listeners to elements, say for instance if at some point you have a dynamically created '#closebox a' element, the click() binding won't apply to it. Here's an article on the subject: http://net.tutsplus.com/tutorials/javascript-ajax/quick-tip-the-difference-between-live-and-delegate/ – Tom Walters Dec 04 '11 at 17:01
  • @TomWalters: Well, you'd need to swap the first two arguments (to use `.on()` that is). – RightSaidFred Dec 04 '11 at 17:01
  • Yes, but because there will be no dynamically added items to the carousel, what is the purpose of using delegate? If it will remain static, would there be any performance increase? – JCHASE11 Dec 04 '11 at 17:03
  • There is some gain, check out http://stackoverflow.com/questions/2954932/difference-between-jquery-click-bind-live-delegate-and-trigger-functions-wit – Tom Walters Dec 04 '11 at 17:05
  • I tried it, and although it works, I dont notice any performance increases – JCHASE11 Dec 04 '11 at 17:13
  • Well I suspect there are other factors, the page you link to is loading scripts in the head, why not move them to the footer? – Tom Walters Dec 04 '11 at 17:17
1

You have a lot of redundancy there, you can start replacing these $(this).find('span') to var span = $(this).find('span')

$("#closeBox a").click(function(){
       var span = $( this ).find( 'span' );
       if ( span.hasClass('minus')) {
           span.removeClass('minus').addClass('plus');
           $(".padCar").css("padding-bottom","0");
       } else if ( span.hasClass('plus')) {
          span.removeClass('plus').addClass('minus');
          $(".padCar").css("padding-bottom","20px");
       }
       $('#carousel').slideToggle('slow');
       return false;
});
0
$("#closeBox a").click(function(){
var spn = $(this).find('span');
            if (spn.hasClass('minus')) {
               spn.removeClass('minus').addClass('plus');
               $(".padCar").css("padding-bottom","0");

            } else {
                 spn.removeClass('plus').addClass('minus');
                 $(".padCar").css("padding-bottom","20px");
            }
            $('#carousel').slideToggle('slow');

        return false;
        });

Sudhir Bastakoti
  • 99,167
  • 15
  • 158
  • 162