0

I'm trying to set the background of a div when it is clicked, and then when it is clicked again it reverses back to no color. Here is my code:

<div class="wrapper">
        <div onclick="set()" id="square"></div>
        <div onclick="set()" id="square"></div>
        <div onclick="set()" id="square"></div>
        <div onclick="set()" id="square"></div>
        <div onclick="set()" id="square"></div>
        <div onclick="set()" id="square"></div>
        <div onclick="set()" id="square"></div>
</div>
.wrapper {
    display: grid;
    grid-template-columns: repeat(107, 1fr);
    grid-template-rows: repeat(59, 1fr);
    grid-gap: 0px;

}

.wrapper div {
    padding: 5px;
    background-color: none;
    text-align: center;
    border: 0.001px solid black;
    font-size: 5px;
    display: flex;
    justify-content: center;
    align-items: center;
}

.wrapper div:hover {
    background: white;
}
function set() {

    var squares = document.getElementById('square');
    for (var i = 0; i < squares.length; i++) {
        squares[i].style.backgroundColor = "none";
    }

    if (squares[i].style.backgroundColor === "none") {
        squares[i].style.backgroundColor = "white";
    } else {
        squares[i].style.backgroundColor = "none";
    }
}

When I click the div it doesn't do anything. My javascript code isn't working, how should I change it so it makes the div background white when clicked and then none when clicked again.

loki
  • 37
  • 8
  • 4
    It's invalid to have the same `id` on more than one element. An `id` must be unique. – Scott Marcus Jul 17 '20 at 23:12
  • `onclick="set(this)"` and change your method to accept the element as an argument, and then just change the style or toggle a class. – Taplar Jul 17 '20 at 23:15

2 Answers2

3

You can put an event listener on the parent element that listens for the click event. When it gets a click event, if it is from one of the squares, toggle a class to keep the background color on it. If it already has that class, it will be removed.

document.querySelector('.squares').addEventListener('click', e => {
  if (e.target.classList.contains('square')) {
    e.target.classList.toggle('selected');
  }
});
.wrapper {
  display: grid;
  grid-template-columns: repeat(107, 1fr);
  grid-template-rows: repeat(59, 1fr);
  grid-gap: 0px;
}

.wrapper div {
  padding: 5px;
  background-color: none;
  text-align: center;
  border: 0.001px solid black;
  font-size: 24px;
  display: flex;
  justify-content: center;
  align-items: center;
}

.wrapper .square:hover, .square.selected {
  background: white;
}

body { background-color: #888; }
<div class="wrapper squares">
  <div class="square">A</div>
  <div class="square">B</div>
  <div class="square">C</div>
  <div class="square">D</div>
  <div class="square">E</div>
  <div class="square">F</div>
  <div class="square">G</div>
</div>
Taplar
  • 24,788
  • 4
  • 22
  • 35
1

Your primary issue is this:

<div onclick="set()" id="square"></div>
<div onclick="set()" id="square"></div>
<div onclick="set()" id="square"></div>
<div onclick="set()" id="square"></div>
<div onclick="set()" id="square"></div>
<div onclick="set()" id="square"></div>
<div onclick="set()" id="square"></div>

It's invalid to have the same id on more than one element. An id must be unique. So this line:

var squares = document.getElementById('square');

just selects the first div and then later, when you try to loop over all the cells, you don't because you never had all of the cells in the first place.

You just need to set one click handler on the parent of all the div cells. Then, in that handler, you can apply a CSS class to the div that triggered the event. This is called event delegation and works because events bubble up through the DOM.

// Get a reference to all the cells
let squares = document.querySelectorAll("div.wrapper > div");

// Set up you event handlers in JavaScipt, not with inline HTML
// event attributes like onclick
document.querySelector(".wrapper").addEventListener("click", function(event) {

  // Remove all the background colors from all the cells
  squares.forEach(function(square){
    square.classList.remove("activeCell");
  });

  // Apply backgroun to clicked cell
  event.target.classList.add("activeCell");

});
.wrapper {
    display: grid;
    grid-template-columns: repeat(107, 1fr);
    grid-template-rows: repeat(59, 1fr);
    grid-gap: 0px;

}

.wrapper > div {
    padding: 5px;
    background-color: none;
    text-align: center;
    border: 0.001px solid black;
    font-size: 5px;
    display: flex;
    justify-content: center;
    align-items: center;
}

.wrapper div:hover {
    background: green;
}

.activeCell {
  background-color:blue;
}
<div class="wrapper">
        <div></div>
        <div></div>
        <div></div>
        <div></div>
        <div></div>
        <div></div>
        <div></div>
</div>
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71