addEventListener using for loop and passing values [duplicate]

I'm trying to add event listener to multiple objects using a for loop, but end up with all listeners targeting the same object --> the last one.

If I add the listeners manually by defining boxa and boxb for every instance, it works. I guess it's the addEvent for-loop that's not working the way I hoped for. Maybe I use the wrong approach altogether.

Example using 4 of the class="container" Trigger on container 4 works the way it´s supposed to. Trigger on container 1,2,3 trigger event on container 4, but only if trigger has already been activated.

// Function to run on click:
function makeItHappen(elem, elem2) {
  var el = document.getElementById(elem);
  el.style.backgroundColor = "red";
  var el2 = document.getElementById(elem2);
  el2.style.backgroundColor = "blue";
}

// Autoloading function to add the listeners:
var elem = document.getElementsByClassName("triggerClass");

for (var i = 0; i < elem.length; i += 2) {
  var k = i + 1;
  var boxa = elem[i].parentNode.id;
  var boxb = elem[k].parentNode.id;

  elem[i].addEventListener("click", function() {
    makeItHappen(boxa, boxb);
  }, false);
  elem[k].addEventListener("click", function() {
    makeItHappen(boxb, boxa);
  }, false);
}
<div class="container">
  <div class="one" id="box1">
    <p class="triggerClass">some text</p>
  </div>
  <div class="two" id="box2">
    <p class="triggerClass">some text</p>
  </div>
</div>

<div class="container">
  <div class="one" id="box3">
    <p class="triggerClass">some text</p>
  </div>
  <div class="two" id="box4">
    <p class="triggerClass">some text</p>
  </div>
</div>

Answers:

Answer

Closures! :D

This fixed code works as you intended:

// Function to run on click:
function makeItHappen(elem, elem2) {
    var el = document.getElementById(elem);
    el.style.backgroundColor = "red";
    var el2 = document.getElementById(elem2);
    el2.style.backgroundColor = "blue";
}

// Autoloading function to add the listeners:
var elem = document.getElementsByClassName("triggerClass");

for (var i = 0; i < elem.length; i += 2) {
    (function () {
        var k = i + 1;
        var boxa = elem[i].parentNode.id;
        var boxb = elem[k].parentNode.id;
        elem[i].addEventListener("click", function() { makeItHappen(boxa,boxb); }, false);
        elem[k].addEventListener("click", function() { makeItHappen(boxb,boxa); }, false);
    }()); // immediate invocation
}
<div class="container">
  <div class="one" id="box1">
    <p class="triggerClass">some text</p>
  </div>
  <div class="two" id="box2">
    <p class="triggerClass">some text</p>
  </div>
</div>

<div class="container">
  <div class="one" id="box3">
    <p class="triggerClass">some text</p>
  </div>
  <div class="two" id="box4">
    <p class="triggerClass">some text</p>
  </div>
</div>


Why does this fix it?

for(var i=0; i < elem.length; i+=2){
    var k = i + 1;
    var boxa = elem[i].parentNode.id;
    var boxb = elem[k].parentNode.id;

    elem[i].addEventListener("click", function(){makeItHappen(boxa,boxb);}, false);
    elem[k].addEventListener("click", function(){makeItHappen(boxb,boxa);}, false);
}

Is actually non-strict JavaScript. It's interpretted like this:

var i, k, boxa, boxb;
for(i=0; i < elem.length; i+=2){
    k = i + 1;
    boxa = elem[i].parentNode.id;
    boxb = elem[k].parentNode.id;

    elem[i].addEventListener("click", function(){makeItHappen(boxa,boxb);}, false);
    elem[k].addEventListener("click", function(){makeItHappen(boxb,boxa);}, false);
}

Because of variable hoisting, the var declarations get moved to the top of the scope. Since JavaScript doesn't have block scope (for, if, while etc.) they get moved to the top of the function. Update: as of ES6 you can use let to get block scoped variables.

When your code runs the following happens: in the for loop you add the click callbacks and you assign boxa, but its value gets overwritten in the next iteration. When the click event fires the callback runs and the value of boxa is always the last element in the list.

Using a closure (closing the values of boxa, boxb etc) you bind the value to the scope of the click handler.


Code analysis tools such JSLint or JSHint will be able to catch suspicious code like this. If you're writing a lot of code it's worthwhile to take the time to learn how to use these tools. Some IDEs have them built-in.

Answer

You facing the scope/closure problem as function(){makeItHappen(boxa,boxb);} boxa and boxb references then always the last one element(s).

To solve the issue:

function makeItHappenDelegate(a, b) {
  return function(){
      makeItHappen(a, b)
  }
}

// ...
 elem[i].addEventListener("click", makeItHappenDelegate(boxa,boxb), false);
Answer

You can use Function Binding.You dont need use closures.See below:

Before:

function addEvents(){
   var elem = document.getElementsByClassName("triggerClass");

   for(var i=0; i < elem.length; i+=2){
      var k = i + 1;
      var boxa = elem[i].parentNode.id;
      var boxb = elem[k].parentNode.id;

      elem[i].addEventListener("click", function(){makeItHappen(boxa,boxb);}, false);
      elem[k].addEventListener("click", function(){makeItHappen(boxb,boxa);}, false);
   }
}

After:

function addEvents(){
   var elem = document.getElementsByClassName("triggerClass");

   for(var i=0; i < elem.length; i+=2){
        var k = i + 1;
      var boxa = elem[i].parentNode.id;
      var boxb = elem[k].parentNode.id;
      elem[i].addEventListener("click", makeItHappen.bind(this, boxa, boxb), false);
      elem[k].addEventListener("click", makeItHappen.bind(this, boxa, boxb), false);
   }
}
Answer

I also had this problem a while ago. I solved it by using a "adds" function outside the loop, to assign events, and it worked perfectly.

Your script should look like.

function addEvents(){
  var elem = document.getElementsByClassName("triggerClass");
  for(var i=0; i < elem.length; i+=2){
    var k = i + 1;
    var boxa = elem[i].parentNode.id;
    var boxb = elem[k].parentNode.id;

//- edit ---------------------------|
    adds(boxa, boxb);
  }
}
//- adds function ----|
function adds(obj1, obj2){
  obj1.addEventListener("click", function(){makeItHappen(obj1, obj2);}, false);
  obj2.addEventListener("click", function(){makeItHappen(obj1, obj2);}, false);
}
//- end edit -----------------------|

function makeItHappen(elem, elem2){
  var el = document.getElementById(elem);
  el.style.transform = "flip it";
  var el2 = document.getElementById(elem2);
  el2.style.transform = "flip it";
}

Answer

It's because of closures.

Check this out: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Closures#Creating_closures_in_loops_A_common_mistake

The sample code and your code is essentially the same, it's a common mistake for those don't know "closure".

To put it simple, when your create a handler function inside addEvents(), it does not just accesses the variable i from the addEvents()'s environment, but it also "remembers" i.

And because your handler "remembers" i, the variable i won't vanish after addEvents() was executed.

So when the handler is called, it will use the i but the variable i is now, after the for-loop, 3.

Tags

Recent Questions

Top Questions

Home Tags Terms of Service Privacy Policy DMCA Contact Us

©2020 All rights reserved.