18

I've tried to create a loop with for, and increment by an onclick event, but it doesn't work.

var gameCase = ['', '', '', '', '', '', '', '', ''],  // 9
    itemLists = $('game').getElementsByTagName('li'); // 9 items

for( var i = 0; i < itemLists.length; i++ ) {
     // i already equals 9
     itemLists[i].onclick = function() {
          // do something
    }
 }

But in this case, the for loop is finished before I was able to click on an element in the list.

Moreover, I would like to get the item list I clicked and save it in an array. I tried gameCase[this] (in onclick function), but I don't know if it's the good way.

MeSo2
  • 450
  • 1
  • 7
  • 18
jeremybarbet
  • 890
  • 1
  • 15
  • 28

4 Answers4

30

John Resig covers this topic very well in "Secrets of the JavaScript Ninja" ( http://ejohn.org/apps/learn/#59 )

You'll need to create a temporary scope to preserve i's value

for ( var i = 0; i < itemLists.length; i++ ) (function(i){ 
  itemLists[i].onclick = function() {
      // do something
  }
})(i);

Edit:

var gameCase = ['', '', '', '', '', '', '', '', ''], // 9
$listParent = $('game').find('ul'), // li's parent
itemLists = $('game').getElementsByTagName('li'); // 9 items

var listHandler = (function() {
  var i = 0;

  return function() {
    // $(this) will in here refer to the clicked li
    i++ // increment i

    if ( i === 9 ) {
      $listParent.off('click', 'li', listHandler); //remove eventhandler when i reaches 9
    }
  }
}());

$listParent.on('click', 'li', listHandler); // attach eventhandler to ul element

This should do what you want, cant test it right now since I'm at work.

ruuska
  • 703
  • 4
  • 9
  • As @Tim van Elsloo tell, it's the good way. But there is just something weird, when I console.log the value of the i before I click on any items of my list, the i value decrease. `for ( var i = 0; i < itemLists.length; i++ ) (function(i){ console.log(i); //0 1 2 3 4 5 6 7 8 itemLists[i].onclick = function() { // i egal to 4 if I clicked on the third element. } })(i);` – jeremybarbet Apr 07 '13 at 09:30
  • The length property starts from 1 and your i var from 0. So you'd either have to change the for loop to start from 1 and end on length+1 or change inside the function to itemLists[i+1].onclick. – ruuska Apr 07 '13 at 09:36
  • Yes, I know, that's not the problem. The interest of the Loop, is when i is egal to 9, I can't do nothing more. But right here, the For loop is already finished, before I click on an element. I would like, when I click on an item of my list, i increment, and when I arrived to 9, the loop stops. – jeremybarbet Apr 07 '13 at 09:43
  • Ok so you just want to increment i every time a listItem is clicked? In that case you dont want a for loop at all. What this does is immediately loop i from 0 to 9 and add an eventhandler to each itemlist. I'll edit my answer with what I think you need when I understand you correctly – ruuska Apr 07 '13 at 09:55
  • Yes, that's what I would like to have. – jeremybarbet Apr 07 '13 at 09:58
  • Ok, edited my answer with a script that will increment i each time a li element is clicked and when it reaches 9 removes the eventhandler. Pretty sure that I misunderstood your problem again. Would be easier if you posted a jsfiddle with your code and what exactly your trying to achieve. – ruuska Apr 07 '13 at 11:47
  • I don't use jQuery, $('') is just my own selector to document.getElementByID(''). I tried your script, it's the way I am looking for, but it doesn't work yet. Here is a little jsfiddle. http://jsfiddle.net/4Rjp9/ – jeremybarbet Apr 07 '13 at 19:50
  • Oh and for future reference you might want to look into binding the eventhandlers in a better way since DOM level 0 is pretty outdated and considered bad practice. – ruuska Apr 12 '13 at 14:06
  • I spent a week dealing with this issue. Never new anything about doing it this way. – Christian Sirolli Apr 18 '18 at 20:27
11

Wrap your listener:

onclick = (function(i) {return function() {
    ...
};})(i);

This fixes your variable scope issues.

cutsoy
  • 10,127
  • 4
  • 40
  • 57
  • That's quite good, when I cliked on a list item, it returns the good number of the case. But is it okay, that the For loop increment before I start to click ? – jeremybarbet Apr 07 '13 at 09:22
1

Sorry if I did not understand your question properly, From the code I understand that, you are trying to add an onclick handler to all the list elements in found in the game tag (perhaps it should be a class / id).

The for loop will be executed when the script tag / file loads without any user interaction.

If you wish to assign a function that uses the current value of the counter. Use the following code:

itemLists[i].onclick = (function() {
    return function() {
         // TODO ---
        // Your handler code here
}
})();
Dev Shangari
  • 336
  • 2
  • 7
1

Another option is to use a forEach loop this creates a new variable for each iteration.

var gameCase = ['', '', '', '', '', '', '', '', ''], // 9
itemLists = $('game').getElementsByTagName('li'); // 9 items

itemLists.forEach(function(item,index){
    item.onclick = function() {
      // do something
 }
});
Richard Harrison
  • 355
  • 6
  • 19