CODEWITHSUNDEEP
javascripthtmlclassonclick
Carlos Prieto
Carlos Prieto

Reputation: 21

Javascript: assign onclick inside class

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?

Upvotes: 2

Views: 828

Answers (1)

user1106925
user1106925

Reputation:

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...

  1. serialize the existing DOM nodes to HTML
  2. concatenate the new HTML fragment to the serialized nodes
  3. destroy the old nodes
  4. 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);
    };
};

Upvotes: 1

Related Questions