Etch a Sketch project: problem with event handlers

#1

Hey all,

I have the basic functions down for the Etch A Sketch Project (can switch to color, opacity mode, reset grid, etc.).

Problem: each button / event handler only works once and the modes won’t “overwrite” each other. For instance if I go to grayscale and then color, the color mode will be at the grayscale mode opacity and not full opacity. Once I switch to grayscale or color mode, I cannot switch back, and resetting the grid doesn’t change it back either.

Does this have to do with the way event listeners / handlers override each other? Do I need to remove them upon clicking a different mode? I’ve tried removeEventListener and various other methods but am still stuck.

Still have to clean up my code a bit but if there’s other errors you see, I welcome feedback on that too! (besides stuff like arrow functions etc, I want to write everything out for now)

Thanks a lot!

HTML:

<!DOCTYPE html>

<head>
<title>Etch A Sketch</title>
<link rel="stylesheet" href="main.css">
<link href="https://fonts.googleapis.com/css?family=Gloria+Hallelujah" rel="stylesheet">
</head>

<body>
  <div class="title">
    <h1>Etch A Sketch</h1>
  </div>
  <div class="container">
    <div class="menu">
      <button class="button" id="reset">reset</button>
      <button class="button" id="black">black</button>
      <button class="button" id="grayscale">grayscale</button>
      <button class="button" id="technicolor">technicolor</button>
    </div>
    <div class="grid" id="grid">
    </div>
</div>

<script type="text/javascript" src="main.js">
</script>

</body>

CSS:

* {
  font-family: "Helvetica";
  text-align: center;
}

h1 {
  font-family: "Gloria Hallelujah", cursive;
}

h1:hover {
  transform: scale(1.1);
  transform: rotateZ(5deg);
}

.title {
  width: 30%;
  margin: auto;
  border: 1px solid black;
  box-shadow: 5px 10px #000;
}

.button {
  background-color: rgb(164, 236, 212);
  border-radius: 10px;
  font-size: 16px;
  color: white;
  padding: 10px;
  margin-bottom: 1em;
}

.container {
  width: 70%;
  margin: auto;
  padding: 2em;
}

.menu {
  display: inline;
  margin: auto;
  padding: 2em;
}

#grid {
  width: 600px;
  margin: auto;
  border: 1px solid black;
}

.grid-cell {
  display: inline-block;
}

JS:

const grid = document.getElementById("grid")
const menu = document.querySelector('.menu')
const cells = document.querySelectorAll('div.grid-cell')
const black = document.getElementById("black")
const grayscale = document.getElementById("grayscale")
const technicolor = document.getElementById("technicolor")
const reset = document.getElementById("reset")

// create grid
function createGrid(rows, cols) {
  for (let r = 0; r < rows; r++) {
      let row = document.createElement('div');
      row.className = "grid-row";
      row.style.height = (600 / cols) + "px";
    for (let c = 0; c < cols; c++) {
      let cell = document.createElement('div');
      cell.className = "grid-cell";
      cell.style.width = (600 / cols) + "px";
      cell.style.height = (600 / cols) + "px";
      row.appendChild(cell);
    }
    grid.appendChild(row);
    grid.addEventListener("mouseover", drawBlack);
  }
  return grid;
}

window.onload = createGrid(16, 16);

// button event listeners
menu.addEventListener("click", function(e) {
  if (e.target.id == "black") {
    grid.addEventListener("mouseover", drawBlack);
  }
  if (e.target.id == "grayscale") {
    grid.addEventListener("mouseover", drawGrayscale);
  }
  if (e.target.id == "technicolor") {
    grid.addEventListener("mouseover", drawTechnicolor);
  }
  if (e.target.id == "reset") {
    resetGrid();
  }
})

// black highlight
function drawBlack(e) {
  if (e.target.className === "grid-cell") {
    e.target.style.background = "#000";
  }
  console.log(e.target.className)
}

// grayscale highlight
function drawGrayscale(e){
  if (e.target.className === "grid-cell") {
    e.target.style.opacity = `${Number(e.target.style.opacity) + 0.1}`
  }
}

// randomized technicolor highlight
function getColor(){
  let letters = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, "a", "b", "c", "d", "e", "f"]
  let color = "#"
  for (let i = 0; i < 6; i++) {
    let randomCode = letters[Math.floor(Math.random() * letters.length)]
    color = color + randomCode;
  }
  return color;
}

function drawTechnicolor(e){
  if (e.target.className === "grid-cell") {
    e.target.style.background = getColor();
  }
  console.log(e.target.className)
}

// reset grid
function resetGrid(){
  let squares = prompt("How many squares in the new grid?", "");
  if (squares % 1 != 0) {
    alert("Please enter a valid integer.")
  }
  else {
    grid.innerHTML = "";
    createGrid(squares, squares);
  }
}

CodePen:

#2

Can you paste your css too. Sometimes it’s easier to put the code in a pen and have a proper look at it.

#3

I had a look

menu.addEventListener("click", function(e) {
      if (e.target.id == "black") {
        grid.removeEventListener("mouseover", drawGrayscale);
        grid.removeEventListener("mouseover", drawTechnicolor);
        grid.addEventListener("mouseover", drawBlack);
      }
      if (e.target.id == "grayscale") {
        grid.removeEventListener("mouseover", drawBlack);
        grid.removeEventListener("mouseover", drawTechnicolor);
        grid.addEventListener("mouseover", drawGrayscale);
      }
      if (e.target.id == "technicolor") {
        grid.removeEventListener("mouseover", drawGrayscale);
        grid.removeEventListener("mouseover", drawBlack);
        grid.addEventListener("mouseover", drawTechnicolor);
      }
      if (e.target.id == "reset") {
        resetGrid();
      }
    })

This does seem to work which removes the other possibly event listeners but it feels a bit clunky. As far as I am aware there is no obvious way to remove all event listeners in one command.

There are some other small issues, like adding black after grayscale means the opacity doesn’t get reset etc so it can still look grayscale. That’s something you’ll have to consider.

#4

Hey, thanks for the suggestion. Just put the project up in Codepen and added the link.

And that does indeed work, thank you!

Another thing I thought of was doing a page refresh upon reset, but that doesn’t sound optimal. I’ve looked at others’ code to see what they did. Many didn’t use removeEventListener at all but used for loops under each event listener to color the grid, which works with the overwriting. Maybe I’ll look into completely restructuring the logic of my code to explore alternatives too…

#5

Personally I am not a fan of using loops to redraw the grid everytime.

One thing you could do is have a function that takes an array of functions and removes them.

So instead of this

menu.addEventListener("click", function(e) {
      if (e.target.id == "black") {
        grid.removeEventListener("mouseover", drawGrayscale);
        grid.removeEventListener("mouseover", drawTechnicolor);
        grid.addEventListener("mouseover", drawBlack);
      }
      if (e.target.id == "grayscale") {
        grid.removeEventListener("mouseover", drawBlack);
        grid.removeEventListener("mouseover", drawTechnicolor);
        grid.addEventListener("mouseover", drawGrayscale);
      }
      if (e.target.id == "technicolor") {
        grid.removeEventListener("mouseover", drawGrayscale);
        grid.removeEventListener("mouseover", drawBlack);
        grid.addEventListener("mouseover", drawTechnicolor);
      }
      if (e.target.id == "reset") {
        resetGrid();
      }
    })

You have this

function removeEventListeners(eventType, ...fns) {
  fns.forEach(fn => {
    grid.removeEventListener(eventType, fn);
  })
}

// button event listeners
menu.addEventListener("click", function(e) {
  removeEventListeners("mouseover", drawGrayscale, drawTechnicolor, drawBlack)
  if (e.target.id == "black") {
    grid.addEventListener("mouseover", drawBlack);
  }
  if (e.target.id == "grayscale") {
    grid.addEventListener("mouseover", drawGrayscale);
  }
  if (e.target.id == "technicolor") {
    grid.addEventListener("mouseover", drawTechnicolor);
  }
  if (e.target.id == "reset") {
    resetGrid();
  }
})

So you just remove the event listeners each time.

The issue remains that because you are manipulating the css styling it remains applied when you remove the Event Listener it can mess up how the next event listeners css is applied.