Having trouble with createElement and appendChild

Question

I have a reasonably simple idea that I would like to implement.

I have an array of objects with two properties: "id" and "name" I would like to list these in a series of "p" tags that would be within a "div".

So here is the HTML:

<body>
    <div id="listView"></div>
</body>

And here is the JavaScript in the tag:

sessionStorage.eventid = 2;
var playerList = [];
playerList[0].id = 0;
playerList[0].name = "Add New Player";
playerList.push({
    id: 5,
    name: "Asako"
});
playerList.push({
    id: 6,
    name: "James"
});
playerList.push({
    id: 7,
    name: "Brian"
});
playerList.push({
    id: 8,
    name: "Helmut Spargle"
});

function listAll() {
    var element = document.getElementById("listView");
    var div;
    var node;
    for (var i = 0; i < playerList.length; i++) {
        div = document.createElement("div");
        div.setAttribute("onClick", "javascript: formOver(" + playerList[i].id + ")");
        node = "<p>" + playerList[i].name + "<br />\n" +
        "<small>&nbsp</small>\n" +
        "</p>\n";
        div.appendChild(node);
        element.appendChild(div);
    }
}
window.onLoad = function(){
    listAll();
}

This doesn't fill the with anything. I have put this up on JSFiddle as well.

Have I misunderstood how Array.push works? Or something to do with appendChile and createElement?

Thanks in advance for your help.

You have two problems (look at the javascript console in your fiddle). The first is you can set a property on `playerList[0]` when `playerList[0]` is undefined. So `playerList[0].id` is invalid. Just add items with `push` as you are will all the others. Second, you can't use `appendChild` and pass it a string. You need to pass it an element. For example, an element created with `createElement`. — Matt Burland, Jun 26 '14 at 01:45
How do I see the console? I have never used JSFiddle before. I couldn't find the button for it. Thank you for all of your other advice though, @MattBurland. It has worked a treat. Except now I have used `document.creatTextNode("string");` and it outputs all of the tags as strings instead of parsing them as HTML. Does that mean I will have to make a whole bunch of elements and nodes separate from one another? Or should I just use `innerHTML`? — SimpleProgrammer, Jun 26 '14 at 01:54
You can use `innerHTML` if you want. For the console, depends on your browser, but F12 is a good bet. Otherwise look in the menus in your browser probably under "tools" and "web developer" or something similar. Or just Google "how to see the javascript console in browser *x*". The console is the first step in debugging, that's where your error messages will appear. — Matt Burland, Jun 26 '14 at 02:18
I thought you meant a console within JSFiddle itself. But now that you have said that it seems far too obvious. Cheers! — SimpleProgrammer, Jun 26 '14 at 03:29

score 0 · Accepted Answer · answered Jun 26 '14 at 01:46

Two problems - up front, trying to set the id and name on playerList[0] (which doesn't exist yet) won't work.

Second, trying to add a whole "node" full of html, jQuery-style, doesn't work in a plain-JS world. You need to build up the individual elements.

sessionStorage.eventid = 2;
var playerList = [];

playerList.push({
    id: 0,
    name: "Add New Player"
});
playerList.push({
    id: 5,
    name: "Asako"
});
playerList.push({
    id: 6,
    name: "James"
});
playerList.push({
    id: 7,
    name: "Brian"
});
playerList.push({
    id: 8,
    name: "Helmut Spargle"
});

function listAll() {

    var element = document.getElementById("listView");
    var div = document.createElement("div");
    var node = "some string";
    for (var i = 0; i < playerList.length; i++) {
        div = document.createElement("div");
        div.setAttribute("onClick", "formOver(" + playerList[i].id + ")");

        var p = document.createElement('p');
        p.innerHTML = playerList[i].name;

        var br = document.createElement('br');
        p.appendChild(br);

        var small = document.createElement('small');
        small.innerHTML = '&nbsp;';
        p.appendChild(small);

        div.appendChild(p);

        element.appendChild(div);
    }
}
listAll();

Example: http://jsfiddle.net/NF45y/

I went with `div.innerHTML = "
" + playerList[i].name + "
\n &nbsp\n
\n";` instead, but your answer gave me a much deeper understanding. Thank you. — SimpleProgrammer, Jun 26 '14 at 02:01
Note that `div.setAttribute("onClick", "formOver(" + playerList[i].id + ")")` effectively results in `formOver(undefined)`. — RobG, Jun 26 '14 at 02:47
I am definitely not seeing that as a problem here (with Chrome, Firefox or Opera). I get `
` and it does what it should when clicked. Just read your comment below, it may just be something I neglected in Fiddle to reduce the amount of unnecessary code to debug. — SimpleProgrammer, Jun 26 '14 at 03:37

score 0 · Answer 2 · answered Jun 26 '14 at 01:49

Initially:

var playerList = [];
playerList[0].id = 0;

You should get an error here about trying to set the id property of undefined. You can do:

var playerList = [{}];
playerList[0].id = 0;
playerList[0].name = "Add New Player";

or add the object the same way the others are. Within the function;

function listAll() {
    var element = document.getElementById("listView");
    var div;
    var node;
    for (var i = 0; i < playerList.length; i++) {
        div = document.createElement("div");
        div.setAttribute("onClick", "javascript: formOver(" + playerList[i].id + ")");

Don't use setAttribute to add listeners that way, it's not how it's intended to be used and doesn't work everywhere, use DOM properties for simplicity:

        div.onclick = function(){...};

Since you are setting values inside a loop, you need to break the closure with the variables. An immediately invoked function expression (IIFE) can help:

        div.onclick = (function(id) {
                         return function(){formOver(id);}
                      }(playerList[i].id));

Don't use XML syntax in an HTML document, and there isn't much point in the '\n' unless you are going to read the markup:

        node = "<p>" + playerList[i].name + "<br><small>&nbsp</small></p>";
        div.appendChild(node);

appendChild expects a DOM element, not markup. Since this is the only content of the div, you can use the markup to set its innerHTML property (you might want to change the name of node to say markup):

        div.innerHTML = node;

        element.appendChild(div);
    }
}

Don't use dom properties for events. Use `addEventListener("click", function(){})` — PitaJ, Jun 26 '14 at 01:50
Because its against the normal convention. It also allows adding more than one handler per element. http://stackoverflow.com/questions/6348494/addeventlistener-vs-onclick — PitaJ, Jun 26 '14 at 01:56
The OP is only adding one listener, it doesn't preculde the adding of other listeners for the same event, it's supported in every browser in use (try *addEventListener* in IE8), it's less code and faster. BTW, in your "fiddle" the handler calls *formOver* with a value of `undefined`. And the answer you linked to (apparently in support of "don't do this") doesn't support not doing that. ;-) — RobG, Jun 26 '14 at 02:08

Having trouble with createElement and appendChild

2 Answers2