5
\$\begingroup\$

I'm trying to come up with a way to make a drop down box that is displayed through a jquery mouse hover event and with nested dropdown boxes displayed through hovering over elements of the original drop down box. I wrote some terribly inefficient code and I'm struggling to find ways of simplifying it. If anyone has any suggestions that will help me shorten this code and get a better idea of how to take advantage of functions of JQuery, please help.

here is the link:

http://cs-dev.dreamhosters.com/dropd.php

$(document).ready(function(){
$(".tab, .drop").hover(function(){
$(".tab").css("color","#FF7722");
$(".drop").css("display","block");
$("#tv, .droptv").hover(function(){
        $(this).css("color","#FF7722");
        $(".droptv").css("display","block");
        $(".droptv").hover(function(){
                $("#tv, .droptv").css("color","#FF7722");
                },function(){
                $(".droptv").css("color","#005BAB");
                });
        },function(){
        $(this).css("color","#005BAB");
        $(".droptv").css("display","none");
        });
$("#interact").hover(function(){
        $(this).css("color","#FF7722");
        },function(){
        $(this).css("color","#005BAB");
        });
$("#online").hover(function(){
        $(this).css("color","#FF7722");
        },function(){
        $(this).css("color","#005BAB");
        });
$("#vod, .dropvod").hover(function(){
        $(this).css("color","#FF7722");
        $(".dropvod").css("display","block");
        $(".dropvod").hover(function(){
                $("#dai").hover(function(){
                    $(this).css("color","#FF7722");
                    },function(){
                    $(this).css("color","#005BAB");
                    });
                $("#iguide").hover(function(){
                    $(this).css("color","#FF7722");
                    },function(){
                    $(this).css("color","#005BAB");
                    });
                $("#vod").css("color","#FF7722");
                },function(){
                $(".dropvod").css("color","#005BAB");
                });
        },function(){
        $(this).css("color","#005BAB");
        $(".dropvod").css("display","none");
        });
$("#tablet").hover(function(){
        $(this).css("color","#FF7722");
        },function(){
        $(this).css("color","#005BAB");
        });
$("#mobile").hover(function(){
        $(this).css("color","#FF7722");
        },function(){
        $(this).css("color","#005BAB");
        });
},function(){
$(".tab").css("color","#005BAB");
$(".drop").css("display","none");
 });
});
\$\endgroup\$
  • \$\begingroup\$ Try using JavaScript for state and CSS for style. It would make this code so much simpler! \$\endgroup\$ – ANeves Dec 6 '12 at 10:08
4
\$\begingroup\$

Your code is very big and messy so this a bit tricky to really see what you are trying to do. A few obvious things to make the code more readable:

  • Replace .css("display","block") with .show()
  • Replace .css("display","none") with .hide()
  • You repeat the same color changing hover over and over again. Instead, group all your elements together and specify this function only once:

    $("#containo div").hover(function() {
    $(this).css("color","#FF7722");
    },function() {
    $(this).css("color","#005BAB");
    });

  • Do not nest hovers inside other hovers. .hover() creates a new event handler when it is called. If you nest them then you each time you move your mouse over the parent, the child is assigned a new event handler. You do not want these duplicates. Instead assign all of your events in the root level.

 

This, plus a little refactoring could reduce your code considerably. Maybe from here the code will be easier to work with so could see how to reduce it further.

$(document).ready(function() {

    function on(selector) {
        $(selector).css("color","#FF7722");
    };
    function off(selector) {
        $(selector).css("color","#005BAB");
    };

    $("#interact,#online,#tablet,#mobile,#dai,#iguide,#tv,.droptv,#vod,.dropvod")
    .hover(function(){
        on(this)
    },function(){
        off(this)
    });

    $("#tv, .droptv").hover(function(){
        $(".droptv").show();
    },function(){
        $(".droptv").hide();
    });

    $("#vod, .dropvod").hover(function(){
        on("#vod");
        $(".dropvod").show();
    },function(){
        off(".dropvod");
        $(".dropvod").hide();
    });

    $(".tab, .drop").hover(function(){
        on(".tab");
        $(".drop").show();
    },function(){
        off(".tab");
        $(".drop").hide();
    });
});
\$\endgroup\$
3
\$\begingroup\$

I would also suggest, in addition to Buh Buh's answer:

  • Save the references to the elements in a variable, so that instead of $('.tv') and making jQuery perform a search through the DOM every time, you can refer to your variable and apply jQuery methods to it. Furthermore, this can help if you also can give meaningful names to those variables. For example

    var $mainCombos = $('.tv'); // later on $mainCombos.show();

I always use a dollar sign to indicate which of my variables are jQuery objects versus regular variables, but it is really not needed.

Beware that this may bring you problems if you're dynamically adding or removing elements from the DOM. If this is the case, you may use alternative versions of this, like re-setting the reference variable each time your code is called, or applying live events and grasping the references with $(this).

  • Chain calls to jQuery methods whenever possible. This prevents jQuery from searching all over again for those elements (also useful if you can't apply my first suggestion). For instance:

Instead of

$('.droptv').show();
$('.droptv').hover(...);

You could use

$('.droptv').show()
            .hover(...);
\$\endgroup\$

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.