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: stackoverflow.com/questions/111102/… — AlwaysNull
– AlwaysNull, Commented Dec 30, 2015 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
– Patrick Evans, Commented Dec 30, 2015 at 16:11
@PatrickEvans You're absolutely right. I should have read the code more carefully... :( — Andreas
– Andreas, Commented Dec 30, 2015 at 17:13

Patrick Evans · Accepted Answer · 2015-12-30 16:10:38Z

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 · Accepted Answer · 2015-12-30 16:29:35Z

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, 2015 at 16:29

answered Dec 30, 2015 at 15:56

Kostas Minaidis

5,6814 gold badges21 silver badges31 bronze badges

6 Comments

A1rPun Over a year ago

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?

Patrick Evans Over a year ago

This still wouldn't fix OP's problem, the problem is OP effectively destroys the event listeners every time they set planetenID elements innerHTML

Walter Pothof Over a year ago

Thank you so much, this fixed the problem.

Walter Pothof Over a year ago

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.

Kostas Minaidis Over a year ago

This is called an IIFE. I think, what might clear things more is this post here.

|

Collectives™ on Stack Overflow

javascript adding click events to buttons in a loop

2 Answers 2

Comments

6 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related