slideToggle opens up element, but doesn't close it.

Question

Hey guys,

I'm making something really generic - just a collapsible menu list, actually i'm trying to emulate the wordpress admin menus. For some reason beyond my understanding, I manage to get slideToggle to slide open the menus, but they don't close back when I click on the toggle anchor again. This is the jquery:

   $(document).ready(function() {   
  $('.menu-collapse').click(function(e){
   e.preventDefault();
   $(this).next('.admin-submenu').slideToggle();
  }); 
   });

This is the markup

<ul>
 <li class="top"><a href="" class="menu-item">Dashboard</a></a></li>
 <li class="top"><a href="" class="menu-item">Products</a><a href="" class="menu-collapse">
  <div class="admin-submenu">
   <ul>
    <li class="sub"><a href="">My Products</a></li>
    <li class="sub"><a href="">Add New</a></li>
   </ul>
  </div>
 </li>
 <li class="top"><a href="" class="menu-item">Lists</a><a href="" class="menu-collapse">
  <div class="admin-submenu">
   <ul>       
    <li class="sub"><a href="">Brands</a></li>
    <li class="sub"><a href="">Colors</a></li>       
    <li class="sub"><a href="">Locations</a></li>
    <li class="sub"><a href="">Manufacturers</a></li>
   </ul>
  </div>
 </li>
</ul>

Can anyone help please ?

Answer 1

Your HTML isn't valid. You need to make sure that for each opening <a> tag you have exactly one matching closing tag: </a>

Answer 2

Your syntax is broken. Your a.menu-collapse tags just open, but don't close.

As sidenotes, do you need the div.admin-submenu? Or can you just give the admin-submenu class to the nested ul? Also, the a inside the nested li is superfluous. You shouldn't need it either for appearance or Javascript.

Answer 3

As buh buh noted, your markup has several errors. You have closing </a> tags with no matching opener, and opening <a> tags with no matching closer. You could also simplify it quite a bit. You don't need the <div>s, as the <ul> tags will serve perfectly well as containers for their list items. Try using this as your markup:

<ul>
    <li class="top"><a href="" class="menu-item">Dashboard</a></li>
    <li class="top"><a href="" class="menu-item">Products</a>
        <ul class="admin-submenu">
            <li class="sub"><a href="">My Products</a></li>
            <li class="sub"><a href="">Add New</a></li>
        </ul>
    </li>
    <li class="top"><a href="" class="menu-item">Lists</a>
        <ul class="admin-submenu">    
            <li class="sub"><a href="">Brands</a></li>
            <li class="sub"><a href="">Colors</a></li>                          
            <li class="sub"><a href="">Locations</a></li>
            <li class="sub"><a href="">Manufacturers</a></li>
        </ul>
    </li>
</ul>

And then in your jQuery, change the .menu-collapse selector to .menu-item.

Here's a working demo: http://jsfiddle.net/Ender/zUf9W/