JavaScript Etch-a-Sketch shading pen stops increasing opacity after another pen has been clicked on

Question

I'm creating an Etch-a-Sketch for The Odin Project using HTML, CSS, & JavaScript. I'm having some difficulties with the JavaScript for the "shader pen" I've implemented.

Note: The "pens" described in this question are not to be confused with code pens.

There are two pens: a black pen that turns a cell black and the shader pen which, on the first pass, turns the cell black but changes the opacity to 0.1 so that it's almost transparent. This creates the illusion of slight darkening since the background behind the cell is the same as the original color of the cell. After each subsequent pass with the shader pen, the opacity increases by 0.1.

The shader pen is the default pen. It works fine at first, but when I click on the black pen and then go back to the shader pen, the shader pen increases the opacity once and stops. It also overrides the black cells.

Here's how I set up the pens:

makeGrid(16); // generate grid
pen("shader"); // default pen: shader

document.querySelector('#black').addEventListener("click", e => pen("black"));
document.querySelector('#shader').addEventListener("click", e => pen("shader"));

function pen(selected) {
  let cells = document.querySelectorAll('div.cell');
  cells.forEach(cell => {
    cell.addEventListener('mouseover', function(e) {
      if (selected == "black") {
        // turns cell black
        cell.classList.remove('shade');
        cell.style.backgroundColor = '#101010';
        cell.style.opacity = '1';
        console.log('Make cell black')
      } else if (selected == "shader") {
        let opacity = cell.style.opacity;
        if (cell.classList.contains("shade")) {
          // increases opacity by 0.1
          cell.style.opacity = (Number(opacity) + 0.1);
          console.log('If there is shade, increase opacity.')
        } else {
          // turns cell to 0.1 opacity
          cell.classList.add('shade');
          cell.setAttribute('style', 'opacity:0.1');
          cell.style.backgroundColor = '#101010';
          console.log('Else, add shade class.')
        }
      }
    })
  });
}

I ran this code with a console.log statement in each conditional block and it looks like the black pen is still running when I go back to using the shader pen. My best guess is that this is what's causing the problem, but I'm not sure how to fix it.

I've created a CodePen with the rest of the code so you can see it in action. As previously mentioned, the default pen is the shader. It works as intended at the beginning, but it just stops after the black pen is selected.

Any help or guidance would be greatly appreciated.

user7148391 · Accepted Answer

There's so many problems with your code, The main one is when you change the pen type (when you call the pen() function) you add a new mouseover event listener.

Let's walk through it

Your code starts by calling pen('shader') making the shader the default pen type

pen("shader");

let's look at the pen() definition

function pen(selected) {
  let cells = document.querySelectorAll('div.cell');
  cells.forEach(cell => {    
    cell.addEventListener('mouseover', function(e){     
       ...
    })  
  });
}

What happens is the event listener takes the string shader and closes on it (This is called a closure) for that instance now whenever you hover over a cell the pen type will be shader

Now when you select the black pen you call the function pen() again adding a new set of event listeners.

This does not replace the old event listener with the new one, it stacks them like calling two functions one after the other.

Now when you hover over a cell the first event listener with the pen type shader will fire and then the second one with the black pen will fire after it, you can see this in the console you see two messages being printed from both if statement conditions.

Now when you pick the shader again you add a new set of event listeners with the shader pen type, what happens is the very first event listener will fire setting opacity to 0.1 then comes the black one removing the shade class then comes the third listener again checking if the cell has the class shade if(cell.classList.contains("shade")) which is false then falls down to the last else statement.

There's a ton of ways to fix this, but seeing that you're a beginner i will give you simple solutions

First make the pen type a global variable and on each button click you change it
Second add only one event listener to all cels at the start of your code and have them check the global variable

Demo

let penType = 'shader';

makeGrid(4); // generate grid


document.querySelector('#black').addEventListener("click", e => penType = "black");
document.querySelector('#shader').addEventListener("click", e => penType = "shader");



// generate grid
function makeGrid(dimension) {
  const canvas = document.querySelector('#canvas');
  const canvasWidth = document.getElementById("canvas").offsetWidth;
  const cellWidth = canvasWidth / dimension;

  for (let x = 1; x <= dimension * dimension; x++) {
    const makeCell = document.createElement('div');
    makeCell.classList.add('cell');
    canvas.appendChild(makeCell);
  };

  canvas.style.gridTemplateRows = `repeat(${dimension}, ${cellWidth}px [row-start]`;
  canvas.style.gridTemplateColumns = `repeat(${dimension}, ${cellWidth}px [column-start]`;


  // after all cells have been generated add the event listener
  // this is not the most optimize version of this but it gets the job done
  let cells = document.querySelectorAll('div.cell');
  cells.forEach(cell => {
    cell.addEventListener('mouseover', function(e) {
      console.log(penType)
      if (penType == "black") { // turns cell black
        cell.classList.remove('shade');
        cell.style.backgroundColor = '#101010';
        cell.style.opacity = '1';
        console.log('Make cell black')
      } else if (penType == "shader") { // turns cell 0.1
        let opacity = cell.style.opacity;
        if (cell.classList.contains("shade")) {
          cell.style.opacity = (Number(opacity) + 0.1);
          console.log('If there is shade, increase opacity.')
        } else {
          cell.classList.add('shade');
          cell.setAttribute('style', 'opacity:0.1');
          cell.style.backgroundColor = '#101010';
          console.log('Else, add shade class.')
        }
      }
    })
  });
}

body {
  margin: 0;
  background-color: #514c53;
  color: #fff;
  font-size: 18pt;
}


/* DIV STYLING */

#container {
  margin: 20px auto;
  max-width: 400px;
  display: flex;
  flex-wrap: wrap;
}

.full {
  width: 100%;
}

.left {
  width: 75%;
  margin-top: 15px;
}

.right {
  width: 25%;
  text-align: right;
}

#canvas {
  display: grid;
  flex-wrap: wrap;
  grid-template-rows: repeat(16, 30px [row-start]);
  grid-template-columns: repeat(16, 30px [col-start]);
  background: #e8dfd6;
}

.cell {
  background: #e8dfd6;
}


/* CELL SHADING */

.black {
  background-color: #101010;
}

.shade {
  background-color: #101010;
  width: 100%;
  height: 100%;
}


/* ELEMENT STYLING */

button {
  font-weight: 700;
  margin: 10px 0;
  background-color: #b9967d;
  color: #fff;
  padding: 0px 15px;
  text-shadow: 1px 1px #2e2e2e;
  box-shadow: 3px 3px #2e2e2e;
  border-radius: 5px;
  border: 0px;
  text-transform: uppercase;
  height: 30px;
  outline: none;
  transition: 0.3s;
}

button:hover {
  cursor: pointer;
  background-color: #aa8062;
  box-shadow: 0px -3px #2e2e2e;
}

button:active {
  background-color: #997963;
}

.circle {
  float: left;
  height: 15px;
  width: 15px;
  border: 1px solid #fff;
  border-radius: 50%;
  margin: 1px 5px 0 0;
}

.circle:hover {
  cursor: pointer;
  /*transition: 0.3s; 
  border: 2px solid #fff;*/
}

.meaning {
  float: left;
  font-size: .542em;
  text-transform: uppercase;
  padding-top: 2px;
  margin-right: 10px;
}

#black {
  background-color: #101010;
}

#shader {
  background-image: linear-gradient(#807b76, #dfd8d0);
}


  
    
    Black
    
    Shader

One tiny problem with your logic, when go to the black pen you don't have to remove the shade class because then when you come back to the shade pen and you hover over a black cell it sets opacity back to 0.1

JavaScript Etch-a-Sketch shading pen stops increasing opacity after another pen has been clicked on

Answers (2)

Related Questions