jQuery buttons to show/hide info and search

Question

I've used multiple tutorials to get the exact look for show/hide using jQuery, but now that I've done it's too repetitive, I've tried to edit it using variables, but it didn't work. Is there a way to avoid repetitiveness here?

$(document).ready(function() {
 $(document).click(function(event) {
 if ($(event.target).is(".info-icon") || $(event.target).is(".info-tab") || ($(".info-tab").has(event.target).length == 1)) {
 if ($(".info-tab").height() != 0 && !$(event.target).is(".info-tab") && !($(".info-tab").has(event.target).length == 1)) {
 $(".info-tab").css("max-height", "0");
 } else {
 if ($(".search-tab").height() != 0) {
 $(".search-tab").css("max-height", "0");
 setTimeout(function() {
 $(".info-tab").css("max-height", "200px");
 }, 300);
 } else {
 $(".info-tab").css("max-height", "200px");
 }
 }
 } else if ($(event.target).is(".search-icon") || $(event.target).is(".search-tab") || ($(".search-tab").has(event.target).length == 1)) {
 if ($(".search-tab").height() != 0 && !$(event.target).is(".search-tab") && !($(".search-tab").has(event.target).length == 1)) {
 $(".search-tab").css("max-height", "0");
 } else {
 if ($(".info-tab").height() != 0) {
 $(".info-tab").css("max-height", "0");
 setTimeout(function() {
 $(".search-tab").css("max-height", "200px");
 }, 300);
 } else {
 $(".search-tab").css("max-height", "200px");
 }
 }
 } else {
 $(".info-tab").css("max-height", "0");
 $(".search-tab").css("max-height", "0");
 }
 });
});

JSFiddle for what it does is here: https://jsfiddle.net/msms92/L9L0tqvp/

Answer 1

The first thing that comes to my mind is that what you're trying to do is handling all of the click events in just one handler function! Don't!

Use delegate event listeners instead:

$(document)
 .on("click", ".info-icon, .info-tab", function(e) {
 e.stopPropagation();
 // Deal with .info-icon or .info-tab
 })
 .on("click", ".search-icon, .search-tab", function(e) {
 e.stopPropagation();
 // Deal with .search-icon or .search-tab
 })
 .on("click", function(e) {
 // Other clicks
 });

Secondarily, define CSS classes list .info-tab-open and .search-tab-open like these:

.info-tab-open {
 max-height: 200px;
}

(And remember: using magic numbers is bad, especially when styling with JavaScript.)

So you'll end up doing this instead:

var $infoTab = $(".info-tab");
if ($infoTab.hasClass("info-tab-open")) {
 $infoTab.removeClass("info-tab-open");
} else {
 var $searchTab = $(".search-tab");
 if ($searchTab.hasClass("search-tab-open")) {
 $searchTab.removeClass("search-tab-open");
 setTimeout(..., 300);
 } else {
 $infoTab.addClass("info-tab-open");
 }
}

Third: since the handler's code is basically the same and repeating code is a code smell, you'll probably be better off if you use a factory function for the handler:

function toggler(selectedClass, otherClass) {
 return function(e) {
 e.stopPropagation();
 var $selectedTab = $("." + selectedClass);
 if ($selectedTab.hasClass(selectedClass + "-open")) {
 $selectedTab.removeClass(selectedClass + "-open");
 } else {
 var $otherTab = $("." + otherClass);
 if ($otherTab.hasClass(otherClass + "-open")) {
 $otherTab.removeClass(otherClass + "-open");
 setTimeout(..., 300);
 } else {
 $selectedTab.addClass(selectedClass + "-open");
 }
 }
 };
}
$(document)
 .on("click", ".info-icon, .info-tab", toggler("info-tab", "search-tab"))
 .on("click", ".search-icon, .search-tab", toggler("search-tab", "info-tab"))
 .on("click", function() {
 $(".info-tab").removeClass("info-tab-open");
 $(".search-tab").removeClass("search-tab-open");
 });

Answer 2

WTF?

Why not just select the block you want with $("CSS _Selector") and use the .show() or .hide() method.

• MSmS: Because I don't want to. It's also not gonna help me avoid repeating the code for each button/tab.
• Adrian Brand: Take at look at my updated fiddle
• Adrian Brand: jsfiddle.net/L9L0tqvp/1
• MSmS: Okay, it's not exactly what I need, it doesn't hide the other "data-target" when clicked on the second one, but I might get it to work.
• Adrian Brand: Just add a $(".info-wrapper").hide() before the toggle.

Answer 3

Don't use the .css method. Create CSS classes and use the class rules to update the display with .addClass and .removeClass, or .toggle

• MSmS: I tried, .css part isn't the problem. It's the same code for each button/tab, I want to get rid off.
• MaxArt: @MSmS It is part of the problem. Your code looks messy to begin with. If you embrace JavaScript's and jQuery's best practices, the solution will come naturally.