current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 0 down vote favorite

I have a navigation, with a sprite that changes based on the class. In my jquery, I'm clearing out he classes so it will just have .nav then addClass the right class based on the click. It works but feels very redundant. Just seeing if anyone has suggestions of optimizing this ?

HTML: 

<div class="grid_12 nav home">
            <ul class="tabs">
                <li class="home"><a href="javascript:;">HOME</a></li>
                <li class="game-stats"><a href="javascript:;">GAME STATS</a></li>
                <li class="game-talk"><a href="javascript:;">GAME TALK</a></li>
                <li class="game-info"><a href="javascript:;">GAME INFO</a></li>
            </ul>
        </div>

JS:

$('DIV.content DIV.nav ul li.home').click(function(){
    $('DIV.content DIV.nav').attr('class', 'nav');
    $('DIV.content DIV.nav').addClass('home');
});

$('DIV.content DIV.nav ul li.game-stats').click(function(){
    $('DIV.content DIV.nav').attr('class', 'nav');
    $('DIV.content DIV.nav').addClass('gamestats');
});

$('DIV.content DIV.nav ul li.game-talk').click(function(){
    $('DIV.content DIV.nav').attr('class', 'nav');
    $('DIV.content DIV.nav').addClass('gametalk');
});
$('DIV.content DIV.nav ul li.game-info').click(function(){
    $('DIV.content DIV.nav').attr('class', 'nav');
    $('DIV.content DIV.nav').addClass('gameinfo');
});
share|improve this question

migrated from stackoverflow.com Jan 4 '12 at 20:08

This question came from our site for professional and enthusiast programmers.
    
what happens to the grid_12 class when you click on something? –  32bitkid Jan 4 '12 at 18:43
    
is the grid_12 class supposed to go away when you click a link? –  32bitkid Jan 4 '12 at 19:16

4 Answers 4

up vote 2 down vote accepted

When an li is clicked, it sets the closest .nav classes to "nav" and the class on the clicked li:

$('.content .nav li').on('click', function(){
    $(this).closest('.nav').attr('class', 'nav ' + $(this).attr('class'));
});
share|improve this answer
    
That's not quite right.. the class he is replacing for 'game-info' is 'gameinfo'. So the current class and the class he is replacing is not the same. –  SKS Jan 4 '12 at 18:43
    
Yes I know but those classes could be easily changed to match for the sake of not having redundant code. div.gameinfo can be unique from li.gameinfo –  anstosa Jan 4 '12 at 18:45
    
Or simply just remove the dash .replace('-', '') –  Jeff Wilbert Jan 4 '12 at 18:46
    
I changed the classes, this was just what I wanted. Thanks! –  Xtian Jan 4 '12 at 18:47
up vote 2 down vote

You can add both classes at once with attr, since all you need to do is pass a string with the classes you want separated by spaces:

$('DIV.content DIV.nav ul li.home').click(function(){
    $('DIV.content DIV.nav').attr('class', 'nav home');
});
share|improve this answer
up vote 1 down vote

How about this? (Untested!)

$('DIV.content DIV.nav ul li').click(function(){
    $(this).closest('DIV.nav').attr('class', 'nav').addClass($(this).attr('class'));
});

The idea is that when an li element is clicked, you take whatever classes are on that li element and apply those classes to the nav element.

The other aspect of this is that I replaced your duplicate references to $('DIV.content DIV.nav') with $(this).closest('DIV.nav'). I prefer this approach because it might be faster (jQuery doesn't have to search through the DOM again), and it also avoids issues in case you have multiple DIV.content and DIV.nav elements on the page.

EDIT: In order to address the issue that the CSS classes that you are applying to the DIV.nav element are slightly different than the CSS classes that are applied to the li elements, you could define the HTML as below:

<div class="grid_12 nav home">
            <ul class="tabs">
                <li class="home"><a href="javascript:;">HOME</a></li>
                <li class="game-stats gamestats"><a href="javascript:;">GAME STATS</a></li>
                <li class="game-talk gametalk"><a href="javascript:;">GAME TALK</a></li>
                <li class="game-info gameinfo"><a href="javascript:;">GAME INFO</a></li>
            </ul>
        </div>

It's a bit of a hack, but not too bad in my opinion.

EDIT: Below is an updated javascript function. This one attempts to preserve any of the existing classes on the DIV.nav element, such as grid_12.

$('DIV.content DIV.nav ul li').click(function(){

    var navDiv = $(this).closest('DIV.nav');

    // first, remove any classes from the li elements that are currently applied to the DIV.nav element
    navDiv.find('ul li').each(function() {
        navDiv.removeClass($(this).attr('class'));
    });

    // next, apply the classes from the clicked li element to the DIV.nav element
    navDiv.addClass($(this).attr('class'));
});
share|improve this answer
    
I like this approach, its more along the lines of the way I would have done it, my only reservation is that $(this).attr('class') == 'game-info' but the handlers add the class of gameinfo (no hyphen). a simple .replace('-','') would resolve it be it feels fragile to me. –  32bitkid Jan 4 '12 at 19:18
    
@32bitkid - ah, yes. I did notice that your CSS class names for your li elements are different than the classes that you are applying to the nav div. Is there a reason for that? I would try to make them the same for the sake of consistency. If you're making them different because of how you've defined your CSS, then perhaps your CSS definitions could be tweaked to make it work. Also, see my updated answer for a simple way to address this issue. –  Dr. Wily's Apprentice Jan 4 '12 at 19:41
    
@32bitkid - woops! sorry, I got you confused with the OP. –  Dr. Wily's Apprentice Jan 4 '12 at 20:00
up vote 0 down vote

Try this javascript:

    $('.content .nav li').click(function(){
    var class_name=$(this).attr('class');
//    class_name.replace('-', '');
        $(this).parents('.nav').addClass(class_name);
    });
share|improve this answer

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.