JavaScript event being overwritten in for loop

Question

I am creating JavaScript events on items within a loop, but every time I fire the event it only ever fires the last event created.

JavaScript:

var dirLinks = document.getElementsByClassName("dirLink"),
    iframe   = document.getElementById("tmpImg");

showDir.addEventListener('click', showDirContent, false);
showImgs.addEventListener('click', showImgsContent, false);

for (var i = 0; i < dirLinks.length; i++) {
    var link = dirLinks[i];
    link.onclick = function () {
        updateIframeSource(link.getAttribute("imgSrc"));
    }
}

function updateIframeSource(source) {
    iframe.src = source;
}

HTML:

<a class="dirLink" imgSrc="img1.jpg" href="#">img1.jpg</a><br />
<a class="dirLink" imgSrc="img2.jpg" href="#">img2.jpg</a><br />
<a class="dirLink" imgSrc="img3.jpg" href="#">img3.jpg</a><br />
<a class="dirLink" imgSrc="img4.jpg" href="#">img4.jpg</a><br />
<a class="dirLink" imgSrc="img5.jpg" href="#">img5.jpg</a>

<iframe id="tmpImg"></iframe>

No matter which link I click on, it will always hit the updateIframeSource function with img5.jpg passed in.

what is `showDir` and `showImgs`? – Vicky Gonsalves Feb 01 '16 at 16:31 — Vicky Gonsalves, Feb 01 '16 at 16:31
They are part of the script that I forgot to remove, sorry. – ltoodee Feb 01 '16 at 20:53 — ltoodee, Feb 01 '16 at 20:53

score 5 · Answer 1 · answered Feb 01 '16 at 16:34

5

It's because in javascript for loop don't create new scope in each iteration (only functions does) so you reference one variable link that in a function that is executed when loop is finished so it's always 5, to fix this just wrap the whole thing in anonymous function like this:

for(var i = 0; i < dirLinks.length; i++) {
    (function(link) {
        link.onclick = function () {
            updateIframeSource(link.getAttribute("imgSrc"));
        };
    })(dirLinks[i]);
}

answered Feb 01 '16 at 16:34

jcubic

61,973
54
229
402

@Dale-G: Also see [Explain JavaScript's encapsulated anonymous function syntax](http://stackoverflow.com/questions/1634268/explain-javascripts-encapsulated-anonymous-function-syntax) for additional information about jcubic's solution. – Yogi Feb 01 '16 at 16:44
And you must read what are the closures in Javascript, here an [Article](http://javascriptissexy.com/understand-javascript-closures-with-ease/) – dexhering Feb 01 '16 at 17:15

score 2 · Answer 2 · answered Feb 01 '16 at 16:39

This is because the loop does not create a new scope. Because javascript has function scope:

var bindEvent = function(intId){
     var link = dirLinks[intId];
     link.onclick = function () {
            updateIframeSource(link.getAttribute("imgSrc"));
     }
}

for(var i = 0; i < dirLinks.length; i++) {
    bindEvent(i);
}

see jsfiddle

score 0 · Answer 3 · answered Feb 01 '16 at 17:09

use also this

for(var i = 0; i < dirLinks.length; i++) {
    var link = dirLinks[i];
    link.addEventListener("click", updateIframeSource());
}

function updateIframeSource() {

    return (function(event){
      iframe.src  = event.target.getAttribute("imgSrc");
      alert(iframe.src);
    });



}

score 0 · Answer 4 · answered Feb 01 '16 at 17:29

The operation of assign an eventhandler are expensive it's better wrap all anchor links and use event delegation:

here how:

<div id="container">
    <a class="dirLink" imgSrc="img1.jpg" href="#">img1.jpg</a><br />
    <a class="dirLink" imgSrc="img2.jpg" href="#">img2.jpg</a><br />
    <a class="dirLink" imgSrc="img3.jpg" href="#">img3.jpg</a><br />
    <a class="dirLink" imgSrc="img4.jpg" href="#">img4.jpg</a><br />
    <a class="dirLink" imgSrc="img5.jpg" href="#">img5.jpg</a>
</div>
<script>
var dirLinks = document.getElementsByClassName("dirLink");
var iframe = document.getElementById("tmpImg");
// Get the container
var container = document.getElementById("container");

//showDir.addEventListener('click', showDirContent, false);
//showImgs.addEventListener('click', showImgsContent, false);

container.addEventListener('click', function(evt){
    // You can send the event or extract like this the value of "imgsrc"
    updateIframeSource(evt.target.attributes.imgsrc.value);
});

function updateIframeSource(source) {
    iframe.src = source;
}
</script>

score 0 · Answer 5 · answered Feb 02 '16 at 04:30

Here is another way with forEach loop

HTML:

  <a class="dirLink" imgSrc="img1.jpg" href="#">img1.jpg</a>
  <br />
  <a class="dirLink" imgSrc="img2.jpg" href="#">img2.jpg</a>
  <br />
  <a class="dirLink" imgSrc="img3.jpg" href="#">img3.jpg</a>
  <br />
  <a class="dirLink" imgSrc="img4.jpg" href="#">img4.jpg</a>
  <br />
  <a class="dirLink" imgSrc="img5.jpg" href="#">img5.jpg</a>

  <iframe id="tmpImg"></iframe>

JS:

var dirLinks = document.getElementsByClassName("dirLink");
var iframe = document.getElementById("tmpImg");

[].forEach.call(dirLinks, (l) => {
  l.addEventListener('click', function() {
    updateIframeSource(l.getAttribute("imgSrc"));
  })
})

function updateIframeSource(source) {
  iframe.src = source;
}

JavaScript event being overwritten in for loop

5 Answers5