HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

jQuery functions to toggle navbar menus

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
menusjqueryfunctionsnavbartoggle

Problem

This is the first JS/JQuery script I had written for my website, few months ago. It toggles/hide/show menus (and some sub-menus/interactions) when a user clicks on specific items from the navbar.

Right now I have 6 main menus, and I believe my code is far from being optimized.

```
$(document).ready(function(){

$(document).click(function(event) {
$("#website-header #tab-lang").hide("slide", { direction: "right" }, 300);
$("#website-header #tab-apps").hide("slide", { direction: "right" }, 300);
$("#website-header #tab-apps #apps-reveal").delay(300).hide(0);
$("#website-header #tab-dashboard").hide("slide", { direction: "right" }, 300);
$("#website-header #tab-login").hide("slide", { direction: "right" }, 300);
$("#website-footer #tab-about").hide("slide", { direction: "down" }, 300);
$("#website-footer #tab-about #credits-reveal").delay(300).hide(0);
$("#website-footer #tab-contact").hide("slide", { direction: "down" }, 300);
});

// lang tab
$("#website-header #tab-lang").hide().click(function(event) {
event.stopPropagation();
});
$("#website-header #nav-lang").click(function(event) {
event.stopPropagation();
$("#website-header #tab-lang").toggle("slide", { direction: "right" }, 300);
$("#website-header #tab-apps").hide("slide", { direction: "right" }, 300);
$("#website-header #tab-apps #apps-reveal").delay(300).hide(0);
$("#website-header #tab-dashboard").hide("slide", { direction: "right" }, 300);
$("#website-header #tab-login").hide("slide", { direction: "right" }, 300);
$("#website-footer #tab-about").hide("slide", { direction: "down" }, 300);
$("#website-footer #tab-about #credits-reveal").delay(300).hide(0);
$("#website-footer #tab-contact").hide("slide", { direction: "down" }, 300);
});

// apps tab
$("#website-header #tab-apps").hide().click(function(event) {
event.stopPr

Solution

Don't query the same nodes

One good practice is to store nodes you use repeatedly. Every time you click anywhere on the page you query the same 8 items which is expensive. Query them once and store them

var tabLang = $("#website-header #tab-lang");


then you can use the variable tabLang in the events:

tabLang.hide("slide", { direction: "right" }, 300);


Querying IDs

IDs are supposed to be unique. It is a little faster (and shorter) to query the ID directly than to establish its ancestry so:

$("#website-header #tab-lang");


should be:

$("#tab-lang");


Repetition

Seeing as most of your functions close the tabs and toggle the clicked one you could create a function that closes all tabs and call it from within the click event. Making sure the proper tab gets toggled instead of closed could be accomplished by checking the tabs state before it gets hidden and hide or show accordingly:

tabLang.click(function ( event ) {
    if (tabLang.is('visible')) {
        hideTabs();
    } else {
        hideTabs();
        // we want the tab to be visible so we stop the running animation
        tabLang.stop(true, true);
        // and tell it to show itself
        tabLang.show("slide", { direction: "right" }, 300)
    }
});


You would obviously need to repeat this for every clickable tab.

Further improvements

Seeing as you already have a click event on the document you could move the logic for the tab clicks into that event and discriminate between the tabs by checking event.target. Keep in mind though that event.target is not a jQuery object. You could check equality like this:

$(event.target).is(tabLang) // returns Boolean


With this in mind you could also reduce the size of each click event if you handled the hide / show exception in the hideTabs() function, by calling it with the clicked element as an argument and do some comparisons there. (Though that won't improve performance, just file size which should not be necessary for you purposes)

Code Snippets

var tabLang = $("#website-header #tab-lang");
tabLang.hide("slide", { direction: "right" }, 300);
$("#website-header #tab-lang");
$("#tab-lang");
tabLang.click(function ( event ) {
    if (tabLang.is('visible')) {
        hideTabs();
    } else {
        hideTabs();
        // we want the tab to be visible so we stop the running animation
        tabLang.stop(true, true);
        // and tell it to show itself
        tabLang.show("slide", { direction: "right" }, 300)
    }
});

Context

StackExchange Code Review Q#154955, answer score: 3

Revisions (0)

No revisions yet.