0

I'm writing a small JavaScript game.

The code I'm looking at is this bit:

cells[i].onmouseover = function() {
        cells[i].style.top =
        (cells[i].getAttribute('data-top') - 25).toString() + "px";
    }

Each cells[i] element is a member of an array of <img> elements.

Currently, whenever I hover over an img element it generates the following error:

Uncaught TypeError: Cannot read property 'style' of undefined

I can't work out what's going on. Why is cells[i] undefined?

Here's all the potentially relevant code:

for(var i = 0; i < mapSize; ++i) {
    cells[i] = document.createElement('img');
    cells[i].src = 'base1.png';
    cells[i].class = 'base1';
    cells[i].id =  'cell' + i;

    game.appendChild(cells[i]);

    row = Math.floor(i / mapWidth);

    top = tops[i%mapWidth + row];
    left = lefts[mapWidth + (i % mapWidth) - row];

    cells[i].setAttribute('data-top',top);
    cells[i].setAttribute('data-left',left);

    cells[i].style.top = top.toString() + "px";
    cells[i].style.left = left.toString() + "px";
    cells[i].style.zindex = i;

    console.log(cells[i]);

    cells[i].onmouseover = function() {
        cells[i].style.top =
        (cells[i].getAttribute('data-top') - 25).toString() + "px";
    }
}
JimmyM
  • 360
  • 3
  • 20
  • 1
    Read the dup to know what's happening, but I suggest you use event delegation. – elclanrs Apr 03 '14 at 22:34
  • Thank you! This is interesting. – JimmyM Apr 03 '14 at 22:35
  • 2
    Please note that `onclick` will only support a single event function. `addEventListener` allows multiple events to be attached to an element. – Sterling Archer Apr 03 '14 at 22:38
  • 1
    The duplicate is somewhat relevant, but really addresses a different situation. The solutions presented there are not what you would use here. In your case, the element to which the handler is bound will be available as the `this` value, so if that was the only reason for using the `i` value, then instead you'd just do this: `this.style.top = (this.getAttr...` – cookie monster Apr 03 '14 at 22:43
  • Thank you cookiemonster, that's perfect! I was actually pretty close to that, but for some reason was trying alternate versions of this and cells[i], which obviously didn't work. In other words, conceptually I was still miles away from your answer! Thanks a lot, really appreciate it - the dupe question was really cool as well. – JimmyM Apr 03 '14 at 22:46
  • 1
    @JimmyM: You're welcome. And yeah, the other Q/A is certainly worth remembering. It's almost guaranteed to be relevant at some point. – cookie monster Apr 03 '14 at 22:47

1 Answers1

-3

This should work...

cells.each(function(key, index){
    var _this = $(this);
    _this.mouseover(function() {
        var top = _this.data("top") - 25;
        $(this).css("top",top);
    });
})
hjuster
  • 3,985
  • 9
  • 34
  • 51