Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Onclick adds to last element in loop only [duplicate]

Tags:

javascript

Possible Duplicate:
Assign click handlers in for loop

I need help with a loop in my code.

I loop through an array and add on clicks to divs. But it always adds onclicks to the last cycle through the loop and effectively cancels out the ones before it.

So i have this as my loop:

    start = 0;

for(i = start; i < (start+8); i++){ //8 per page
    if(i == array.length){ 
        break;  //page end
    } else {
        (function(i){
             document.getElementById('cell'+i).onclick = function(){ select(i); }
        })(i);  
    }
}

What occurs here is div id cell7 gets the on click added, but the div ids cell0 to cell6 do not. I'm guessing its something to do with the fact that i changes in the loop and so is also effected the i in the function ?

How do I fix this problem ?

like image 833
Sir Avatar asked Oct 09 '12 03:10

Sir


3 Answers

You're close:

(function(i){
    document.getElementById('cell' + i).onclick = function(){ select(i); }
})(i); 

I think what you want is:

document.getElementById('cell'+i).onclick = (function(i){
    return function(){ select(i); }
})(i); 

Also, because you have:

for(i = start; i < (start+8); i++)

i is not declared, so it becomes global. Declare 'i' to keep it local (presuming this is function code):

for (var i = start; i < (start+8); i++)

If it's global code it won't make any difference (but declare it anyway).

Edit

The above doesn't fix your issue, it just changes the syntax.

The following "works" for me, cell0 to cell7 get a listener, the rest don't. select shows the correct value (0 to 7 depending on which one is clicked on).

<script>
    function addListener() {
      var start = 0;
      for(i = start; i < (start+8); i++){ 
        document.getElementById('cell'+i).onclick = (function(i){
            return function(){ select(i); }
        })(i);  
      }
    }
    function select(arg) {
      alert(arg);
    }
    window.onload = addListener;
</script>

<p id="cell0">0</p>
<p id="cell1">1</p>
<p id="cell2">2</p>
<p id="cell3">3</p>
<p id="cell4">4</p>
<p id="cell5">5</p>
<p id="cell6">6</p>
<p id="cell7">7</p>
<p id="cell8">8</p>
<p id="cell9">9</p>
like image 79
RobG Avatar answered Nov 16 '22 19:11

RobG


Try this example. var i = 1 assuming your HTMLElement id starts at 1. Change to reflect the situation.

var select = function(i) {
    console.log(i);
};

var arr = ["1", "2", 3, 4, 5, 6];

var start = 0;
var max = 8;

for (var i=1; i<(start + max); i++) {
    if (i === arr.length) {
        break;
    } else {
        (function(i) {
            var element = document.getElementById(['cell', i].join(''));
            element.onclick = function() {
                 select(i);  
            };
        })(i);
    }
}

​ Calling an external function should be a faster implementation, however.

like image 2
krg Avatar answered Nov 16 '22 19:11

krg


I'm not sure why but these suggestions didn't fix my problem - but i managed to find a way around it by adding to the onclicks as i display the divs during the loop so i ended up doing:

innerHTML = '<Div onclick="">'; //etc etc

Instead of adding the divs to the output "then" assigning the onclicks later... don't know if my new method is the best choice but it works so i'll have to stick to it!

like image 1
Sir Avatar answered Nov 16 '22 17:11

Sir