Set current element in onclick event handler loop

Question

I'm looping over multiple elements, which I'd like to assign an onclick event handler too.

The problem is that the element sent to the goTo function (event handler), is always the last element in the loop. What am I doing wrong?

var navLinks = document.getElementsByClassName('navigation');

    for (var i = 0; i < navLinks.length; i++) {
        var navLink = navLinks[i];
        navLink.onclick = function() { goTo.call(navLink); }
    }

Edgar Villegas Alvarado · Accepted Answer · 2014-01-12T23:57:33.713

1

You should add a closure, like this:

var navLinks = document.getElementsByClassName('navigation');

for (var i = 0; i < navLinks.length; i++) {
    var navLink = navLinks[i];
    (function(navLink){   //This line does the magic
         navLink.onclick = function() { goTo.call(navLink); }  
    })(navLink);  //This calls the created function      
}

This way, the inner navLink will be unique per loop cycle, so it won't get overwritten (that's why it remained with the latest value in your previous code)

Cheers

edited Jan 12 '14 at 23:57

answered Jan 12 '14 at 23:44

Edgar Villegas Alvarado

18,204
2
42
61

I know this is old, but the solution is *not* to "add a closure". The solution works because a function is *executed*, thus creating a new scope. Whether the function is a closure or not is irrelevant. – Felix Kling Jan 16 '18 at 00:15

score 0 · Answer 2 · answered Jan 16 '18 at 00:14

What am I doing wrong?

See JavaScript closure inside loops – simple practical example for an explanation.

Inside the event handler you can access the element itself via this (this works also if you used addEventListener to bind the handler instead):

navLink.onclick = function() { goTo.call(this); }

And assuming goTo is a function, you can shorten the code to

navLink.onclick = goTo;

in your case.

Set current element in onclick event handler loop

2 Answers2