views:

42

answers:

1

I've been playing with jQuery today, and I want to know if I can improve this. I noticed it is a bit slow and not as smooth too. Anyone know why?

Test - http://twomuchwork.com/playground/

My jQuery

/* View Toggle */

$(function(){
 $("#hide-top").toggle( 
  function() {
   $("#logo").fadeTo(1000,0,function(){
    $("#logo").animate({'height':'0'},'fast', function(){
     $("#logo").css("display", "none");
     $("#content").animate({'height':'86%'},'400');
     $("#hide-top").html('Show Top');
    });
   });
  },
  function() {
   $("#content").animate({'height':'70%'},'400', function(){
    $("#logo").animate({'height':'80px'},'fast');
    $("#logo").fadeTo(1000,1);
    $("#hide-top").html('Hide Top');
   });
 });

 $("#hide-right").toggle( 
  function() {
   $("#sidebar ul").fadeTo(1000,0,function(){
    $("#content").animate({'width':'96%'},'fast');
    $("#nav-bg").animate({'width':'96%'},'fast');
    $("#nav-bg").animate({'width':'96%'},'fast');
    $("#hide-right").html('Show Right');
   });
  },
  function() {
   $("#content").animate({'width':'77%'},'fast', function(){
    $("#nav-bg").animate({'width':'76%'},'fast');
    $("#sidebar ul").fadeTo(1000,1);
    $("#hide-right").html('Hide Right');
   });
  }
 );
});
+2  A: 

A couple of things:

Cache your jQuery objects. Each time you do $('selector') it creates a new jQuery object from that selector, which is a very expensive operation. Caching commonly used jQuery objects like this:

var logo = $('#logo');
var nav_bg = $('#nav_bg');

Chain as much as possible. Similarly, chaining makes your code cleaner and more expressive, as well as having to avoid calling the jQuery function repeatedly.

$("#logo").animate({'height':'80px'},'fast').fadeTo(1000,1);

Use built in functions. Since jQuery already has them, why not use them? Using these also makes the code more readable.

// Your's
$("#logo").css("display", "none").fadeTo(1000,1);

// jQuery's
$('#logo').hide().fadeIn(1000);

And this last one really depends on what you're doing this for, but don't hard code numbers into the script. Put them in variables so that you can change them as necessarily, instead of having to find-and-replace every one of the same values when you need them change.

Yi Jiang
+1 - I am going to put variables for the percentages right now.
Doug
I only have 1 occurrence of most of the hard coded numbers. Should I really create a variable for each?
Doug
Is there a way to revert to the original width/height, so I do not have to hard code those numbers in the toggle?
Doug
@Doug Not really, if you're lazy ;) but using variables keeps the script tidy and all of the numbers in one place. Alternatively, grab the original width using the `css` function, then use that instead.
Yi Jiang