1

I have a problem - I want to add an .onClick event to each cell of a dynamically generated table while I am creating it.

cell.onclick = function(){                
        mouseClick(row_number,i, cell);                               
    };

However, it appears that the event is just added to the last cell in each row. How is it possible? Is it somehow overriding the previous ones? And how to make it work?

function drawRow(rowData, length, keys, row_number){   
 //rowData - object where the row data is stored, length-row length, keys - object with table keys stored, row_number - number of the row
    var rowData=rowData;
    var row = document.createElement("tr");    
    for(i=0; i<length; i++){    
        var cell = document.createElement("td");
        console.log("Creating event for "+i+" .");
        cell.onclick = function(){                
            mouseClick(row_number,i, cell);                               
        };
        var key = keys[i];       
        var cell_text = document.createTextNode(rowData[key]);        
        cell.appendChild(cell_text);
        row.appendChild(cell);
    }  
    return row;
}
Vertexwahn
  • 7,709
  • 6
  • 64
  • 90
Rionick
  • 11
  • 2
  • Welcome to the horribly inconvenient world of closures ;) – Nick Zuber May 30 '16 at 23:03
  • `i` has a global scope (as far as I can tell — this is bad practice, by the way) and `row_number` and `cell` are scoped to outer functions, so they will be the same for all cells. You could [fix it with a closure](http://stackoverflow.com/q/750486/4642212), but I would strongly recommend **event delegation** instead. – Sebastian Simon May 30 '16 at 23:03
  • This question is a duplicate of this one http://stackoverflow.com/questions/1451009/javascript-infamous-loop-issue See answer there for one possible solution and an explanation of javascripts infamous loop-closure issue. –  May 30 '16 at 23:07

1 Answers1

1

You are seeing one of the most common problems in JavaScript-land.

Your onclick functions are not called until after that for-loop is done. When the for-loop is done, the value of i is that of length. Only then do you try to read it. In other words, every single one of your callbacks say

function () {mouseClick(row_number, i, cell}

By the time you finally execute the callback, the value of i is its last value.

If you are using ES6, instead write

for (let i=0; i<length; i++)

The let gives each invocation of the block its own variable i. In your code you only have one i shared among all the cells. That is why all cells are appearing to do the same thing!

If you are still in the ES5 game, replace the inner portion of your code with

    cell.onclick = (function (j) {
        return function () {
            mouseClick(row_number, j, cell); 
        }                              
    })(i);

This gives to each of your callback functions a copy of i at each point of the loop. The first time through the loop, i has a value of 0, and the callback you get is

function () {mouseClick(row_number, 0, cell}

The second iteration of the loop produces

function () {mouseClick(row_number, 1, cell}

and so on!

Pretty cool...but the ES6 way with let does the same thing, a bit more nicely.

(Alternatively, there are ways to do this thing with the forEach method on arrays. These also ensure the loop iterations do not share the index variable.)

Ray Toal
  • 86,166
  • 18
  • 182
  • 232
  • Nice answer Ray - mind adding an ES5 solution as well? ES6 isn't fully supported yet, and there's a good chance OP isn't using ES6 – Nick Zuber May 30 '16 at 23:05