Issue with Javascript performance (Odin Project, Etch-a-Sketch)

124
February 17, 2022, at 10:50 PM

I'm currently learning Javascript on the Odin Project and I'm doing the Etch-a-Sketch exercise in which you're supposed to create a Etch-a-Sketch board you can draw on with your cursor. You're supposed to implement a resize button which allows the user to enter how many tiles they want on each side of the board, with a max value of 100 (for a 100x100 board). I did just that, and it technically works... except my browser starts chugging around 60 tiles, and crashed if I try to go for the max.

The exercise teaches you how to use Javascript to manipulate the DOM, so I got my script to create a div container for the entire board, and a default 16x16 board is created when starting the page. Each tile is a div with an event listener assigned to it which detects when it's hovered. A new class is given to the tile, which changes its color according to the CSS rule tied to that class.

The reset button calls a function when it's clicked. That function resets the classes for all tiles and prompts the user for a value. Once it gets a proper value, the CSS rule associated with the original div container gets updated with the requested amount of rows and columns, a function is called which removes all existing tiles, and another function fills the div container with new tiles.

HTML

<!DOCTYPE html>
<html>
<head>
  <title>Page Title</title>
  <meta charset="UTF-8"/>
  <link rel="stylesheet" href="style.css">
  <script src="script.js" defer></script>
</head>
<body>
  <button class="reset">Reset and resize</button>
</body>
</html>

CSS

body {
    display: flex;
    flex-direction: column;
}
.reset {
    align-self: flex-start;
}
.sketchContainer {
    width: 900px;
    height: 900px;
    border: solid 1px black;
    display: grid;
    align-self: center;
    grid-template-rows: repeat(16, auto);
    grid-template-columns: repeat(16, auto);
}
div.tileHovered {
    background-color: #c3a375;
}

JavaScript

generateDefaultGrid();
placeEventListenersOnTiles();
const button = document.querySelector(".reset");
button.addEventListener("click", reset);
function generateDefaultGrid() {
    const sketchContainer = document.createElement("div");
    document.body.appendChild(sketchContainer);
    sketchContainer.classList.add("sketchContainer");
    const gridTile = document.createElement("div");
    sketchContainer.appendChild(gridTile);
    gridTile.classList.add("gridTile");
    let clonedTile;
    for (let i = 0; i < 16*16-1; i++) {
        clonedTile = gridTile.cloneNode();                  //On ne peut pas copier un node en répétant les appendChild, autrement ça ne manipulera qu'une seule instance du node. IL faut le cloner.
        sketchContainer.appendChild(clonedTile);
    }
}
function placeEventListenersOnTiles() {
    let allTiles = document.querySelectorAll(".gridTile");
    allTiles.forEach(element => {
        element.addEventListener("mouseover", changeColor);
    });
}
function changeColor() {
    this.classList.add("tileHovered");                      //"this" permet de faire référence à l'élément sur lequel le listener a été placé.
}
function reset() {
    let allTiles = document.querySelectorAll(".tileHovered");
    allTiles.forEach(element => {
        element.classList.remove("tileHovered");
    });
    resizeGrid();
}
function resizeGrid() {
    let newSize = prompt("How many squares do you want per side of the Etch-a-Sketch?", "Please insert a number inferior or equal to 100.");
    if (newSize > 100) {
        newSize = prompt("Sorry, that number is too high!", "Please insert a number inferior or equal to 100.");
    } else if (Math.sign(newSize) == 0 || Math.sign(newSize) == -1) {                                                                           //Ne pas oublier que les returns de prompt sont des strings, pas des chiffres !
        newSize = prompt("Only integers between 1 and 100, please!", "Please insert a number inferior or equal to 100.");
    } else if (newSize === null) {
        generateDefaultGrid();
    } else {
        const styleSheet = document.styleSheets[0];
        styleSheet.cssRules[2].style.gridTemplateRows=`repeat(${newSize}, auto)`;
        styleSheet.cssRules[2].style.gridTemplateColumns=`repeat(${newSize}, auto)`;
        removeAllTiles();
        generateUserGrid(newSize);
    }
}
function removeAllTiles() {
    let container = document.querySelector(".sketchContainer");
    while (container.firstChild) {
        container.removeChild(container.firstChild);
    }
}
function generateUserGrid(newSize) {
    const sketchContainer = document.getElementsByClassName("sketchContainer");
    const gridTile = document.createElement("div");
    sketchContainer[0].appendChild(gridTile);
    gridTile.classList.add("gridTile");
    let clonedTile;
    for (let i = 0; i < newSize*newSize-1; i++) {
        clonedTile = gridTile.cloneNode();                  //On ne peut pas copier un node en répétant les appendChild, autrement ça ne manipulera qu'une seule instance du node. IL faut le cloner.
        sketchContainer[0].appendChild(clonedTile);
        placeEventListenersOnTiles();
    }
}

I hope I'm doing the whole asking questions right, I'm pretty new to this. I've got the current state of my project pushed to my GitHub if it helps. Can someone tell me why my script is so inefficient, and how I could go about to fix it? That sort of thing wasn't really covered in the course up to that point, I'm afraid.

Thanks a lot!

Answer 1

The performance problem is caused by placeEventListenersOnTiles(); being called within the for loop in generateUserGrid function.
So each iteration adds more and more event listeners again and again.
E.g. in 100x100 setup the first tile ends up with 10000 event listeners, the second one with 9999, and so on.

Just move it outside of the loop:

generateDefaultGrid();
placeEventListenersOnTiles();
const button = document.querySelector(".reset");
button.addEventListener("click", reset);
function generateDefaultGrid() {
  const sketchContainer = document.createElement("div");
  document.body.appendChild(sketchContainer);
  sketchContainer.classList.add("sketchContainer");
  const gridTile = document.createElement("div");
  sketchContainer.appendChild(gridTile);
  gridTile.classList.add("gridTile");
  let clonedTile;
  for (let i = 0; i < 16 * 16 - 1; i++) {
    clonedTile = gridTile.cloneNode(); //On ne peut pas copier un node en répétant les appendChild, autrement ça ne manipulera qu'une seule instance du node. IL faut le cloner.
    sketchContainer.appendChild(clonedTile);
  }
}
function placeEventListenersOnTiles() {
  let allTiles = document.querySelectorAll(".gridTile");
  allTiles.forEach(element => {
    element.addEventListener("mouseover", changeColor);
  });
}
function changeColor() {
  this.classList.add("tileHovered"); //"this" permet de faire référence à l'élément sur lequel le listener a été placé.
}
function reset() {
  let allTiles = document.querySelectorAll(".tileHovered");
  allTiles.forEach(element => {
    element.classList.remove("tileHovered");
  });
  resizeGrid();
}
function resizeGrid() {
  let newSize = prompt("How many squares do you want per side of the Etch-a-Sketch?", "Please insert a number inferior or equal to 100.");
  if (newSize > 100) {
    newSize = prompt("Sorry, that number is too high!", "Please insert a number inferior or equal to 100.");
  } else if (Math.sign(newSize) == 0 || Math.sign(newSize) == -1) { //Ne pas oublier que les returns de prompt sont des strings, pas des chiffres !
    newSize = prompt("Only integers between 1 and 100, please!", "Please insert a number inferior or equal to 100.");
  } else if (newSize === null) {
    generateDefaultGrid();
  } else {
    const styleSheet = document.styleSheets[0];
    styleSheet.cssRules[2].style.gridTemplateRows = `repeat(${newSize}, auto)`;
    styleSheet.cssRules[2].style.gridTemplateColumns = `repeat(${newSize}, auto)`;
    removeAllTiles();
    generateUserGrid(newSize);
  }
}
function removeAllTiles() {
  let container = document.querySelector(".sketchContainer");
  while (container.firstChild) {
    container.removeChild(container.firstChild);
  }
}
function generateUserGrid(newSize) {
  const sketchContainer = document.getElementsByClassName("sketchContainer");
  const gridTile = document.createElement("div");
  sketchContainer[0].appendChild(gridTile);
  gridTile.classList.add("gridTile");
  let clonedTile;
  for (let i = 0; i < newSize * newSize - 1; i++) {
    clonedTile = gridTile.cloneNode(); //On ne peut pas copier un node en répétant les appendChild, autrement ça ne manipulera qu'une seule instance du node. IL faut le cloner.
    sketchContainer[0].appendChild(clonedTile);
  }
  placeEventListenersOnTiles();
}
body {
  display: flex;
  flex-direction: column;
}
.reset {
  align-self: flex-start;
}
.sketchContainer {
  width: 450px;
  height: 450px;
  border: solid 1px black;
  display: grid;
  align-self: center;
  grid-template-rows: repeat(16, auto);
  grid-template-columns: repeat(16, auto);
}
div.tileHovered {
  background-color: #c3a375;
}
<button class="reset">Reset and resize</button>

Probably you want to go further and learn event delegation. So you'd have just 1 event listener set on the container to handle all the tiles:

generateDefaultGrid();
const button = document.querySelector(".reset");
button.addEventListener("click", reset);
function generateDefaultGrid() {
  const sketchContainer = document.createElement("div");
  document.body.appendChild(sketchContainer);
  sketchContainer.classList.add("sketchContainer");
  // SEE HERE:
  sketchContainer.addEventListener("mouseover", changeColor);
  const gridTile = document.createElement("div");
  sketchContainer.appendChild(gridTile);
  gridTile.classList.add("gridTile");
  let clonedTile;
  for (let i = 0; i < 16 * 16 - 1; i++) {
    clonedTile = gridTile.cloneNode(); //On ne peut pas copier un node en répétant les appendChild, autrement ça ne manipulera qu'une seule instance du node. IL faut le cloner.
    sketchContainer.appendChild(clonedTile);
  }
}
// AND HERE:
function changeColor(event) {
  event.target.classList.add("tileHovered");
}
function reset() {
  let allTiles = document.querySelectorAll(".tileHovered");
  allTiles.forEach(element => {
    element.classList.remove("tileHovered");
  });
  resizeGrid();
}
function resizeGrid() {
  let newSize = prompt("How many squares do you want per side of the Etch-a-Sketch?", "Please insert a number inferior or equal to 100.");
  if (newSize > 100) {
    newSize = prompt("Sorry, that number is too high!", "Please insert a number inferior or equal to 100.");
  } else if (Math.sign(newSize) == 0 || Math.sign(newSize) == -1) { //Ne pas oublier que les returns de prompt sont des strings, pas des chiffres !
    newSize = prompt("Only integers between 1 and 100, please!", "Please insert a number inferior or equal to 100.");
  } else if (newSize === null) {
    generateDefaultGrid();
  } else {
    const styleSheet = document.styleSheets[0];
    styleSheet.cssRules[2].style.gridTemplateRows = `repeat(${newSize}, auto)`;
    styleSheet.cssRules[2].style.gridTemplateColumns = `repeat(${newSize}, auto)`;
    removeAllTiles();
    generateUserGrid(newSize);
  }
}
function removeAllTiles() {
  let container = document.querySelector(".sketchContainer");
  while (container.firstChild) {
    container.removeChild(container.firstChild);
  }
}
function generateUserGrid(newSize) {
  const sketchContainer = document.getElementsByClassName("sketchContainer");
  const gridTile = document.createElement("div");
  sketchContainer[0].appendChild(gridTile);
  gridTile.classList.add("gridTile");
  let clonedTile;
  for (let i = 0; i < newSize * newSize - 1; i++) {
    clonedTile = gridTile.cloneNode(); //On ne peut pas copier un node en répétant les appendChild, autrement ça ne manipulera qu'une seule instance du node. IL faut le cloner.
    sketchContainer[0].appendChild(clonedTile);
  }
}
body {
  display: flex;
  flex-direction: column;
}
.reset {
  align-self: flex-start;
}
.sketchContainer {
  width: 450px;
  height: 450px;
  border: solid 1px black;
  display: grid;
  align-self: center;
  grid-template-rows: repeat(16, auto);
  grid-template-columns: repeat(16, auto);
}
div.tileHovered {
  background-color: #c3a375;
}
<button class="reset">Reset and resize</button>

Rent Charter Buses Company
READ ALSO
Unable to get sass file to override existing stylesheet

Unable to get sass file to override existing stylesheet

I have a sass file that I want to override the existing stylesheet but I'm unable to see the sass override when I inspect in the browserI included a screen shot that doesn't show the design system and override:

150
How to change start point of animation in CSS?

How to change start point of animation in CSS?

I want to change the start point of the SVG path animationI am a beginner in SVG paths, so might be a dumb question

138
Making an image stay at the bottom of the page but not go over content

Making an image stay at the bottom of the page but not go over content

I have an a logo image that is supposed to remain at the bottom of a nav drawerThe only way I know to have it be at the bottom is with the absolute and bottom properties, but that's causing it to overlap the list items on a short device (shrink the height...

120
How to enable a select dropdown element once the first dropdown element value has been chosen (with CSS)

How to enable a select dropdown element once the first dropdown element value has been chosen (with CSS)

I've seen multiple ways involving JS/Jquery that allow enabling a second select element (from a disabled state) once the first select element option has been chosen, but I am wondering if anyone can share how to accomplish this with CSS

144