6

I want to use event listeners to prevent event bubbling on a div inside a div with onclick functions. This works, passing parameters how I intended:

<div onclick="doMouseClick(0, 'Dog', 'Cat');" id="button_id_0"></div>
<div onclick="doMouseClick(1, 'Dog', 'Cat');" id="button_id_1"></div>
<div onclick="doMouseClick(2, 'Dog', 'Cat');" id="button_id_2"></div>

<script>
function doMouseClick(peram1, peram2, peram3){
    alert("doMouseClick() called AND peram1 = "+peram1+" AND peram2 = "+peram2+" AND peram3 = "+peram3);
}
</script>

However, I tried to create multiple event listeners in a loop with this:

<div id="button_id_0"></div>
<div id="button_id_1"></div>
<div id="button_id_2"></div>

<script>
function doMouseClick(peram1, peram2, peram3){
    alert("doMouseClick() called AND peram1 = "+peram1+" AND peram2 = "+peram2+" AND peram3 = "+peram3);
}

var names = ['button_id_0', 'button_id_1', 'button_id_2'];

    for (var i=0; i<names.length; i++){

        document.getElementById(names[i]).addEventListener("click", function(){
        doMouseClick(i, "Dog", "Cat");

    },false);

}

</script>

It correctly assigns the click function to each div, but the first parameter for each, peram1, is 3. I was expecting 3 different event handlers all passing different values of i for peram1.

Why is this happening? Are the event handlers not all separate?

Trojan
  • 2,256
  • 28
  • 40

5 Answers5

21

Problem is closures, since JS doesn't have block scope (only function scope) i is not what you think because the event function creates another scope so by the time you use i it's already the latest value from the for loop. You need to keep the value of i.

Using an IIFE:

for (var i=0; i<names.length; i++) {
  (function(i) {
    // use i here
  }(i));
}

Using forEach:

names.forEach(function( v,i ) {
  // i can be used anywhere in this scope
});
elclanrs
  • 92,861
  • 21
  • 134
  • 171
3

2022 edit

As someone is still reading and upvoting this answer 9 years later, here is the modern way of doing it:

for (const [i, name] of names.entries()) {
    document.getElementById(name).addEventListener("click", () => doMouseClick(i, "Dog", "Cat"), false);
}

Using const or let to define the variables gives them block-level scope and the value of i passed to the handler function is different for each iteration of the loop, as intended.

The old ways will still work but are no longer needed.

2013 answer

As pointed out already the problem is to do with closures and variable scope. One way to make sure the right value gets passed is to write another function that returns the desired function, holding the variables within the right scope. jsfiddle

var names = ['button_id_0', 'button_id_1', 'button_id_2'];

function getClickFunction(a, b, c) {
  return function () {
    doMouseClick(a, b, c)
  }
}
for (var i = 0; i < names.length; i++) {
  document.getElementById(names[i]).addEventListener("click", getClickFunction(i, "Dog", "Cat"), false);
}

And to illustrate one way you could do this with an object instead:

var names = ['button_id_0', 'button_id_1', 'button_id_2'];

function Button(id, number) {
  var self = this;
  this.number = number;
  this.element = document.getElementById(id);
  this.click = function() {
    alert('My number is ' + self.number);
  }
  this.element.addEventListener('click', this.click, false);
}
for (var i = 0; i < names.length; i++) {
  new Button(names[i], i);
}

or slightly differently:

function Button(id, number) {
  var element = document.getElementById(id);
  function click() {
    alert('My number is ' + number);
  }
  element.addEventListener('click', click, false);
}
for (var i = 0; i < names.length; i++) {
  new Button(names[i], i);
}
Stuart
  • 9,597
  • 1
  • 21
  • 30
  • This was very helpful thanks. A little advanced for me as I just started using event listeners in javascript - I will study this further. –  Jan 06 '13 at 02:54
1

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, it does not just accesses the variable i from the outer environment, but it also "remembers" i.

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

olindk
  • 91
  • 5
  • 1
    Links to external resources are encouraged, but please add context around the link so your fellow users will have some idea what it is and why it’s there. Always quote the most relevant part of an important link, in case the target site is unreachable or goes permanently offline. From [How to Answer](http://stackoverflow.com/help/how-to-answer). – Gustavo Morales Jun 24 '16 at 15:34
  • @MoralesBatovski Thank your for your reminder. I just edited my post. – olindk Jun 24 '16 at 16:43
0

I've been struggling with this problem myself for a few hours and now I've just now managed to solve it. Here's my solution, using the function constructor:

function doMouseClickConstructor(peram1, peram2, peram3){
    return new Function('alert("doMouseClick() called AND peram1 = ' + peram1 + ' AND peram2 = ' + peram2 + ' AND peram3 = ' + peram3 + ');');
}

for (var i=0; i<names.length; i++){
    document.getElementById(names[i]).addEventListener("click", doMouseClickConstructor(i,"dog","cat"));
};

Note: I havn't actually tested this code. I have however tested this codepen which does all the important stuff, so if the code above doesn't work I've probably just made some spelling error. The concept should still work.

Happy coding!

-1

Everything is global in javascript. It is calling the variable i which is set to 3 after your loop...if you set i to 1000 after the loop, then you would see each method call produce 1000 for i.

If you want to maintain state, then you should use objects. Have the object have a callback method that you assign to the click method.

You mentioned doing this for event bubbling...for stopping event bublling, you really do not need that, as it is built into the language. If you do want to prevent event bubbling, then you should use the stopPropagation() method of the event object passed to the callback.

function doStuff(event) {
    //Do things
    //stop bubbling
    event.stopPropagation();
}
jyore
  • 4,715
  • 2
  • 21
  • 26
  • Thanks for helping me with the stopPropogation() as well - It solved the other part of my problem. The eventlistener's left me with the same bubbling problem as you suggested - that I was trying to solve - but this answer here, solved my bubbling problem as well. Thanks again : ) Man! this Stackoverflow is a helpful bunch of people! –  Jan 06 '13 at 02:57
  • Your comments about "if you set i to 1000 after the loop, then you would see each method call produce 1000 for i" were also completely accurate as well : ) –  Jan 06 '13 at 02:59