Reputation:
I am setting up a page to show/hide content by clicking on a menu item, using jquery. In each case I also hide the other divs. The following code seems sensible to me, but I guess I'm missing something because it works inconsistently. Sometimes clicking on a menu item works as expected and other times it does not. Something to do with hiding divs even when they are hidden?
$(document).ready(function() {
$('#commercial-menu-item').click(function() {
$('#other').toggle();
$('#intuito').hide();
$('#pro-bono').hide();
$('#all').hide();
});
$('#other-menu-item').click(function() {
$('#other').toggle();
$('#commercial').hide();
$('#pro-bono').hide();
$('#all').hide();
});
$('#pro-bono-menu-item').click(function() {
$('#pro-bono').toggle();
$('#other').hide();
$('#commercial').hide();
$('#all').hide();
});
$('#all-menu-item').click(function() {
$('#all').toggle();
$('#other').hide();
$('#pro-bono').hide();
$('#commercial').hide();
});
});
This is the first real thing I've done with jquery, so it probably shows...
Upvotes: 3
Views: 2674
Reputation: 93631
You are much better off writing a single click handler for all the buttons (with a common class on them) and driving their related items from data-... attributes on the options.
Then the logic becomes "collapse everything except the selected item and toggle the selected item"
A menu option would look like:
<a id="pro-bono-menu-item" class="menu" data-item="#pro-bono">Pro bono</a>
Where data-item is simply a selector for the related div etc. The id's on the menu items are also no longer needed.
An example of the code would look like:
$(document).ready(function () {
$('.menu').click(function () {
var $clicked = $(this)
$('.menu').each(function () {
var $menu = $(this);
if (!$menu.is($clicked)) {
$($menu.attr('data-item')).hide();
}
});
$($clicked.attr('data-item')).toggle();
});
});
This will give you more flexibility. You can easily add new options without changing the code of every button.
An added bonus is that you can style the clicked/unclicked items (e.g. by toggling a style on it), again with no duplication of code as everything goes through one function.
Upvotes: 3
Reputation: 1949
Its a good start but I would probably use $('div').toggleClass('show');
Then you can write your css like
div {
display:none;
}
div.show {
display:block;
}
i'll try to write a jsfiddle for your specific solution, but give me a bit! It should clear it up alot! Let me know if you have another question or need anything else :)
EDIT
Here is a solution of a neater way to implement what I think you have :)
$('ul li').each(function(){
$(this).click(function(){
$('div.show').toggleClass('show');
target = '#'+$(this).data('target');
$(target).toggleClass('show');
});
});
Upvotes: -1