Javascript Dynamic Table with each cell having an onmouse event?

Question

I've created a dynamic table using Javascript. Now what I'm trying to do is for each cell that is dynamically generated, there is an onmouseover event that would change that particular cell's backgroundColor.

The problem I have is that when I generate the table and try to have an onmouseover function with each dynamically generated cell the function only works for the last generated cell.

Here's a copy of my code. (Note: I have only tested this on Chrome)

<!DOCTYPE html>
<html>
<head>
<meta charset="ISO-8859-1">
<title>Insert title here</title>
<style>
 table, th, td {
  border: 1px solid black;
  border-collapse: collapse;
  padding: 5px;
  text-align: center;
 }
</style>
<script type="text/javascript">
 var table;
 
 function init(){
  table = document.getElementById("mytable");
 }
 
 function makeCells(){
  init();
    
  for(var a=0;a<20;a++){
   var row = table.insertRow(-1);
   
   for(var b=0;b<20;b++){
    cell = row.insertCell(-1);
    cell.innerHTML = a*b;
    cell.onmouseover = function(){cell.style.backgroundColor = "yellow";};
   }
  }
 }
</script>
</head>
<body onload="javascript: makeCells();">
 <table id="mytable"></table>
</body>
</html>

Any advice would be greatly appreciated.

http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example — Farzher, Mar 28 '15 at 04:13
Your problem is that `cell` always refers the last cell. It's a general `closure` problem. — Farzher, Mar 28 '15 at 04:14

score 0 · Answer 1 · answered Mar 28 '15 at 04:15

0

I actually found the answer myself playing around with my code.

In the line:

cell.onmouseover = function(){cell.style.backgroundColor = "yellow";};

I changed it to:

cell.onmouseover = function(){this.style.backgroundColor = "yellow";};

answered Mar 28 '15 at 04:15

primetimeCanuck

5
3

score 0 · Accepted Answer · answered Mar 28 '15 at 04:46

Some Improvements. 3 things I would change:

Don't edit inline styles with javascript. Instead, add or remove a class. see # 3.
Don't do so many event handlers in your "onload", "onmouseover". Its better to add an event listener.
It is better performance to add all of the new elements at one time instead of individually. See this article: https://developers.google.com/speed/articles/reflow

Here is a way to optimize the Javascript:

HTML

<table id="table"></table>

CSS

body {
  padding: 40px;
}
.yellow {
  background: yellow;
}
td {
    padding: 10px 20px;
    outline: 1px solid black;
}

JavaScript

    function propegateTable() {
      var table = document.getElementById("table");

      //will append rows to this fragment
      var fragment = document.createDocumentFragment();

      for(var a=0; a<10; a++){ //rows
          //will append cells to this row
          var row = document.createElement("tr");

          for(var b=0;b<5;b++){ //collumns
            var cell = document.createElement("td");
            cell.textContent = a + ", " + b;
            // event listener
            cell.addEventListener("mouseover", turnYellow);
            row.appendChild(cell);
          }

          fragment.appendChild(row);
        }
      //everything added to table at one time
      table.appendChild(fragment);
    }

    function turnYellow(){
      this.classList.add("yellow");
    }

    propegateTable();

http://codepen.io/ScavaJripter/pen/c3f2484c0268856d3c371c757535d1c3

Thanks scavaJripter! I'll try out this way. I'm just learning html,css and javascript so I'm not that well versed yet.. — primetimeCanuck, Mar 28 '15 at 05:16
keep it up.. Just try to get javascript knowledge from MDN and not w3schools — Sam Eaton, Mar 28 '15 at 05:27

Javascript Dynamic Table with each cell having an onmouse event?

2 Answers2