Is this the best way to do this? [closed]

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance. Closed 10 years ago.

I have a navigation bar that has two dropdowns (as nested ul's). I'm trying to toggle the subnavs when its parent is clicked. Or hide one subnav when the other parent is clicked. Here's the markup:

<div id="nav-bar">
  <a href="#">A link</a> |
  <ul>
    <li id="feedback">
      <a href="javascript:void(0);">Feedback</a>
      <ul class="subnav">
        <li><a href="#">Give us feedback</a></li>
       </ul>
     </li>
  </ul> |
  <a href="#">Another link</a> |
  <ul>
    <li id="location">
      <a href="javascript:void(0);">Pick your location</a>
      <ul class="subnav">
        <li&开发者_StackOverflow社区gt;<a href="#">Los Angeles</a></li>
        <li><a href="#">New York</a></li>
       </ul>
     </li>
  </ul>
</div>

And the code:

//hide the subnavs and give them a little down arrow
$('ul.subnav').hide().parent().append('<small>&#9660;</small>');

// show its subnav when clicked
$('#nav-bar ul li').click(function() {
  var subnav = $(this).children('ul.subnav');
  // hide the other's subnav if it's visible
  if ($(this).attr('id') == 'location') {
    subnav.toggle();
    $('li#feedback').children('ul.subnav').hide();
  } else {
    subnav.toggle();
    $('li#location').children('ul.subnav').hide();
  }
});

Still a novice to JS and jQuery, I'd like to know if there's a less verbose way to accomplish what I'm trying to do above.

Edit: A better question is, is there a way to do this without having to explicitly give the li's an id?

---

A better format would be something like this for your HTML:

<ul id="nav-bar">
  <li><a href="#">A link</a> |</li>
  <li>
    <a href="#">Feedback</a> |
    <ul class="subnav">
      <li><a href="#">Give us feedback</a></li>
     </ul>
  </li>
  <li><a href="#">Another link</a> |</li>
  <li>
    <a href="#">Pick your location</a> |
    <ul class="subnav">
      <li><a href="#">Los Angeles</a></li>
      <li><a href="#">New York</a></li>
     </ul>
   </li>
</ul>

Then your jQuery code would look like this:

$(function(){
  var $nav    = $("#nav-bar"),
      $subnav = $nav.find("ul.subnav").hide();

  $nav.children('li:has(ul.subnav)').click(function(e){
    e.preventDefault();
    var $sub = $(this).children('ul.subnav').toggle();
    $subnav.not($sub).hide();
  })
})

---

Well, there are a few ways that you could make it a little bit easier to maintain your code. Here's a quick example:

//hide the subnavs and give them a little down arrow
$('ul.subnav').hide().parent().append('<small>&#9660;</small>');

// show its subnav when clicked
$('#nav-bar ul li').click(function() {
  var subnav = $(this).children('ul.subnav');
  // hide the other's subnav if it's visible

 $('ul.subnav').hide();
 subnav.toggle();
});

Basically, this will re-hide all of the 'subnav' elements again, and then show the one that was clicked on. But the main difference here is that you can now add a third, fourth, firth, etc dropdown menu, or change the IDs around without having to change the javascript.

The HTML that you're using looks pretty good, and relatively similar to "standard" navigation menus.

Edit: Which is apparently similar to the previous answer :P

---

I'd get rid of the inline javascript:void(0) code it's unecessary. Have your link link to "#".

You definitely don't want to be matching ID's like that, it doesn't scale very well. Instead, something like this should work (untested):

$('#nav-bar ul li').click(function() {      
  $(this)
    //hide all subnavs
    .closest("#nav-bar").find("ul.subnav").hide().end().end()
    //show subnav under the li clicked
    .find("ul.subnav").show();    
});

Whenever possible, you want to chain your jquery code so that you don't have to grab a new element each time from the dom. From what I understand, it's more efficient to chain as it traverses the already found element, rather than fetching a new one.

the .end() reverts your chain back to the previous found element, so when I say .closest("blah").find("blah2") I'm going two levels deep. .end().end() will bring me back to the original item (this) so that I can then find the specific subnav underneath it.