Javascript: assign onclick inside class

Question

I created a constructor that will handle a custom list control. I created a method in order to allow the user to add elements to the list, and I need to assign event handlers to the click events of the list elements (divs).

A simplified version of the code is here. The list elements are created using the innerHTML property and a string template upon which I substitute specific parts. Later I get the element by it's id and assign it a function in closure:

function prueba(){
    var plantilla = '<div id="«id»">«texto»</div>';

    var f = function(nombre){
        return function(){console.log('mi nombre es ' + nombre)};
    };

    this.agregar = function(id, texto){
        var tmp = plantilla.replace('«id»', id);
        tmp = tmp.replace('«texto»', texto);
        document.body.innerHTML += tmp;

        document.getElementById(id).onclick = f(id);
    };
};

The problem is that, apparently, the event handler is unasigned to previous created divs, so is only retained by the last one, as it can be tested with the following code:

var p = new prueba;

p.agregar('i1', 'texto1');
console.log(document.getElementById('i1').onclick.toString());//shows the function code
p.agregar('i2', 'texto2');
console.log(document.getElementById('i2').onclick.toString());//shows the function code
console.log(document.getElementById('i1').onclick.toString());//returns 'null' error
p.agregar('i3', 'texto3');
console.log(document.getElementById('i3').onclick.toString());//shows the function code
console.log(document.getElementById('i2').onclick.toString());//returns 'null' error

This happens in Iceweasel as well as in Chromium. It does NOT happen when I add 'onclick = f(«id»)' in the template (which I cannot do here because of the assigned function scope), and neither happens if I use document.createElement. What am I doing wrong?

2 revs user1106925 · Accepted Answer · 2015-06-30 19:55:42Z

1

You destroy elements previously created when you do this:

document.body.innerHTML += tmp;

Instead use insertAdjacentHMTL() if you want to append using HTML markup.

document.body.insertAdjacentHTML("beforeend", tmp);

Now instead of going through this destructive process...

serialize the existing DOM nodes to HTML
concatenate the new HTML fragment to the serialized nodes
destroy the old nodes
recreate the nodes with the new nodes

...it simply creates the new content and places it before the close of the body element.

Basically, remove element.innerHTML += ... from your coding practices. It's never necessary, it's inefficient and it causes problems like what you've described.

FYI, the .insertAdjacentHTML() method receives 4 different string possibilities as the first argument. Each one designates a position relative to the element on which you're calling it.

The strings are...

"beforebegin"
"afterbegin"
"beforeend"
"afterend"

The labels are pretty self-explanatory. They position the new content before the current element, inside the current element at the beginning, inside the current element at the end, or after the current element, respectively.

Your full code will look like this, which I shortened a bit too since the tmp really isn't needed here:

function prueba(){
    var plantilla = '<div id="«id»">«texto»</div>';

    var f = function(nombre){
        return function(){console.log('mi nombre es ' + nombre)};
    };

    this.agregar = function(id, texto){
        document.body.insertAdjacentHTML("beforeend",
                                             plantilla.replace('«id»', id)
                                                      .replace('«texto»', texto));

        document.getElementById(id).onclick = f(id);
    };
};

edited Jun 30, 2015 at 19:55

community wiki

2 revs
user1106925

Sign up to request clarification or add additional context in comments.

3 Comments

Carlos Prieto Over a year ago

insertAdjacentHTML indeed solves the problem, thanks!. I just have a further doubt: you say that this destroys the elements previously created, but the only thing that is destroyed is the function assigned to them (I actually see the new divs with their corresponding text) Is the function considered a new element?

user1106925 Over a year ago

@CarlosPrieto: It does destroy the elements as well as anything associated with them, like event handler functions. It then recreates them. This is very inefficient. Say you have a total of 100 DOM nodes in the body. What happens is that the JS engine needs to traverse through every node, convert them to a string of HTML, add your new HTML to the end of that string, destroy the old nodes, parse the HTML you just created, create new nodes and put them in the body. So yes, it basically destroys them and then recreates them, which is wasteful.

user1106925 Over a year ago

...there may be some optimizations to make it slightly more efficient than that, but who knows. The principle is destructive, and definitely wipes out event handlers and other stuff, so the .insertAdjacentHTML helps immensely.

Collectives™ on Stack Overflow

Javascript: assign onclick inside class

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related