views:

129

answers:

1

Hi,

I have the following code. And my problem is that when clicking, the event is firing but the argument passed is not the correct one. It is passing the last dynamically created row i.e. dataStructure.length value. Anyone knows a solution how to get around this?

        var table = document.getElementById('output');                  

        for(i =0; i<dataStructure.length; i++){
            var row = document.createElement('tr');
            row.setAttribute('id', i);
            var url = dataStructure[i].url;

            if(document.addEventListener)
                row.addEventListener('click', function(){handleRowClick(i);}, false);
            var obj = dataStructure[i];
            var cellCount = 0;
            for(field in obj){
                var cell = document.createElement('td');
                cell.setAttribute('id', cellCount++);
                //cell.addEventListener('click', function(){window.open(dataStructureObj.links[i].url);}, false);
                cell.innerHTML = obj[field];
                row.appendChild(cell);
            }
            cellCount = 0;
            table.appendChild(row);
        }           
        }

        function handleRowClick(rowClicked){
            var rowHTML = rowClicked.innerHTML;
            var cells = rowHTML.getElementsByTagName('td');
            for(cell in cells)
            {
                alert(cell.value);
            }
            window.open(cells[1].innerHTML); 
        }
+2  A: 

i in the function(){handleRowClick(i);} is the loop variable i. When the function gets called, the loop is long-finished and i holds the last value it did at the end of the loop, not the value it held when you created the function(){}.

This is the closure loop problem. See this question for solutions using either closures or Function#bind:

row.addEventListener('click', handleRowClick.bind(window, i), false);

However, you're not really using that i at the moment. It would be easier to say:

row.addEventListener('click', handleRowClick, false);

and then use this to retrieve the row:

function handleRowClick(){
    window.open(this.cells[1].innerHTML);
}

(Side note: unfortunately if you add attachEvent backup in order to support IE<=8, it doesn't get this right, so you'd still be looking at closures/bind to get the object you want.)

also:

    for(i =0; i<dataStructure.length; i++){

Missing var: i is an accidental global.

        row.setAttribute('id', i);

Avoid getAttribute/setAttribute in HTML documents. They don't do what they should in IE. Use the DOM Level 2 HTML properties instead, they're more reliable and more readable: row.id= i.

bobince
That did the trick :) Never did this before
Ryan