0

This is an extension to my previous question javascript dynamically adding and removing classes

As per the answer given, it uses a forEach loop but I am trying to use a for loop and implement the same:

function test() {
  var paragraphs = document.querySelectorAll("#divid p[id ^= 'p']");
  for(index =0; index < paragraphs.length; index++) {
    paragraphs[index].addEventListener('click', (e) => {
      for (i = 0; i < index; i++) {
        paragraphs[i].classList.remove('active');
      }
      for (i = index; i < paragraphs.length; i++) {
        paragraphs[i].classList.add('active');
      }
    });
  }
}

But I am getting the following error:

11 Uncaught TypeError: Cannot read property 'classList' of undefined
    at HTMLSpanElement.paragraphs.(anonymous function).addEventListener

I also want to remove the special symbol => in above code and solve the issue.

Cookie
  • 558
  • 8
  • 24
learner
  • 6,062
  • 14
  • 79
  • 139
  • 1
    _"avoiding forEach loop"_ - but... why? – Joseph Jul 21 '17 at 20:10
  • while testing my team reported that some browsers are not supporting this feature, I am not sure which browser is that. – learner Jul 21 '17 at 20:13
  • Yes, IE<9 are on the list ... – Teemu Jul 21 '17 at 20:14
  • 1
    @user3181365 Those browsers would not support arrow functions and `classList` as well. – Yury Tarabanko Jul 21 '17 at 20:14
  • @YuryTarabanko, oh can you please tell me what is the alternate solution in this case. I want achieve same output without using foreach – learner Jul 21 '17 at 20:15
  • @Teemu, I have seen the duplicate answer mentioned, but I am not clear how it provides solution for my issue. Can you please help me with a solution. – learner Jul 21 '17 at 20:16
  • But the dup is your solution ... It's worth of nothing, that we explain the same thing over and over again. Closures in JS are like a nose in a head, you can't separate them. Read the article carefully, and study the closures, you're not going to succeed as a JS programmer, if you're not familiar with closures. – Teemu Jul 21 '17 at 20:17
  • You could polyfill [`.forEach`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach#Polyfill) and `classList`. And replace arrow function with function or use babel to convert your code to ES5 compatible. – Yury Tarabanko Jul 21 '17 at 20:20
  • @YuryTarabanko, I am still at basic level in JS, so unable to understand it, can you please help me with an answer, so I can try from there. – learner Jul 21 '17 at 20:22
  • @user3181365 BTW linked question does provide solution to your current problem. You have canonical example of "infamous js loop issue" – Yury Tarabanko Jul 21 '17 at 20:22

1 Answers1

1

You are running into the classic "closure inside loop" issue, where your loop index variable is the same between all the closure you are creating inside the loop (the functions you are creating as event handlers).

The solution is to wrap the event handler function in an immediately invoked function expression to which you pass the index variable, binding its current value to scope.

function test() {
  var paragraphs = document.querySelectorAll("#divid p");
  for (index = 0; index < paragraphs.length; index++) {
    paragraphs[index].addEventListener('click', (function(index) {
      return function() {
        var i;
        for (i = 0; i < index; ++i) {
          paragraphs[i].classList.remove('active');
        }
        for (i = index; i < paragraphs.length; ++i) {
          paragraphs[i].classList.add('active');
        }
      }
    })(index));
  }
}

test();
p.active {
  color: red;
}
<div id="divid">
  <p>p1</p>
  <p>p2</p>
  <p>p3</p>
  <p>p4</p>
  <p>p5</p>
  <p>p6</p>
</div>
Julien Zakaib
  • 196
  • 10