0
\$\begingroup\$

I have a navigation, with a sprite that changes based on the class. In my jQuery, I'm clearing out the classes so it will just have .nav and then addClass the right class based on the click. It works but feels very redundant. Does anyone have suggestions on 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');
});
\$\endgroup\$
  • \$\begingroup\$ what happens to the grid_12 class when you click on something? \$\endgroup\$ – J. Holmes Jan 4 '12 at 18:43
  • \$\begingroup\$ is the grid_12 class supposed to go away when you click a link? \$\endgroup\$ – J. Holmes Jan 4 '12 at 19:16
2
\$\begingroup\$

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'));
});
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ 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. \$\endgroup\$ – SKS Jan 4 '12 at 18:43
  • \$\begingroup\$ 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 \$\endgroup\$ – Ansel Santosa Jan 4 '12 at 18:45
  • \$\begingroup\$ Or simply just remove the dash .replace('-', '') \$\endgroup\$ – Jeff Wilbert Jan 4 '12 at 18:46
  • \$\begingroup\$ I changed the classes, this was just what I wanted. Thanks! \$\endgroup\$ – Xtian Jan 4 '12 at 18:47
2
\$\begingroup\$

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');
});
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

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'));
});
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ 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. \$\endgroup\$ – J. Holmes Jan 4 '12 at 19:18
  • \$\begingroup\$ @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. \$\endgroup\$ – Dr. Wily's Apprentice Jan 4 '12 at 19:41
  • \$\begingroup\$ @32bitkid - woops! sorry, I got you confused with the OP. \$\endgroup\$ – Dr. Wily's Apprentice Jan 4 '12 at 20:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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