Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have this code and basically it changes color of the lines and move text for menu navigation is there a simpler way of writhing it?

$('.nav1 a').click(function () {
    $('.nav1').addClass("current_nav1");
    $('.sf_menu li').removeClass('current_nav2 current_nav3 current_nav4 current_nav5 current_nav6');
    $('#content, #home').slideDown(2000);
    $('#design, #seo, #galery, #about, #contact').slideUp(2000);
});

$('.nav2 a').click(function () {
    $('.nav2').addClass("current_nav2");
    $('.sf_menu li').removeClass('current_nav1  current_nav3 current_nav4 current_nav5 current_nav6 ');
    $('#content, #design').slideDown(2000);
    $('#home, #seo, #galery, #about, #contact').slideUp(2000);
});

$('.nav3 a').click(function () {
    $('.nav3').addClass("current_nav3");
    $('.sf_menu li').removeClass('current_nav1 current_nav2  current_nav4 current_nav5 current_nav6');
    $('#content, #seo').slideDown(2000);
    $('#home, #design, #galery, #about, #contact').slideUp(2000);
});

$('.nav4 a').click(function () {
    $('.nav4').addClass("current_nav4");
    $('.sf_menu li').removeClass('current_nav1 current_nav2 current_nav3  current_nav5 current_nav6');
    $('#content, #galery').slideDown(2000);
    $('#home, #design, #seo, #about, #contact').slideUp(2000);
});

$('.nav5 a').click(function () {
    $('.nav5').addClass("current_nav5");
    $('.sf_menu li').removeClass('current_nav1 current_nav2 current_nav3 current_nav4  current_nav6');
    $('#content, #about').slideDown(2000);
    $('#home, #design, #seo, #galery, #contact').slideUp(2000);
});

$('.nav6 a').click(function () {
    $('.nav6').addClass("current_nav6");
    $('.sf_menu li').removeClass('current_nav1 current_nav2 current_nav3 current_nav4 current_nav5 ');
    $('#content, #contact').slideDown(2000);
    $('#home, #design, #seo, #galery, #about').slideUp(2000);
});
share|improve this question
    
Is there anyway you can show your relevant html also? It would probably be easier for testing purposes –  wirey Dec 6 '12 at 17:36

1 Answer 1

Why do you need different navXXX and current_navXXX classes? Just have a single

var menus = {//you need some kind of mapping between elements clicked and menu items
  id1: '#home',
  id2: '#design',
  id3: '#seo',
  id4: '#galery',
  id5: '#about',
  id6: '#contact'
};

$('.nav a').click(function () {
    $('.sf_menu li').removeClass('current_nav');
    $(this).addClass("current_nav");

    var selected = menus[$(this).attr('id')];
    $('#content, ' + selected).slideDown(2000);

    for (var menu in menus) {
        if (menus[menu] != selected)
           $(menus[menu]).slideUp(2000);
    }
});
share|improve this answer
    
updated answer as I misread original code –  almaz Dec 6 '12 at 17:56
    
Awesome thanks. Adapted this as each nav is assigned to separate color –  Adam Dec 6 '12 at 19:14

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.