1
function newMenu(items,results){
    document.getElementById("menu").innerHTML = "";
    for (var i = 0; i < items.length; i++) {
        document.getElementById("menu").innerHTML += "<li>" + items[i] + "</li>";
        document.getElementById("menu").childNodes[i].addEventListener("click",function(){document.write("foo")});

    };
}

I am trying to use this code to generate a list and then give each of the list items a different function to be activated when they are clicked (right now I've just made them all write "foo" to test that the event listeners work). However, when I run this function, it will write all of the list content correctly (they're passed as an array into 'items').

My problem arises with the event listeners. Only the last list item's event listener works. That is to say, only when I click on the last li in the list does it execute document.write("foo"). I've tried changing the size of the items array and every time it's the last list item, regardless of length. Why might this be? Are the event listeners overwriting one another?

  • 2
    possible duplicate of [adding 'click' event listeners in loop](http://stackoverflow.com/questions/8909652/adding-click-event-listeners-in-loop) – dee-see Jul 30 '14 at 19:38
  • Depending on your use case, you may also want to attach a single onclick event to a higher-level element in the DOM, and then check the target when the click event fires. – HartleySan Jul 30 '14 at 20:43

1 Answers1

1

Wow! This took me a while to figure out. The problem is that when you call

document.getElementById("menu").innerHTML += "<li>" + items[i] + "</li>"

you completely replace the existing nodes with new list items. Instead of appending a string to the existing innerHTML, use the appendChild() function instead

for (var i = 0; i < items.length; i++) {
  node = document.createElement("li");
  node.innerText = items[i];
  node.addEventListener("click", function() { console.log("ok") });
  document.getElementById("menu").appendChild(node);
};
fbonetti
  • 6,652
  • 3
  • 34
  • 32
  • 1
    It's clear I made a technical error is asserting that closures were the reason for the trouble here. Nice catch on clobbering the DOM tree. +1 – jdphenix Jul 30 '14 at 21:04