How to hide a menu when clicked on any menu-item?

Question

I can not find where is the problem. Any idea about hiding on clicking any menu-item?

var button = document.getElementById("#1");
var menu = document.getElementById("#menu");
button.addEventListener('click', function(event) {
  if (menu.style.display == "block") {
    menu.style.display = "none";
  } else {
    menu.style.display == "block";
  }
});

<ul id="menu">
  <li><a href="#home" id="1" class="active">Home</a></li>
  <li><a href="#model-s" id="1" class="one">Model S</a></li>
  <li><a href="#model3" id="1">Model 3</a></li>
  <li><a href="#modelx" id="1">Model X</a></li>
  <li><a href="#modely" id="1">Model Y</a></li>
</ul>

This error means that "button" doesn't exist. That's because you incorrectly used getElementById. Check [documentation](https://developer.mozilla.org/en-US/docs/Web/API/Document/getElementById) to see how it was applied correctly. Apart from that there shouldn't be more than one identical id. — DGX37, May 07 '22 at 18:35
FYI there are a ton of [other reasons](/q/14028959) why getElementById might fail. Most important is to run it only after the DOM exists, meaning classically running the js — cachius, May 07 '22 at 19:28

Nick Vu · Accepted Answer · 2022-05-07T18:40:38.870

You cannot get document.getElementById("#1") with #. getElementById is already an id selector, so you don't need to have #.

menu.style.display, you don't have inline styles for menu, your condition won't pass for the first time.

You also cannot have multiple id in your elements because id should be unique. In that case, you should use class instead (I added menu-item classes for element selectors)

I've tried to change your code with some comments

//get all menu items
var menuItems = document.querySelectorAll(".menu-item");
var menu = document.getElementById("menu");
for (const menuItem of menuItems) {
  //add click events to menu items
  menuItem.addEventListener('click', function(event) {
    //hide menu if click on menu item
    menu.style.display = "none";
  });
}

<ul id="menu">
  <li><a href="#home" class="active menu-item">Home</a></li>
  <li><a href="#model-s" class="one menu-item">Model S</a></li>
  <li><a href="#model3" class="menu-item">Model 3</a></li>
  <li><a href="#modelx" class="menu-item">Model X</a></li>
  <li><a href="#modely" class="menu-item">Model Y</a></li>
</ul>

How to hide a menu when clicked on any menu-item?

1 Answers1