How to remove all classes from a menu except from the actual active element?

I have a menu which opens a submenu onclick by adding a class active to the corresponding element. However, whenever the first submenu was opened it stayed active if a second submenu was opened and so on.

Therefore I added a forEach to first remove all active classes and only after that add the active class to the next submenu.

Unfortunately, this causes the problem, I can’t wrap my head around: I need the active menu to close if the link is clicked again.

The following code does not work in this case, because when clicking the same link it will first remove all active classes and then add it again which causes the menu to stay open instead of closing.

const megamenu = document.querySelector('.megamenu');
const menuSection = megamenu.querySelector('.megamenu-section');  
const submenus = document.querySelectorAll('.megamenu-submenu');

menuSection.addEventListener('click', (e) => {
    e.preventDefault();
    submenus.forEach(submenu => {
        if (submenu.classList.contains('active')) {
            submenu.classList.remove('active');
            console.log("hasActive")
    })
    e.target.closest('.megamenu-submenu').classList.toggle('active');
});

I think I need a way to remove all active classes except for the actual active submenu.

Any way to achieve this? Or any better solution? Thanks.

Answer

The multiple approaches are for demonstration that there are always multiple ways to solve a single problem in programming.
Some might be more efficient, others might be more readable, etc.

You may chose whichever approach you prefer.

Approach 1 (recommended by me)

Save reference of last active submenu as lastActive.
onclick, remove .active from lastActive, and toggle .active on clicked submenu.

To fix the case where lastActive is the clicked submenu, we toggle depending on if .active was present or not before removal on lastActive.

This requires no looping, and almost no branching (if-statements) from our part (but most likely uses some in the native code (may use some anyway)).
But I think it creates a single closure (no problem).

So that lastActive is not visible in the global context or the rest of the script, we encapsulate it inside an IIFE.

const megamenu = document.querySelector('.megamenu');
const menuSection = megamenu.querySelector('.megamenu-section');  
const submenus = document.querySelectorAll('.megamenu-submenu');


(function() {
  let lastActive = submenus[0];
  menuSection.addEventListener('click', evt => {
    const currentSubmenu = evt.target.closest('.megamenu-submenu');
    if (!currentSubmenu) return;
    
    const wasActive = currentSubmenu.classList.contains('active');
    
    lastActive.classList.remove('active');
    currentSubmenu.classList.toggle('active', !wasActive);
    
    lastActive = currentSubmenu;
  });
})();

.megamenu-submenu {
  border: 1px solid black;
  height: 1.6rem;
  box-sizing: border-box;
}
.megamenu-submenu.active {
  background-color: rgba(255, 0, 0, .3);
}

<div class="megamenu">
  <div class="megamenu-section">
    <div class="megamenu-submenu"></div>
    <div class="megamenu-submenu"></div>
    <div class="megamenu-submenu"></div>
    <div class="megamenu-submenu"></div>
  </div>
</div>

Approach 2

Loop through each active submenu and remove .active, except for the currently clicked one. Toggle .active of the clicked one.

To only loop through the currently active submenus, we need to query inside the listener. This may hit the performance (minimally) for excessively many elements.

const megamenu = document.querySelector('.megamenu');
const menuSection = megamenu.querySelector('.megamenu-section');  
const submenus = document.querySelectorAll('.megamenu-submenu');

menuSection.addEventListener('click', evt => {
  const currentSubmenu = evt.target.closest('.megamenu-submenu');
  if (!currentSubmenu) return;
  
  for (const submenu of document.querySelectorAll('.megamenu-submenu.active')) {
    if (submenu !== currentSubmenu)
      submenu.classList.remove('active');
  }
  currentSubmenu.classList.toggle('active');
});

.megamenu-submenu {
  border: 1px solid black;
  height: 1.6rem;
  box-sizing: border-box;
}
.megamenu-submenu.active {
  background-color: rgba(255, 0, 0, .3);
}

<div class="megamenu">
  <div class="megamenu-section">
    <div class="megamenu-submenu"></div>
    <div class="megamenu-submenu"></div>
    <div class="megamenu-submenu"></div>
    <div class="megamenu-submenu"></div>
  </div>
</div>

Approach 3

Similar to approach 2, but loop through the already existing NodeList submenus.

This may hit the performance (minimally) for excessively many submenus, but should generally perform better than approach 2.

const megamenu = document.querySelector('.megamenu');
const menuSection = megamenu.querySelector('.megamenu-section');  
const submenus = document.querySelectorAll('.megamenu-submenu');

menuSection.addEventListener('click', evt => {
  const currentSubmenu = evt.target.closest('.megamenu-submenu');
  if (!currentSubmenu) return;
  
  for (const submenu of submenus) { // Only changed line
    if (submenu !== currentSubmenu)
      submenu.classList.remove('active');
  }
  currentSubmenu.classList.toggle('active');
});

.megamenu-submenu {
  border: 1px solid black;
  height: 1.6rem;
  box-sizing: border-box;
}
.megamenu-submenu.active {
  background-color: rgba(255, 0, 0, .3);
}

<div class="megamenu">
  <div class="megamenu-section">
    <div class="megamenu-submenu"></div>
    <div class="megamenu-submenu"></div>
    <div class="megamenu-submenu"></div>
    <div class="megamenu-submenu"></div>
  </div>
</div>