Hamburger Menu Toggle-CodePudding

I am creating a landing page for a photography site, and I am having issues with the hamburger menu.

Desktop view - The navbar is working as it should. When I switch to mobile view, the hamburger menu only shows the first list item. I used HTML, CSS, and JS to create it.

It seems to only show the first list item. I'm sure I'm missing something simple, but can't spot it!

Any help would be really appreciated!

const hamburger =  document.getElementById('hamburger');
const navItem =  document.getElementById('nav-item');

hamburger.addEventListener('click', () => {
  navItem.classList.toggle('show');
});

.hamburger {
  cursor: pointer;
  padding-left: 1rem;
}

.nav-item {
  display: none;
}

.nav-item.show {
  display: flex;
  flex-direction: column;
  align-items: center;
}

ul {
  background-color: var(--Dark-blue);
  padding-top: 1rem;
  padding-bottom: 1rem;
}

@media (min-width: 750px) {
  .hamburger {
    display: none;
  }

  .nav-list {
    display: flex;
    justify-content: right;
  }

  .nav-item {
    display: block;
  }
}

<ul  id="nav-list">
        <img
          
          id="hamburger"
          src="icon-hamburger.svg"
          alt=""
          aria-hidden="true"
        />

        <li id="nav-item" ><a href="#">Home</a></li>
        <li id="nav-item" ><a href="#">Highlights</a></li>
        <li id="nav-item" ><a href="#">Portfolio</a></li>
        <li id="nav-item" ><a href="#">Bio</a></li>
        <li id="nav-item" ><a href="#">Blog</a></li>
        <li id="nav-item" ><a href="#">Contact</a></li>
</ul>

CodePudding user response：

When you use getElementById it will return you only the first match. IDs expected to be unique per usage, so it makes sense why you will only get the first li element in your code. Although I don't understand why you made your query selection with id while you also defined classes (which is used to affect at least one element in contrast to id), it is what you should have used.

By the way you should use classes for the same type of elements such as your li items instead of adding it to the ul tag as well. Event if it was for styling, the styling you add to your list elements would be added to your ul element as well.

So here is an example:

<ul  id="nav-list">
  <img id="hamburger" src="icon-hamburger.svg" alt="HAMBURGER" aria-hidden="true" />

  <li ><a href="#">Home</a></li>
  <li ><a href="#">Highlights</a></li>
  <li ><a href="#">Portfolio</a></li>
  <li ><a href="#">Bio</a></li>
  <li ><a href="#">Blog</a></li>
  <li ><a href="#">Contact</a></li>
</ul>

const hamburger =  document.getElementById('hamburger');
const navItems =  Array.from(document.querySelectorAll('.nav-item'));

 hamburger.addEventListener('click', () => {
   console.log(navItems); // you can see your nav items in console everytime you click the button
   navItems.forEach(navItem => navItem.classList.toggle('show'))
 });

CSS is the same.

Side note: George Sun's answer is what you should have done. My answer is more about how you should have used ids/classes and collected multiple elements.

CodePudding user response：

Each value for the id attribute should only be used once in a document. Therefore, your document.getElementById('nav-item') is only looking for the first element with id attribute nav-item and acting on that. To make all of your list items show, I've instead applied id="nav-item" to span that parents all li and removed the id from the list items. This will allow all list items to display when the menu icon is clicked and does not depend on the list items all having a correct class applied. I've also updated your CSS accordingly.

For the sake of illustration I've also replaced your image reference to the Stack Overflow logo as your code snippet referenced a local file.

const hamburger =  document.getElementById('hamburger');
const navItem =  document.getElementById('nav-item');

hamburger.addEventListener('click', () => {
  navItem.classList.toggle('show');
});

.hamburger {
  cursor: pointer;
  padding-left: 1rem;
}

#nav-item li { /* updated */
  display: none;
}

#nav-item.show li { /* updated */
  display: flex;
  flex-direction: column;
  align-items: center;
}

ul {
  background-color: var(--Dark-blue);
  padding-top: 1rem;
  padding-bottom: 1rem;
}

@media (min-width: 750px) {
  .hamburger {
    display: none;
  }

  .nav-list {
    display: flex;
    justify-content: right;
  }

  .nav-item {
    display: block;
  }
}

<ul  id="nav-list">
        <img
          
          id="hamburger"
          src="https://stackoverflow.design/assets/img/logos/so/logo-stackoverflow.svg"
          alt=""
          aria-hidden="true"
        />

    <span id="nav-item">
        <li ><a href="#">Home</a></li>
        <li ><a href="#">Highlights</a></li>
        <li ><a href="#">Portfolio</a></li>
        <li ><a href="#">Bio</a></li>
        <li ><a href="#">Blog</a></li>
        <li ><a href="#">Contact</a></li>
    </span>
</ul>