I’m looking to build a toggle menu using jQuery. I have the toggle feature working but for some reason my if statement isn’t working. Basically I need to check to see if the list item has any children (sub menu items) and if so then toggle them open/closed on click preventing the default click-through action, and if not then allow the default click-through action.
$(document).ready(function() {
// Hide the sub menu items first
$('#menu > li > ul').hide();
// Add the toggle & prevent default action only if there are children
if ( $('#menu > li').has('ul').length ) {
$('#menu > li').toggle(function() {
$(this).find('ul').slideDown();
}, function() {
$(this).find('ul').slideUp();
});
return false; // Prevent default action
}
});
The toggle currently works but the click-through doesn’t. By this I mean if you click the menu item which doesn’t have any children it should go to test.html but doesn’t. Also clicking on each of the children doesn’t send through to the relevant page (hashtag).
Any help or suggestions on how best to achieve it are very welcome.
Right now you are adding the toggle behaviour on to all list items.
$('#menu > li').toggle(function() {
What it seems that you need, is to filter those before you add the toggle event to the remaining ones. You can do that by removing the if statement, and moving the has() method to just before the toggle
$('#menu > li').has('ul').toggle(function() {
Currently they are “#” when I suspect they should be something else, such as “#contactus”
Thanks. I knew my if statement was causing some problems but I wasn’t quite sure how to filter on it. Your example solved the click through issues but the only issue that remains is that if I click on one of the sub menu items instead of taking me to that page it now just re-hides the that sub menu.
How can I elaborate on it so that instead of re-hiding that sub navigation it follows the link instead? Another check of some kind for child items?
Yeah sure, I’ve changed the links now to point to pages and I can see that these are now working on the top level items because it follows through (even though those pages don’t exist I am still taken to that page).
I see. Would it be better to target the link specifically then? Also, would it be better to use toggle() instead of the individual slideUp() and slideDown() methods?
I’ve uploaded the changes to show you the effect it has. Child links are now followed but the slide goes back up right after clicking and before transferring to the new page. Also if you click on one of the parents twice (once to open it, once when it is open) it follows that link too.
I’m open to suggestions on how to alter it to produce a better toggle menu/accordion (not quite an accordion because multiple items are allowed to be open at the same time).
It’s more the learning than the time involved at the moment. I’d rather learn how to do it myself and understand why things work or don’t work than use something off-the-shelf. I wouldn’t have thought it would be too hard to get working but when you get down to the nitty gritty it’s the little things you didn’t count on which cause the problems.
So the extra check is to add the toggle to the parent if it has no children and if not then follow the link. It seems easy when you see how it’s done.
Just to check I’m following correctly, (evt.target) is the ‘li a’, the parent is the ‘li’ and we’re checking to see if there is a ‘ul’ in that ‘li’?
So when using .length I need to make sure I also give it a conditional such as > 0 to get it to return true or false.
function(evt) is necessary because of event delegation, so the event handler isn’t inherited?
window.location = evt.target; gives the browser back it’s default link action?
Out of interest, if I had multiple nested items would this work on sub-sub-menus too? I used the child selectors for that reason of flexibility and the rest of the solution looks like it would work but I’ve not yet tried.
It’s not mandetory, as 0 will resolve to the same result as false, but it makes it easier to understand the code.
if (someFunc(stuff)) {
// huh?
}
if (someFunc(stuff) > 0) {
// that's better
}
On IE events are a global object, whereas on most other web browsers the event info is passed to the first parameter of the function handler. So no, it’s not inherited.
Not exactly. Your return false does nothing to prevent the default action, so you can remove that.
The click event on the link bubbles up through the elements, from a to li to ul and so on upwards to the body tag. The event is captured by jQuery at the li where your toggle event is, and the jQuery’s toggle method automatically prevents the default action from occurring.
From: the .toggle() documentation page
The implementation also calls .preventDefault() on the event, so links will not be followed and buttons will not be clicked if .toggle() has been called on the element.
Because the links can no longer be automatically followed, we need to simulate that instead by using window.location
Yes it should work on multiply nested items.
Something that I noticed though is that the user has no way to tell if a link will open a submenu, or if it will navigate to another page. You may want to consider how to clear up that confusion for users of the navigation.
On IE events are a global object, whereas on most other web browsers the event info is passed to the first parameter of the function handler. So no, it’s not inherited.
So giving it a name such as function(evt) just helps to identify it in the code making it easier to read?
Not exactly. Your return false does nothing to prevent the default action, so you can remove that.
Interesting to know. I wouldn’t have known toggle() added this behaviour by default without reading through the API docs (or you pointing it out :). I’ve removed the return false now and it still works as expected.
Something that I noticed though is that the user has no way to tell if a link will open a submenu, or if it will navigate to another page. You may want to consider how to clear up that confusion for users of the navigation.
True. That’s another usability enhancement that can be added to improve the menu. For the purpose it’s not a problem at the moment but it could be something I add later on. I imagine the best way would be to add a class to each list item depending on whether they have any nested ul’s? Then style that class by providing an arrow or such to indicate it contains sub content?
Thanks again, all good to know for future reference.
It doesn’t just help, it’s mandetory. That’s the W3C way of handling events that occurs on web browsers that are not IE.
It’s commonly done by using CSS to set the background image to the indicator. If you want to get fancy, you can script changes to the background image when you toggle things.
