javascript events being applied to wrong elements

Question

I'm looping through a counter, finding elements based on the counter value, and binding events to that set that matches the counter.

psuedo code:

for index in counter
    find elements of data attribute matching counter
    attach click event to that matched query set

js:

var groups = [];

$('.match-me').each(function(){
    var group_id = $(this).data('instance');
    if(groups.indexOf(group_id) < 0){
        groups.push(group_id);
    }
});

for(var i of groups ){
    window['test'+i] = $('*[data-instance=' + i + ']');
    window['test'+i].each(function(){
        $(this).on('click',function(){


            //CONSOLE.LOG OF window['test'+i] OUTPUTS THE LAST MATCHED SET


            window['test'+i].css('color','red');
        });
    });
}

The issue is inside where I am binding the event listening, the variable for the matched set of elements is always the last matched set of elements. I thought that by creating dynamic global variables in window I would escape this problem, but I'm not sure what I'm doing wrong.

Why would you assign things to the window? Globals are a bad idea. `$('*[data-instance]').on("click", function () { $(this).css('color','red');} );` — epascarello, Oct 26 '15 at 18:28
@epascarello I believe that it was an attempt to get around the issue of only referencing the last variable. You should be able to take the answer below, and replace the `window[` instances with `var etc`, to get the same effect. (No need to try naming the variables differently by number - they will be different instances per closure execution) — Katana314, Oct 26 '15 at 18:30
@epascarello Because I need to create a dynamic variable name or that variable will be overwritten on each iteration. — smilebomb, Oct 26 '15 at 18:44
Don't use globals. Make one global and assign to it if you really think that is what you need to do. `var myObj = {}; myObj['test'+i] = "foo";`Seems like there should be a better solution than what you are doing. — epascarello, Oct 26 '15 at 18:47

score 2 · Accepted Answer · answered Oct 26 '15 at 18:27

When the click event happens, i has already been incremented to the last element.

Use a closure:

for(var i of groups ){
    window['test'+i] = $('*[data-instance=' + i + ']');
    window['test'+i].each(function(){
         (function(i) {
              $(this).on('click',function(){
                window['test'+i].css('color','red');
              });
          })(i);
     });
}

javascript events being applied to wrong elements

1 Answers1