views:

811

answers:

6

I kind of feel like I'm abusing the DOM with my code...

for(var i=0; i<json.length; ++i){    
    var newobj = document.createElement("div");
    var inner = "<img src='"+json[i].picture+"' alt='Item preview' class='icon' />";

    inner += "<p>"+json[i].id+"<br />qty:"+json[i].qty;
    inner += "<a href=\"javascript:clearProduct('"+json[i].id+"')\">";
    inner += "<img alt='Seperator' src='images/form-sep.gif' />";
    inner += "<img src='images/cross.png' alt='Remove this item.' title='Remove this item.' />";
    inner += "</a></p>";
    newobj.innerHTML = inner;
    newobj.className = "cart_item";
    $('cartitems').appendChild(newobj);
    $('cartcount').innerHTML = json.length;
}

Is there a better way to do this? I mean, yes I could through and use createElement for each element and setting each attribute separately but that seems like a lot just when this is so simple. Is there a better way?

+2  A: 

I don't know prototype, but I usually build my objects in jQuery like this :

$("<p>").append(
  $("<img>")
  .attr("alt", "Sperator")
  .attr("src", "images/form-sep.gif")
).append(
  $("<img>")
  ... etc ...
)
Paul Tarjan
+2  A: 

You can use document.createElement() to create every element in your DOM tree - but this would indeed be way more clumsier (although arguably more "correct"). Not to mention WAY slower on IE. I've run into this problem before - DOM operations are extremely sluggish for IE. Assigning a whole bunch of elements via innerHTML is several orders of magnitude faster for it. Of course, you won't feel it if you're creating just a few elements. But for a 10x300 table it takes several seconds (more than 10 on my old Sempron 2200+ PC).

Vilx-
I never realized _how_ much slower createElement et al are compared to innerHTML until it was mentioned on "High-performance JavaScript: Why Everything You've Been Taught Is Wrong",http://video.yahoo.com/watch/1041101/3881103
VolkerK
A: 

I never use innerHTML. If you try and append strings to innerHTML and then later refer to the DOM objects, you can get into trouble in IE. I could use innerHTML safely in many cases, but I see no reason to. My preferred idiom is to create each DOM object, and then append text as follows:

var div = document.createElement('div');
div.appendChild(document.createTextNode('This is the inner text'));

However, I use some abbreviations, so my code actually would look like this:

var div = dec('div');
div.appendChild(dct('This is the inner text'));

I would like to abbreviate the appendChild() method as well, since that's such a common JavaScript method, but IE also complains if you try and monkey-patch the DOM by creating a .ac() method.

Andrew Johnson
div.textContent += 'This is the inner text' is a simpler but still standards-compliant way to do this.
Matthew Flaschen
Interesting... you never know what you don't know.
Andrew Johnson
+4  A: 

I like to create a nice element-creation interface; something like this:

function el( name, attrs ) {

    var elem = document.createElement(name),
        styles = attrs.style;

    for (var prop in styles) {
        try {
            elem.style[prop] = styles[prop];
        } catch(e) {}
    }

    for (var attr in attrs) {
        if (attr === 'style') { continue; }
        elem[attr] = attrs[attr];
    }

    return elem;

}


var myImage = el('img', {
    src: '/path/to/image.jpg',
    alt: 'Really cool image',
    style: {
        border: '1px solid red',
        padding: '10px'
    }
});
J-P
Wow, you just wrote an almost identical function to what I was writing as an answer, but you were faster :D
Jani Hartikainen
A: 

TBH, unless you're using a pre-rolled library (or are OCDish enough write your own), you're probably best off using innerHTML for as long as IE<8 is a significant factor (which, given the inertia of corporate IT, will be a long time). The (using the term loosely) DOM implementation in IE prior to IE8 is so buggy, non-standard and incomplete that any at least usably complete code will be littered with conditionals and special cases.

FWIW, I've found it clearest to build HTML from a data denotation, with the code that translates it as far as possible assuming a working DOM, with post-hoc fixup for IE done with code that's only loaded for broken browsers.

Here's what your code would look like:

horus.appendChild
  ('cartitems',
   [ 'div', '.cart_item',
     [ 'img', { src: json[i].picture, alt: 'Item preview', classname: 'icon' } ],
     [ 'p', null,
       json[i].id, [ 'br' ], json[i].qty,
       [ 'a', { href: 'javascript:clearProduct('+json[i].id+')' },
         [ 'img', { alt: 'Seperator', src: 'images/form-sep.gif' } ],
         [ 'img', { src: 'images/cross.png', title: 'Remove this item.' } ] ] ] ]);

horus.childText('cartcount', json.length);

I'm not going to post our libraries here in their entirely, but have a look in here (starting at line 574, with the meat at 743) if you're curious, with the IE fixup code in here.

Pete Jordan
+1  A: 

Whenever I am doing complicated element creation via JS - I normally use Prototypes interpolate method.

To do this, create a template in HTML for the element you want to create and put it on the bottom of your page with display:none so it is hidden. Then use Prototype's interpolate method to replace the specific fields with your content: http://www.prototypejs.org/api/string/interpolate and call show() on the html to make it visible.

msingleton