5

I'm writing some very simple code to dynamically change image src on mouseover/mouseout:

   function e(id) {
     return document.getElementById(id);
   }

   function changeimg_bw(ele) {
      e(ele).src='rating_bw.png';
   }

   function changeimg_color(ele) 
      e(ele).src='rating_color.png';
   }

   for(var i=1;i<=5;i++) {
     var img ='rating'+i;
     e(img).addEventListener('mouseover', function(event) {
          changeimg_color(img);
     });
     e(img).addEventListener('mouseout', function(event) {
          changeimg_bw(img);
     });
   }

The idea is fairly simple: use an array of images to simulate the rating bar. And when some image is covered by mouse pointer, it should change color (well ideally including all previous images should change color but I got stuck before getting there). My question is when I hover on any image, only the last image changes color ('rating5'). Looks when i == 5 its eventlistener overwrote all other eventlistener (i=1,2,3,4)?

Yandong Liu
  • 335
  • 4
  • 13
  • 2
    `img` inside the anonymous listener functions refers to the `var` in the surrounding scope (which holds the last assigned value: `rating5`.) Try and replace `changeimg_color(img);` with `changeimg_color(this.id);` or just pass `this` (the image spawning the mouse events) to the `change*` functions omitting the `e()` wrapper. – jensgram Jul 09 '13 at 07:24

4 Answers4

2

Easiest way to delegate event... In such way you don't need to add listeners per element

Live Demo

var parent = document.getElementById("rating1").parentNode;

parent.addEventListener("mouseover", changeimg_color, false);
parent.addEventListener("mouseout", changeimg_bw, false);

function changeimg_bw(e) {
    if (e.target.nodeName.toLowerCase() === "img") {
        e.target.src='rating_bw.png';
    }
    e.stopPropagation();
    e.preventDefault();
}

function changeimg_color(e) {
    if (e.target.nodeName.toLowerCase() === "img") {
        e.target.src='rating_color.png';
    }
    e.stopPropagation();
    e.preventDefault();
}
Givi
  • 1,674
  • 2
  • 20
  • 35
2

Since javascript don't have block scope but function scope, the img within the anonymous listener functions will refer to the last value.
You could solve this by simply encloses the listeners to a private closure like

for (var i = 1; i <= 5; i++) {
    var img = 'rating' + i;
    (function (img) {
        e(img).addEventListener('mouseover', function (event) {
            changeimg_color(img);
        });
        e(img).addEventListener('mouseout', function (event) {
            changeimg_bw(img);
        });
    })(img);
}

DEMO

For better understanding about closures read this

Community
  • 1
  • 1
code-jaff
  • 9,230
  • 4
  • 35
  • 56
1

In JS you can add properties to any object at run time. Using this behavior you can do something like this...

for(var i=1;i<=5;i++) {
    var img ='rating'+i;
    e(img).index = i;
    e(img).addEventListener('mouseover', function(event) {
        changeimg_color("rating" + event.target.index);
    });
    e(img).addEventListener('mouseout', function(event) {
        changeimg_bw("rating" + event.target.index);
    });
}
mohkhan
  • 11,925
  • 2
  • 24
  • 27
1

You could simply add your listeners in a custom function:

function addImgListeners(img) {
    e(img).addEventListener('mouseover', function(event) {
        changeimg_color(img);
    });
    e(img).addEventListener('mouseout', function(event) {
        changeimg_bw(img);
    });
}

Then:

for(var i=1; i<=5; i++) {
    var img = "rating" + i;
    addImgListeners(img);
    // or even addImgListeners("rating" + i);
}

Demo

sp00m
  • 47,968
  • 31
  • 142
  • 252