primetimeCanuck
primetimeCanuck

Reputation: 5

Javascript Dynamic Table with each cell having an onmouse event?

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.

Upvotes: 0

Views: 1033

Answers (3)

KooiInc
KooiInc

Reputation: 122966

It is generally not a good idea to use inline event handlers.

Using event delegation (ED) you only need to assign a handler once on a level higher up the DOM tree (often document or document.body). A further advantage of ED may that you can assign handler(s) before the table rows/cells exist.

In the example the mouseover is replaced with the css :hover pseudo class.

A click handler is assigned for the table cells using the aforementioned ED.

document.addEventListener(`click`, handle);
makeCells(document.querySelector(`#mytable`));

function handle(evt) {
  // only handle when the target is a table cell
  // AND it's a cell from table#mytable
  if (evt.target.closest(`td`).closest(`#mytable`)) {
    evt.target.classList.toggle(`selected`);
  }
}

function makeCells(forTable) {
  for (let a = 0; a < 20; a +=1 ) {
    forTable.insertRow(-1);
  }
  
  [...forTable.rows].forEach( (row, i) => {
    for (let b = 0; b < 10; b += 1) {
      row.insertAdjacentHTML(`beforeend`, `<td>r${i}/c${b}</td>`);
    }
  })
}
table,
th,
td {
  border: 1px solid black;
  border-collapse: collapse;
  padding: 5px;
  text-align: center;
  cursor: pointer;
}

td:hover {
  color: blue;
}

td.selected {
  background-color: #c0c0c0;
}
<table id="mytable"></table>

Upvotes: 0

Sam Eaton
Sam Eaton

Reputation: 1835

Some improvements. 3 things I would change:

  1. Don't edit inline styles with javascript. Instead, add or remove a class. see # 3.

  2. Don't do so many event handlers in your "onload", "onmouseover". Its better to add an event listener.

  3. 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++){ //columns
            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

Upvotes: 0

primetimeCanuck
primetimeCanuck

Reputation: 5

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";};

Upvotes: 0

Related Questions