Add a simple class to this element when clicked on, and remove class from other elements

Question

I'm trying to make it so that when clicking on a li:

1. The li clicked on gets the class 'selected'
2. The neighbouring li siblings have the class 'selected' removed

The aim is, whenever clicking on a li, only that li gets a class of 'selected'.

I wrote a simple for of loop and tried adding an event listener to each li, but nothing happens when clicking on any li. Why does this happen, and how can it be fixed?

Also, out of curiosity, would using the const keyword be more applicable than var in this case?

Thanks for any help here - the code demos can be found below:

Codepen Demo URL: https://codepen.io/anon/pen/MMgWZY

var menuLis = document.querySelectorAll("#top-nav > li");

for (let li of menuLis) {
  
  li.addEventListener("click", function(){
    // 1. Remove Class from All Lis
    for (let li of menuLis) {
      li.classList.removeClass('selected');
      console.log('class removed');
    }
    
    // 2. Add Class to Relevant Li
    this.classList.addClass('selected');
    console.log('class added');
  });
  
}

<ul id='top-nav'>
  <li>Coffee</li>
  <li>Tea</li>
  <li>Milk</li>
</ul>

Answer 1

li.classList.remove is correct and for add class use add instead of addClass

you should change your code to:

    var menuLis = document.querySelectorAll("#top-nav > li");
    
    for (let li of menuLis) {
      
      li.addEventListener("click", function(){
        // 1. Remove Class from All Lis
        for (let li of menuLis) {
          li.classList.remove('selected');
          console.log('class removed');
        }
        
        // 2. Add Class to Relevant Li
        this.classList.add('selected');
        console.log('class added');
      });
      
    }

<ul id='top-nav'>
  <li>Coffee</li>
  <li>Tea</li>
  <li>Milk</li>
</ul>

Answer 2

removeClass() and addClass() are not methods of classList. What you're looking for is add() and remove().

const is useful for preventing against accidental variable overwriting. In this situation since you're not reassigning the value of menuLis, I would use const

const menuLis = document.querySelectorAll("#top-nav > li");

for (let li of menuLis) {
  
  li.addEventListener("click", function(){
    // 1. Remove Class from All Lis
    for (let li of menuLis) {
      li.classList.remove('selected');
      console.log('class removed');
    }
    
    // 2. Add Class to Relevant Li
    this.classList.add('selected');
    console.log('class added');
  });
  
}

<ul id='top-nav'>
  <li>Coffee</li>
  <li>Tea</li>
  <li>Milk</li>
</ul>

Answer 3

1) I would use var (use var to be able to reassign/alter. Without specifically knowing what your intentions are, I would use var, because I dont know what else you are planning on doing with that particular assignment) If you DO NOT reassign/alter in any case, then you could also use const

2) This is a solution using jquery:

var menuLis = $("#top-nav > li");

for (let i = 0; i < menuLis.length; i++) {
  menuLis[i].addEventListener("click", add_remove_class);
}


function add_remove_class() {
  // 1. Remove Class from All Lis
  menuLis.each(function(index) {
    $(this).removeClass("selected");
  });

  for (let i = 0; i < menuLis.length; i++) {

  }
  // add Class to clicked element
  $(this).addClass("selected");


}

li {
  cursor: pointer;
}

.selected {
  background-color: red;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.0/jquery.min.js"></script>
<ul id='top-nav'>
  <li>Coffee</li>
  <li>Tea</li>
  <li>Milk</li>
</ul>