CODEWITHSUNDEEP
javascriptarraysfor-loopaddeventlistener
Asd Asd
Asd Asd

Reputation: 23

Javascript: addEventListener not working in a for-loop

SITUATION: I am getting a JSON array from an API. The "div" is from a getElementByID earlier in my code.

PROBLEM: the addEventListener only gets attached to the last element in my list.

    for(var i = 0; i < JSONarray.response.artists.length; i++){
      var artistName = JSONarray.response.artists[i];
      div.innerHTML += "<h4 id=\"artist" + i + "\" accessKey=\"" + artistName.id + "\">Artist Name: " +  artistName.name + "</h4>";
      document.getElementById("artist" + i).addEventListener("click", moreInfo, false);
    }

WEIRD THING: If I place the addEventListener in its own for-loop (with exact same loop script), it is perfectly happy and all my list items get their Listeners.

QUESTION: Is there a reason why I cannot put everything in one for-loop, or is it my code?

NOTE: I'm using Chrome to run this, if it happens to be a browser thing.

Thanks for your time!

Upvotes: 2

Views: 1526

Answers (3)

JayC
JayC

Reputation: 7141

You are modifying the innerHTML of the div each iteration for loop, thus causing all event handlers bound to some node that has that div as parent to be lost, as modifying the innerHTML causes all child nodes to be recreated from HTML.

Consider that div.innerHTML += foo; is really shorthand for div.innerHTML = div.innerHTML + foo; and it should be apparent what I mean.

Edit: I realize my answer has already been accepted (as of April 8, 2013), but I still feel like I could have worded things a little better.

Think of node.innerHTML = bar as "replace everything that's a child of the node with whatever HTML can be interpreted from the string given (in bar)". It just so happens as part of the string given, you were giving the HTML serialization of the node's former children via node.innerHTML.. so you are possibly loosing much more than just event handlers!

As you can see, this is not the best way to add html.

As a side note, supposing you do have to add a bunch of children to a node, if the event can bubble (see HTML DOM: Which events do not bubble?, or a more complete list on Wikipedia's article on DOM events), instead of adding a handler to each and every child node, you could also add a event handler to the parent node, and in modern browsers (IE9+), many events could be handled as they are bubbled up. I haven't directly messed with event bubbling in the DOM as of late, but many third party libraries easily support this kind of event delegation. With jQuery, look at jQuery.on, and binding declarations would work something like:

 jQuery(parentnodeselector).on(eventname,childselector)

AFAIK, it's not terribly hard to handle event bubbling on your own... you would need to be able to identify via event.target and event.target's parents if the event bubbled up through the right children, though.

Upvotes: 2

Ian
Ian

Reputation: 50905

Inline event handlers, especially when created dynamically with innerHTML should not be encouraged. I even remember reading bad things about what happens in IE with this scenario. Try using something like this, which uses a more appropriate method for adding elements to the DOM:

for (var i = 0; i < JSONarray.response.artists.length; i++) {
    var artistName = JSONarray.response.artists[i];
    var newH4 = document.createElement("h4");
    newH4.id = "artist" + i;
    newH4.accessKey = artistName.id;    // You might have to use newH4.setAttribute("accessKey", artistName.id) instead
    newH4.innerHTML = "Artist Name: " +  artistName.name;
    newH4.addEventListener("click", moreInfo, false);
    div.appendChild(newH4);
}

If you care, older IE doesn't have addEventListener - so you'd need to check for addEventListener, and use attachEvent if it's not available. Just a warning. Something like:

if (newH4.addEventListener) {
    newH4.addEventListener("click", moreInfo, false);
} else if (newH4.attachEvent) {
    newH4.attachEvent("onclick", moreInfo);
} else {
    newH4.onclick = moreInfo;
}

Also, you shouldn't use innerHTML on the same element in a loop. It's more efficient to create one big string and append to that inside the for loop, then set innerHTML on the element after the loop. This gets considerably more important as the size of your JSONarray.response.artists array increases.

Upvotes: 1

Arun P Johny
Arun P Johny

Reputation: 388316

It looks like a dom update problem

for(var i = 0; i < JSONarray.response.artists.length; i++){
      var artistName = JSONarray.response.artists[i];
      div.innerHTML += '<h4 id="artist' + i + '" accessKey="' + artistName.id + '" onclick="moreInfo(this)">Artist Name: ' +  artistName.name + '</h4>';
}

Demo: Fiddle

Upvotes: 0

Related Questions