This question already has an answer here:
-
JavaScript closure inside loops – simple practical example
39 answers
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>
Closures! :D
This fixed code works as you intended:
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
}
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.
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);
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);
}
}
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.
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\";
}