javascript adding click events to buttons in a loop

Question

The code below gets info from xml file. I succesfully presents the id and name of each planet with a button.

I want to add an onclick event on the button. Problem now is: it does add the onclick event but only on the last button created in the loop.

What am i doing wrong? Why doesnt it create a onclick event for each button, but only for the last one in loop?

function updatePlaneten() {
    var valDiv, planets, valButton, textNode;
    // Get xml files
    planets = this.responseXML.getElementsByTagName("planeet");
    // loop through the <planet> tags 
    for (var i = 0; i < planets.length; i++) {
        valDiv = ''; // clear valDiv each time loop starts

        // Get the id and the name from the xml info in current <planet> tag
        valDiv += planets[i].getElementsByTagName("id")[0].childNodes[0].nodeValue + "<br>";
        valDiv += planets[i].getElementsByTagName("name")[0].childNodes[0].nodeValue + "<br>";
        document.getElementById("planetenID").innerHTML += valDiv + "<br>";

        // Create button with a value and pass in this object for later reference use (valButton.object=this)
        valButton = document.createElement("input");
        //  valButton.setAttribute("planeetID", planets[i].getElementsByTagName("id")[0].childNodes[0].nodeValue);
        valButton.setAttribute("value", 'Meer info');
        valButton.setAttribute("type", 'button');
        valButton.id = (i + 1);
        valButton.object = this;    
        //
        // Here is the problem i cant get fixed
        //    
        //valButton.onclick = function(){ showinfo(); }
        valButton.addEventListener('click', showinfo);
        // Place the button on screen
        document.getElementById("planetenID").appendChild(valButton);
    }
}

// simple function to check if it works
function showinfo() {
    console.log(this.object);
    console.log(this.id);
}

use closure for this: http://stackoverflow.com/questions/111102/how-do-javascript-closures-work — AlwaysNull, Dec 30 '15 at 15:48
@Andreas, this is not a duplicate as the issue is that OP is destroying the buttons everytime they set innerHTML effectively destroying the previously set event listeners — Patrick Evans, Dec 30 '15 at 16:11
@PatrickEvans You're absolutely right. I should have read the code more carefully... :( — Andreas, Dec 30 '15 at 17:13

score 2 · Answer 1 · answered Dec 30 '15 at 16:10

The trouble is this line:

document.getElementById("planetenID").innerHTML += valDiv + "<br>";

When you set innerHTML the content currently in there gets destroyed and replaced with the new html, meaning all your old buttons are now destroyed and new ones are created. The previously attached event listeners do not get attached to the new buttons.

Instead simply create a div/span or whatever container would best help, add your planet text or whatever to it and then use appendChild

valDiv = document.createElement("div");
var id = planets[i].getElementsByTagName("id")[0].childNodes[0].nodeValue;
var name = planets[i].getElementsByTagName("name")[0].childNodes[0].nodeValue;
valDiv.innerHTML = id+"<br>"+name+"<br>";
document.getElementById("planetenID").appendChild(valDiv);

You could also use insertAdjacentHTML

var id = planets[i].getElementsByTagName("id")[0].childNodes[0].nodeValue;
var name = planets[i].getElementsByTagName("name")[0].childNodes[0].nodeValue;
valDiv = id+"<br>"+name+"<br>";
document.getElementById("planetenID").insertAdjacentHTML("beforeend",valDiv);

Kostas Minaidis · Answer 2 · 2015-12-30T16:29:35.230

0

function updatePlaneten() {
    var valDiv, planets, valButton, textNode;
    // Get xml files
    planets = this.responseXML.getElementsByTagName("planeet");
    // loop through the <planet> tags 
    for (var i = 0; i < planets.length; i++) {

        (function(num){

            valDiv = document.createElement("div");

            // Get the id and the name from the xml info in current <planet> tag
            var id = planets[num].getElementsByTagName("id")[0].childNodes[0].nodeValue + "<br>";
            var name = planets[num].getElementsByTagName("name")[0].childNodes[0].nodeValue + "<br>";
            valDiv.innerHTML = id+"<br>"+name+"<br>";
           document.getElementById("planetenID").appendChild(valDiv);

            // Create button with a value and pass in this object for later reference use (valButton.object=this)
            valButton = document.createElement("input");
            //  valButton.setAttribute("planeetID", planets[i].getElementsByTagName("id")[0].childNodes[0].nodeValue);
            valButton.setAttribute("value", 'Meer info');
            valButton.setAttribute("type", 'button');
            valButton.id = (num + 1);
            valButton.object = this;    
            // FIX: PASS showinfo TO AN ANONYMOUS FUNCTION CONTAINING THE OBJECT
            valButton.addEventListener('click', function(){
             showinfo(valButton);   
            });
            // Place the button on screen
            document.getElementById("planetenID").appendChild(valButton);

        }(i));


    }
}

// simple function to check if it works
function showinfo(valButton) {
    console.log(valButton.object);
    console.log(valButton.id);
}

edited Dec 30 '15 at 16:29

answered Dec 30 '15 at 15:56

Kostas Minaidis

4,681
3
17
25

An IIFE is usually the fix for this problem but `showinfo` is already scoped and he isn't using `i` inside his listener. Can you explain why an IIFE is the fix here? – A1rPun Dec 30 '15 at 16:00
1

This still wouldn't fix OP's problem, the problem is OP effectively destroys the event listeners every time they set `planetenID` elements innerHTML – Patrick Evans Dec 30 '15 at 16:16
Thank you so much, this fixed the problem. – Walter Pothof Dec 30 '15 at 17:34
One more thing where can i find more info on the (function(num){ ... }(i)); methode, or function you used here, it solved my problem but i dont understand yet what is happening here. – Walter Pothof Dec 30 '15 at 17:37
This is called an [IIFE](https://en.wikipedia.org/wiki/Immediately-invoked_function_expression). I think, what might clear things more is this [post here](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example). – Kostas Minaidis Dec 30 '15 at 17:41
@WalterPothof, The IIFE did not solve your problem there is no need for the IIFE here unless you end up using your loop variable in an anonymous function for the callback. The problem was simply that you were destroying and recreating the html by appending innerHTML – Patrick Evans Dec 30 '15 at 20:41

javascript adding click events to buttons in a loop

2 Answers2