How to add css-class active for menu items by using just javascript code?

Question

I tried to add "active class" which will change the color of the navigation item (displayed through li tags) when user clicks on it. To do this, I make a function to remove active class if there is any in all li elements. After that, when there is a click on navi item, I will add the active class to that element. The problem is that when running my code, instead of just one item has "active" class, all items have. I found many solutions for this problem, but most of them use jQuery which I have no knowledge about the library. I hope someone can point my code errors below. Thank you!

// Find all li tags
const liTags = document.querySelectorAll('li');

// Function to remove the current element has active class
function RemoveActive() {
    for (let i = 0; i < liTags.length; i++) {
        const currentActiveClass = document.querySelector('.active');
        // Remove active class in the current li element
        if (currentActiveClass != null) {
            liTags[i].classList.remove('active');
        }
    }   
}

// Add the active class to the clicked item
for (let i = 0; i < liTags.length; i++) {
    liTags[i].addEventListener('click', function() {
        RemoveActive;
        liTags[i].classList.add('active');
    })
}

Answer 1

From the above direct comment on the OP's question ...

"Regardless of whatever caused the malfunction rethink the entire approach. Right now every LI element available downwards the document level at query time features its own click handling. And even though one can do that (instead of event delegation) a much bigger question arises. Is there just one un/ordered list in the entire document? If not, be aware that the current approach will work across the active states of different lists ... means, any item click from within a random list removes the active class name (if it exists) of any other list item from other lists as well."

The next following example is for demonstration purposes only in order to show what can be achieved with an event delegation based approach ...

function getNestedListRoot(listNode) {
  let elmNode = listNode;
  while (listNode = listNode?.parentNode.closest('ol, ul')) {
    elmNode = listNode;
  }
  return elmNode;
}

function handleNestedListItemsActiveState({ target }) {
  // the LI element closest to the `click` source.
  const srcLiElm = target.closest('li');

  // guard
  if (srcLiElm === null) {
    return;
  }
  // the LI element's un/ordered list parent.
  const listNode = srcLiElm.parentNode;
  // the top most un/ordered list node
  // of a nested list structure.
  const listRoot = getNestedListRoot(listNode);

  listRoot
    .querySelectorAll('li')
    .forEach(elmNode =>
      // de'active'ate every LI element
      // within the nested list structure.
      elmNode.classList.remove('active')
    );
  let liElm = srcLiElm;

  while (liElm) {
    // follow the path of directly nested
    // LI elements from the current inner to the
    // most outer LI element and 'active'ate each.    
    liElm.classList.add('active');

    liElm = liElm.parentNode.closest('li');
  }
}

function initialize() {
  document
    .querySelectorAll('ol, ul')
    .forEach(elmNode =>
      elmNode.addEventListener('click', handleNestedListItemsActiveState)
    )
}
initialize();

body { margin: -2px 0 0 0; }
ol, ul { margin: 0 0 10px 0; }
ol ul, ul ol, ol ol, ul ul { margin: 0; }
li { margin: 0 0 0 -20px; font-size: 12px; }
li.active,
li.active li.active { color: #fc0; }
li.active li { color: initial; }

<ol>
  <li class="active">
    OL_A-a
    <ul>
      <li>
        OL_A-a__UL_A-a
      </li>
      <li class="active">
        OL_A-a__UL_A-b
        <ol>
          <li>
            OL_A-a__UL_A-b__OL_B-a
          </li>
          <li>
            OL_A-a__UL_A-b__OL_B-b
          </li>
          <li class="active">
            OL_A-a__UL_A-b__OL_B-c
          </li>
        </ol>
      </li>
      <li>
        OL_A-a__UL_A-c
      </li>
    </ul>
  </li>
  <li>
    OL_A-b
  </li>
  <li>
    OL_A-c
    <ul>
      <li>
        OL_A-c__UL_B-a
      </li>
      <li>
        OL_A-c__UL_B-b
      </li>
      <li>
        OL_A-c__UL_B-c
      </li>
    </ul>
  </li>
</ol>

<ol>
  <li>
    OL_A-a
    <ul>
      <li>
        OL_A-a__UL_A-a
      </li>
      <li>
        OL_A-a__UL_A-b
        <ol>
          <li>
            OL_A-a__UL_A-b__OL_B-a
          </li>
          <li>
            OL_A-a__UL_A-b__OL_B-b
          </li>
          <li>
            OL_A-a__UL_A-b__OL_B-c
          </li>
        </ol>
      </li>
      <li>
        OL_A-a__UL_A-c
      </li>
    </ul>
  </li>
  <li>
    OL_A-b
  </li>
  <li class="active">
    OL_A-c
    <ul>
      <li class="active">
        OL_A-c__UL_B-a
      </li>
      <li>
        OL_A-c__UL_B-b
      </li>
      <li>
        OL_A-c__UL_B-c
      </li>
    </ul>
  </li>
</ol>

Answer 2

you should follow this practice for clean and less code

var root = document.querySelector(".root")

root.addEventListener("click",e=>{
  var t = e.target,
      li = t.closest("li")
  if(li){
    root.querySelectorAll("li").forEach(each=>each.classList.remove("active")) 
    li.classList.add("active")
  }
})

.active {
  background:blue;
  color:white;
}

.root li {
  padding:2px;
  cursor:pointer;
} 

<ul class="root">
  <li class="active">items</li>
  <li>items</li>
  <li>items</li>
  <li>items</li>
  <li>items</li>
  <li>items</li>
  <li>items</li>
</ul>

Answer 3

I believe your error was RemoveActive; where it should have been RemoveActive(), but thought I'd take the time to refactor the code.

I would advise using camel case for the function names removeActive() and name them a more descriptive name than "RemoveActive" as it will make it easier for future development / to understand what this function does as the program grows.

const navigationItems = document.querySelectorAll('li');

function toggleActiveNavItem() {
  navigationItems.forEach(item => {
    item.addEventListener("click", function() {
      addClickEventToNavigation(item)
    }
  }
}

function addClickEventToNavigation(item) {
  // Remove active from every navigation item
  navigationItems.forEach(individualNavigationItem =>{
    // Other than the one passed to the function as having been clicked
    if (individualNavigationItem != item) {
      individualNavigationItem.classList.remove("active");
    }

    // If the clicked item does not have the active class, add it
    if (!item.classList.contains("active")) {
      individualNavigationItem.classList.add("active");
    }
  });
}

Answer 4

As you can see from my example, i add classList.contains instead of check element then you have a typo error () into function

// Find all li tags
const liTags = document.querySelectorAll('li');

function RemoveActive() {
    for (let i = 0; i < liTags.length; i++) {
        if (liTags[i].classList.contains('active')) {
            liTags[i].classList.remove('active');
        }
    }   
}

for (let i = 0; i < liTags.length; i++) {
    liTags[i].addEventListener('click', function() {
        RemoveActive();
        liTags[i].classList.add('active');
    })
}

.active{
  background-color:red;
}

<ul>
  <li>1</li>
  <li>2</li>
  <li>3</li>
</ul>

Instead of use remove and add you can use toggle like:

// Find all li tags
const liTags = document.querySelectorAll('li');

function RemoveActive() {
  const li = document.querySelector('li.active')
  if (li) {
    li.classList.toggle("active");
  }
}

for (let i = 0; i < liTags.length; i++) {
  liTags[i].addEventListener('click', function() {
    RemoveActive();
    liTags[i].classList.toggle('active');
  })
}

.active {
  background-color: red;
}

<ul>
  <li>1</li>
  <li>2</li>
  <li>3</li>
</ul>