views:

158

answers:

3

I have the following jQuery statement which works fine but I was wondering if it could be shortened and if making it more concise would speed up performance or not?

The reason for the strange starting tag is that it is for a wordpress theme.

Cheers in advance!

jQuery(document).ready(function($) {

$(".sidebar ol#navigation li.work a").click(function() {
$("#content #second_nav").toggle("fast");
$("#content #second_nav ul.options_3").css('display','none');
$("#content #second_nav ul.options_2").css('display','none');
$("#content #second_nav ul.options_4").css('display','none');
$('.active_2').removeClass('active_2');
$('.active').removeClass('active');
$(this).toggleClass('selected');
});

$("#content #second_nav ul.options li a#work").click(function() {
    $('.active').removeClass('active');
    $(this).attr('class','active');
    $("#content #second_nav ul.options_2").toggle("fast");
    $("#content #second_nav ul.options_4").css('display','none');
    $("#content #second_nav ul.options_3").css('display','none');
    });

$("#content #second_nav ul.options li a#writing").click(function() {
    $('.active').removeClass('active');
    $(this).attr('class','active');
    $("#content #second_nav ul.options_4").toggle("fast");
    $("#content #second_nav ul.options_3").css('display','none');
    $("#content #second_nav ul.options_2").css('display','none');
    $('.active_2').removeClass('active_2');
    });

$("#content #second_nav ul.options_2 li a#collage").click(function() {
    $('.active_2').removeClass('active_2');
    $('ul.options_3').css('display','none');
    $(this).attr('class','active_2');
    $("#content #second_nav ul.options_3#collage").toggle("fast");
    });

$("#content #second_nav ul.options_2 li a#painting").click(function() {
    $('.active_2').removeClass('active_2');
    $('ul.options_3').css('display','none');
    $(this).attr('class','active_2');
    $("#content #second_nav ul.options_3#painting").toggle("fast");
    });

$("#content #second_nav ul.options_2 li a#print").click(function() {
    $('.active_2').removeClass('active_2');
    $('ul.options_3').css('display','none');
    $(this).attr('class','active_2');
    $("#content #second_nav ul.options_3#print").toggle("fast");
    });

$("#content #second_nav ul.options_2 li a#photo").click(function() {
    $('.active_2').removeClass('active_2');
    $('ul.options_3').css('display','none');
    $(this).attr('class','active_2');
    $("#content #second_nav ul.options_3#photo").toggle("fast");
    });

});
+5  A: 

For performance sake:

  1. it's best to use ID and CLASSES with as few nodes in the selector as possible. If you use an ID, don't specify the tag. So $('#myId'); is much faster than $('a#myId'); as it uses the native javascript getElementById(). If you specify the tag, then it resorts to more complex selection strategies. Also, ID are supposed to be unique in the DOM, so instead of $("#content #second_nav ul.options_2 li a#photo") use $("#photo"); . The shorter, the faster, the better.

  2. Cache your selected DOM elements if you intend to use them more than once:

    var secondNav= $("#content #second_nav");
    secondNav.toggle("fast");
    
  3. For concision, you can specify several elements in the selector if they share the same operation. For instance:

    var secondNav = $("#content #second_nav");
    secondNav.toggle("fast");
    $("ul.options_3, ul.options_2, ul.options_4",secondNav).hide();
    $('.active_2').removeClass('active_2');
    $('.active').removeClass('active');
    $(this).toggleClass('selected');
    

Hope this helps...

pixeline
sorry, i can't seem to get the code to display highlighted...
pixeline
I would add chaining whenever you can.
Elzo Valugi
+1  A: 

There are a few things to do. The most obvious one that comes to mind is to use variables.

var $option2 = $("#content #second_nav ul.options_2");
var $option3 = $("#content #second_nav ul.options_3");
var $option4 = $("#content #second_nav ul.options_4");

You can then sub in the variables wherever you have that long selector.

$option2.css('display', 'none');
$option3.css('display', 'none');
$option4.css('display', 'none');

Should help with performance, readabilty, and duplication.

Patrick McElhaney
+1  A: 

$("#content #second_nav ul.options_3").css('display','none'); $("#content #second_nav ul.options_2").css('display','none');

== $("#content #second_nav").find("ul.options_3,ul.options_2").css('display','none');

01