my "click" addEventListener does not work well

Question

I'm using the following code to add listeners to a (that I append to a TBODY in the table) so i can see the nice highligh effect when I put my mouse over the TR. I also have a "click" event, that makes the whole row "clickable" so user can click easily on the whole row and go to that particular page.

I'm using JS, because I use AJAX call to populate the TR's (and their few TD's).

First 12 elements in my list are done nicely with PHP, but after that i load next 10, 10, 10...by Ajax call.

When I use PHP and add javascript: to the TR with "onClick" it works perfectly, but in this code below (javascript), only the mouseover and mouseout work fine, the "click" event listener adds to all window.location.href's the last value of i, insted of current value (13, 14, 15)...it adds to all only 15 (so always, last value of i - my counter...it does not increment as the counter does).

I think there's something wrong regarding how the event listener functions, how it initialises, something I do not know.

             for(i=0; i<10; i++){

                    myTr.addEventListener("mouseover",function(){
                        this.style.backgroundColor = "#083636"
                        this.style.cursor = "pointer"
                    });

                    myTr.addEventListener("mouseout",function(){
                        this.style.backgroundColor = "transparent"
                    });

                    myTr.addEventListener("click",function(){
                        window.location.href = '/clubbers/' + clubber_url + '/' + i + '#threads'
                    });

             }

*forgot to start my for bracket when typing/copying the code, now it looks good

Can you please review your code, you didn't close the bracket... — Pierre, Aug 03 '13 at 18:02
You repeat the loop ten times, and it sets the window.location.href value accordingly, thus you end up with only the last value sent. — Whistletoe, Aug 03 '13 at 18:04
also you do not need mouseover and mouseout for changing color, you can just add tr:hover{background-color:#083636; cursor:pointer} to your css — Desu, Aug 03 '13 at 18:04

score 1 · Answer 1 · edited May 23 '17 at 10:27

1

Your problem is that the variable i from the outer for loop isn't captured properly by the event, because by time the event is evaluated the loop already finished, and the variable is at it's last value.

look at this post for some solutions:

Caputured variables in Javascript

edited May 23 '17 at 10:27

Community

1
1

answered Aug 03 '13 at 18:04

zewa666

2,593
17
20

score 1 · Accepted Answer · edited May 23 '17 at 12:23

1

It's a scope issue. Basically you're passing i to your eventListener, which is the same variable being incremented by the loop. See How do JavaScript closures work? for a better explanation

You can just do:

myTr.addEventListener("click",(function(j){
  return function(e) {
    window.location.href = '/clubbers/' + clubber_url + '/' + j + '#threads'
  }
})(i));

edited May 23 '17 at 12:23

Community

1
1

answered Aug 03 '13 at 18:05

axelcdv

753
6
18

your function makes my code jump immediately to the 1st post i load with AJAX, meaning i load 1st 12 rows with PHP, normally, and then when i start using the Javascript function, AJAX triggers the code where i put my event listeners, and as soon as I trigger the AJAX code, it "clicks" on the 1st row loaded...automatically somehow – Adrian Tanase Aug 03 '13 at 18:15
Whoops you're right, my code wasn't correct, it should be now – axelcdv Aug 03 '13 at 18:18
works like a charm! thanks for being there for us on saturday evenings :)) – Adrian Tanase Aug 03 '13 at 18:22

score 1 · Answer 3 · answered Aug 03 '13 at 18:15

function clickTr(i){
    return function(){
        window.location.href = '/clubbers/' + clubber_url + '/' + i + '#threads';
    }
}
for(i=0; i<10; i++){

                myTr.addEventListener("mouseover",function(){
                    this.style.backgroundColor = "#083636"
                    this.style.cursor = "pointer"
                });

                myTr.addEventListener("mouseout",function(){
                    this.style.backgroundColor = "transparent"
                });

                myTr.addEventListener("click",clickTr(i));

         }

i think your solution works too mate, just got it from the above answer, but your answer seems more like I do the code :) reading easily and simple :) thanks! — Adrian Tanase, Aug 03 '13 at 18:23

my "click" addEventListener does not work well

3 Answers3