2
\$\begingroup\$

I have some jquery that pretty much just runs a small drop-line navigation. Each tab size differs, so i used the same jquery line 5 times to make it look and function the way i wanted it to. I'm sure there is a way to condense it but im not quite a pro. Can anybody help me out a bit? The jquery is below, and here is the site for reference: http://solocreativegroup.com/ed_spa/index.php. Thanks!

here's the code that runs my drop-line nav.

$(document).ready(function() {

$("ul#topnav li.home").hover(function() { //Hover over event on list item
    $(this).css({ 'background' : 'url(images/tab_hobt.png) no-repeat top left'}); //Add background color + image on hovered list item
    $(this).find("span").show(); //Show the subnav
}, function() { //on hover out...
    $(this).css({ 'background' : 'none'}); //Ditch the background
    $(this).find("span").hide(); //Hide the subnav
});

$("ul#topnav li.about").hover(function() { 
    $(this).css({ 'background' : 'url(images/tab_hobt.png) no-repeat top left'});
    $(this).find("span").show(); 
    $('.services a').css({'color':'#ffffff'});
}, function() { 
    $(this).css({ 'background' : 'none'});
    $(this).find("span").hide(); 
    $('.services a').css({'color':'#921d49'});
});

$("ul#topnav li.services").hover(function() { 
    $(this).css({ 'background' : 'url(images/tab_services.png) no-repeat top left'});
    $(this).find("span").show(); 
    $('.services a').css({'color':'#ffffff'});
}, function() { 
    $(this).css({ 'background' : 'none'});
    $(this).find("span").hide();
    $('.services a').css({'color':'#921d49'});
});

$("ul#topnav li.testimonial").hover(function() {
    $(this).css({ 'background' : 'url(images/tab_testimonials.png) no-repeat top left'});
    $(this).find("span").show();
}, function() {
    $(this).css({ 'background' : 'none'}); 
    $(this).find("span").hide(); 
});

$("ul#topnav li.contact").hover(function() { 
    $(this).css({ 'background' : 'url(images/tab_contact.png) no-repeat top left'});
    $(this).find("span").show(); 
}, function() {
    $(this).css({ 'background' : 'none'});
    $(this).find("span").hide(); 
});

HTML:

<ul id="topnav">
        <li class="home" style="width:80px;"><a href="index.php">HOME</a></li>
        <li class="about" style="width:80px;"><a href="about.php">ABOUT</a>
         <span>
                <a href="products.php">PRODUCTS</a>
            </span>
        </li>
        <li class="services" style="width:111px;">
            <a href="services.php">SERVICES</a>
            <span>
                <a href="#">FACIALS</a>
                <a href="#">AGE MANAGEMENT</a>
                <a href="#">MICRODERMABRASION</a>
                <a href="skin-resurfacing.php">SKIN RESURFACING</a>
                <a href="add-it-on.php">ADD IT ON</a>
                <a href="#">WAXING</a>
            </span>
        </li>
        <li class="testimonial" style="width:141px;"><a href="testimonials.php">TESTIMONIALS</a></li>
        <li class="contact" style="width:94px;"><a href="contact.php">CONTACT</a></li>
    </ul>
\$\endgroup\$
1
\$\begingroup\$

You're jQuery can be written in one simple function as a catch all for the entire navigation. As you stated you think it could be condensed, you'd be right. As a rule of thumb if you see yourself writing the same thing over and over you should be able to cut it all down into one function.

Starting with one of your methods I'll step through it to show how you can condense this and make it more efficient.

$("ul#topnav li.home").hover(function() { //Hover over event on list item
    $(this).css({ 'background' : 'url(images/tab_hobt.png) no-repeat top left'}); //Add      background color + image on hovered list item
    $(this).find("span").show(); //Show the subnav
}, function() { //on hover out...
    $(this).css({ 'background' : 'none'}); //Ditch the background
    $(this).find("span").hide(); //Hide the subnav
});

First things first, you're creating no less than 5 jQuery objects for each time someone hovers and moves out of a menu item.

Two things on this :

  1. Don't use jQuery where you don't need it
  2. Cache jQuery objects for re-use
  3. Make your code scalable

Don't use jQuery to...

Modify CSS unless it is absolutely required (ie CSS alone can't achieve what you want). In this case CSS can handle you're requirements easily.

If you find you need to change a lot of CSS and would like this CSS to change due to a jQuery or javascript event. use .addClass() and .removeClass(). Applying the styles in CSS rather than in jQuery. This is far more efficient in terms of code and your eyes. :)

Cache your jQuery objects

$(this).css(...);
$(this).find("span");

would be better served with one cached object

var self = $(this);

self.css(...);
self.find("span");

Make your code scalable

By this I mean using techniques that allow you to do some relatively advanced things without the need for re-writes / re-factors in the future. One of these is to bind events to a lower level element with .on rather than .hover or .click. If in the future you modify the menu for some reason with jQuery/JS and DOM elements are removed / re-added, event bindings will be lost. Using .on avoids this problem.

Refactoring your code

I've deliberately not included any CSS changes in this code as you can and should include these in your CSS rather than dynamically with jQuery / JS. I have however added and removed a class, this should contain all the style changes required for the mouseenter and mouseleave events.

$("#topnav").on({
    mouseenter : function(event)
    {
        var self = $(this),
            menu = self.find("span");

        // Check if a menu is present and show it
        if (menu.length > 0)
        {
            menu.show();

            // Add an is-active class to the menu link
            menu.siblings("a").addClass('is-active');
        }
    },
    mouseleave : function(event)
    {
        var self = $(this),
            menu = self.find("span");

        // Check if a menu is present and hide it
        if (menu.length > 0)
        {
            menu.hide();

            // Remove is-active class on the menu link
            menu.siblings("a").removeClass('is-active');
        }
    } 
}, "li");

I hope this helps you!

\$\endgroup\$
  • \$\begingroup\$ ah. immensely good sir. i need to take a class. im self taught in JS, and not very well as you can tell. :) \$\endgroup\$ – MDez May 2 '13 at 17:38
  • \$\begingroup\$ how can i keep the tab active while you're on said page? \$\endgroup\$ – MDez May 2 '13 at 18:14
  • \$\begingroup\$ @MDez no problem at all, glad I could assist. The best way to do this is to apply a class to the menu item based on the page you're navigated to. Whilst you could use JS to do this, it might be better to apply in the backend. \$\endgroup\$ – David Barker May 3 '13 at 8:14

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.