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.
-
\$\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– Vogel6122017年02月10日 21:07:57 +00:00Commented 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– QiMath2017年02月10日 21:13:24 +00:00Commented Feb 10, 2017 at 21:13
1 Answer 1
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)
-
\$\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– QiMath2017年02月10日 21:05:31 +00:00Commented Feb 10, 2017 at 21:05
-
\$\begingroup\$ I didn't see posting the result code was not allowed. No problem, and thanks ! \$\endgroup\$QiMath– QiMath2017年02月10日 21:14:22 +00:00Commented Feb 10, 2017 at 21:14