4
\$\begingroup\$

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.stopPropagation();
    });
    $("#website-header #nav-apps").click(function(event) {
        event.stopPropagation();
        $("#website-header #tab-lang").hide("slide", { direction: "right" }, 300);
        $("#website-header #tab-apps").toggle("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);
    });
    $("#website-header #apps-toggle").click( function() {
        $("#website-header #tab-apps #apps-reveal").slideToggle(300);
    } );

    // dashboard tab
    $("#website-header #tab-dashboard").hide().click(function(event) {
        event.stopPropagation();
    });
    $("#website-header #nav-dashboard").click(function(event) {
        event.stopPropagation();
        $("#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").toggle("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);
    });

    // login tab
    $("#website-header #tab-login").hide().click(function(event) {
        event.stopPropagation();
    });
    $("#website-header #nav-login").click(function(event) {
        event.stopPropagation();
        $("#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").toggle("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);
    });

    // about tab
    $("#website-footer #tab-about").hide().click(function(event) {
        event.stopPropagation();
    });
    $("#website-footer #nav-about").click(function(event) {
        event.stopPropagation();
        $("#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").toggle("slide", { direction: "down" }, 300);
        $("#website-footer #tab-about #credits-reveal").delay(300).hide(0);
        $("#website-footer #tab-contact").hide("slide", { direction: "down" }, 300);
    });
    $("#website-footer #credits-toggle").click( function() {
        $("#website-footer #tab-about #credits-reveal").slideToggle(300);
    } );

    // contact tab
    $("#website-footer #tab-contact").hide().click(function(event) {
        event.stopPropagation();
    });
    $("#website-footer #nav-contact").click(function(event) {
        event.stopPropagation();
        $("#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").toggle("slide", { direction: "down" }, 300);
        $("#website-footer #tab-contact #contact-callback").hide().css('visibility','visible');
        $("#website-footer #tab-contact #callback-message").hide().css('visibility','visible');
        $("#website-footer #tab-contact #callback-dismiss").hide().css('visibility','visible');
    });

})

Basically I'm repeating the same thing, when a user clicks on a navbar item, I close every other menus and open the one the user requested (and reset the sub-menus if there were any). When the user clicks on the documents (or anywhere that is outside the menu they opened) I also close everything.

I'm afraid my code, even though it works fine, might be a bit repetitive and somewhat ressources' greedy (does the $(document).click(function(event) trigger even when there is nothing to close?).

I'm looking for some recommendations, ideas, or models from more experienced devs than I am.

\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Vogel612
    Commented Feb 10, 2017 at 21:07
  • \$\begingroup\$ Ah I understand. I missed that rule and assumed it was okay to post the result and ask for a confirmation of the code as my post did not attract many reviewers. \$\endgroup\$
    – QiMath
    Commented Feb 10, 2017 at 21:13

1 Answer 1

Reset to default
3
\$\begingroup\$

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)

\$\endgroup\$
2
  • \$\begingroup\$ Thanks a lot for your answer all theses suggestions ! I went ahead and applied them on my code (I've updated my post if you want to review the result). I was not entirely sure to fully understand the last paragraph so I tried a few things to handle the way tabs close if the user clicks outside of them. Anyways thank you ! It was very helpful. \$\endgroup\$
    – QiMath
    Commented Feb 10, 2017 at 21:05
  • \$\begingroup\$ I didn't see posting the result code was not allowed. No problem, and thanks ! \$\endgroup\$
    – QiMath
    Commented Feb 10, 2017 at 21:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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