3

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>

3
  • 1
    It’s li.classList.remove, not li.classList.removeClass – see the error when you run the snippet. Same for addClassadd. And yes, const is better than var. (var should almost never be used if you have let/const available.)
    – Ry-
    Jun 9, 2019 at 18:03
  • 1
    li.addEventListener("click", function() { [...document.querySelector('.selected')].forEach( e => e.classList.remove('selected') ); this.classList.add('selected') }) Jun 9, 2019 at 18:04
  • 1
    Possible duplicate of How to change an element's class with JavaScript? Jun 9, 2019 at 18:10

3 Answers 3

1

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>

3
  • very helpful, thank you. Quick question - could you give some examples of variable overwriting? And what are you referring to as the value of the menuLis array? I'm just trying to get my head around const :) Jun 9, 2019 at 18:22
  • 1
    Say you create a variable using var (var color = "red"), you can then reassign the value of that variable to something else by saying color = "blue". However, when you create a variable using const, you will get an error if you try to reassign the variable a second time
    – Alex
    Jun 9, 2019 at 18:26
  • ahh I see - hence constant. Thanks! Jun 11, 2019 at 10:25
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>

0

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>

2
  • Rationale for using var?
    – Ry-
    Jun 9, 2019 at 18:39
  • well i would use var to be able to reassign/alter. Without specifically knowing the authors intentions I would always use var, because I dont know what else he/she is planning on doing with that particular assignment later on. Jun 9, 2019 at 18:46

Not the answer you're looking for? Browse other questions tagged or ask your own question.