Toggle the class of unordered list items using simple Javascript

Question

I am working on a project that requires me to toggle the class of li element every time I click on it.

I have the following code:

var list = document.getElementById('nav').children[0];
for (var i = 0; i < list.children.length; i++) {
  var el = list.children[i];
  el.addEventListener("click", function() {

    el.classList.toggle("myClass");
  });
}

<div id='nav'>
  <ul>
    <li>Item 1</li>
    <li>Item 2</li>
    <li>Item 3</li>
    <li>Item 4</li>
    <li>Item 5</li>
  </ul>
</div>

But this code adds the class only to the last li element no matter which li I click on, I have taken reference from Use JavaScript to change the class of an <li> and tried to tailor the same to my needs, but there is something wrong here. Any help will be really appreciated.

score 4 · Accepted Answer · answered Jul 24 '18 at 09:28

4

You shouldn't attach the click event inside the loop, create a function out of the loop and call it when you attach the click event like :

var list_items = document.querySelectorAll('#nav>ul>li');

for (var i = 0; i < list_items.length; i++) {
  list_items[i].addEventListener("click", toggle);
}

function toggle() {
  this.classList.toggle("myClass");
}

.myClass {
  color: green;
}

<div id='nav'>
  <ul>
    <li>Item 1</li>
    <li>Item 2</li>
    <li>Item 3</li>
    <li>Item 4</li>
    <li>Item 5</li>
  </ul>
</div>

answered Jul 24 '18 at 09:28

Zakaria Acharki

66,747
15
75
101

Thanks my code works now, as I understand the problem was that when I referred to the **el** variable inside the loop, it used to bring me to the last list element, to resolve that I need to refer to each element using the **this** keyword, am I correct? Also why should I not add the click event inside the for loop? – CodeHumor Jul 25 '18 at 07:07
Yes you're right, take a look to https://decembersoft.com/posts/understanding-javascript-closures-in-for-loops/ – Zakaria Acharki Jul 25 '18 at 08:12
Thanks mate really appreciate the help – CodeHumor Jul 25 '18 at 09:27

score 1 · Answer 2 · answered Jul 24 '18 at 09:26

1

Change el.classList.toggle("myClass"); to this.classList.toggle("myClass"); and it will work, as it refers to the current element

answered Jul 24 '18 at 09:26

Jimmy Hedström

561
2
10

score 0 · Answer 3 · answered Jul 24 '18 at 09:32

Change the function inside the loop into an IIFE to solve the closure issue that binds only the last element to the function.

var list = document.getElementById('nav').children[0];
for (var i = 0; i < list.children.length; i++) {
  list.children[i].addEventListener("click", (function(j) {
    return function() {
      var el = list.children[j];
      el.classList.toggle("myClass");
    }

  }(i)));
}

.myClass {
  background-color: red;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id='nav'>
  <ul>
    <li>Item 1</li>
    <li>Item 2</li>
    <li>Item 3</li>
    <li>Item 4</li>
    <li>Item 5</li>
  </ul>
</div>

score 0 · Answer 4 · answered Jul 24 '18 at 10:02

var li = document.querySelectorAll("li");
for(var i  of li){
i.addEventListener("click",function(){
this.classList.toggle("myClass")
})
};

.myClass {
  color: green;
}

<div id='nav'>
  <ul>
    <li>Item 1</li>
    <li>Item 2</li>
    <li>Item 3</li>
    <li>Item 4</li>
    <li>Item 5</li>
  </ul>
</div>

Toggle the class of unordered list items using simple Javascript

4 Answers4