Purpletoucan
Purpletoucan

Reputation: 6592

Dynamically Assigned Click Events Triggering Wrong Action in Javascript

I am looping through a Javascript array, attaching events to elements by ID. I can see that the correct event is added, however, when I click on the links to trigger the event, they all display 'link6' was clicked, rather than the one I intended!

Below is a snippet that illustrates the problem.

I would be enormously grateful for an explanation of why this happens and how I should be coding to overcome this. I am guessing it is something to do with dynamic values being assigned, but my initial thoughts are that this doesn't seem to be behaving logically!

<html>
<body>
<p id='link1'>Link 1</p>
<p id='link2'>Link 2</p>
<p id='link3'>Link 3</p>
<p id='link4'>Link 4</p>
<p id='link5'>Link 5</p>
<p id='link6'>Link 6</p>

<script type='text/javascript'>

var sections = new Array('link1', 'link2', 'link3', 'link4', 'link5', 'link6');

    for (var section in sections) {
        console.log('Attaching event to ' + sections[section] );    
        document.getElementById(sections[section]).addEventListener('click', function(e){ alert('click '+sections[section]); });
    }

</script>
</body>

Upvotes: 0

Views: 96

Answers (3)

Purpletoucan
Purpletoucan

Reputation: 6592

With more searching (and with thanks to sys.stderr who pointed me towards closures) I have found an almost identical question with a very detailed answer about scope.

The suggested solution is to change scope of the loop using 'with':-

var sections = new Array('link1', 'link2', 'link3', 'link4', 'link5', 'link6');

for (var section in sections) {
    with({x:section}) {
        console.log('Attaching event to ' + sections[x] );    
        document.getElementById(sections[x]).addEventListener('click', function(e){
        alert('click '+sections[x]); });
    }
}

gabitzish's response about using 'this' also works and so I will accept this the correct answer.

Upvotes: 0

bigblind
bigblind

Reputation: 12867

This is because of how closures work. In the line

for (var setting in settings){}

You're assigning a new value to the setting variable in every iteration. However, the function that you attach as the event handler, keeps a reference to the variable that's local to the loop. Therefore, in the second iteration, you're changing the value, which also gets picked up by the function created in the first one, as it's value for the variable section is bound to the value inside your iteration. By the final iteration, all functions have the same reference.

You can very easily avoid this problem as follows:

var sections = new Array('link1', 'link2', 'link3', 'link4', 'link5', 'link6');

for (var section in sections) {
    console.log('Attaching event to ' + sections[section] );    
    document.getElementById(sections[section]).addEventListener('click', function(e){
    var section = section;
    alert('click '+sections[section]); });
}

This way, you're creating a local reference for each function.

Upvotes: 1

gabitzish
gabitzish

Reputation: 9691

This works:

<html>
<body>
<p id='link1'>Link 1</p>
<p id='link2'>Link 2</p>
<p id='link3'>Link 3</p>
<p id='link4'>Link 4</p>
<p id='link5'>Link 5</p>
<p id='link6'>Link 6</p>

<script type='text/javascript'>

var sections = new Array('link1', 'link2', 'link3', 'link4', 'link5', 'link6');

for (var section in sections) {
    console.log('Attaching event to ' + sections[section] );    
    document.getElementById(sections[section]).addEventListener('click', function(e){ alert('click '+ this.getAttribute("id")); });
}

Try using "this" inside the callback functions to get the right object.

Upvotes: 1

Related Questions