Button changing newest div item instead of the correct-CodePudding

I created a div table of sorts, where there is a button, and that button creates a box, and what is supposed to happen is that you can click any of the {x} boxes and it colors it black However, instead of coloring that box black(or white), it colors the newest one black(or white). How do I change this?

HTML:

<!DOCTYPE html>
<html>

<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>Rule 110</title>
  <link href="style.css" rel="stylesheet" type="text/css" />
</head>

<body>
  <script src="script.js"></script>
  <center><h1>Rule 110</h1></center>
  <div >
  
    <main id="content"></main>
  
  
  </div>
  <button onclick=add() > </button>
</body>

</html>

Javascript:

const first = document.getElementById("first")
const second = document.getElementById("second")
const third = document.getElementById("third")
const fourth = document.getElementById("fourth")
const fifth = document.getElementById("fifth")
const sixth = document.getElementById("sixth")
const seventh = document.getElementById("seventh")
const eigth = document.getElementById("eigth")
const ninth = document.getElementById("ninth")
var extra = 1;
var thingrowx=0;
var thingrowy = 1;
function changeColor(test) {
  if (document.getElementById(test).className === "grid-item-white"){
    document.getElementById(test).className="grid-item-black";}
  else {
    document.getElementById(test).className="grid-item-white";
  }
}

function createTable() {
  rn = window.prompt("Input number of rows", 1);
  cn = window.prompt("Input number of columns",1);
  
   for(var r=0;r<parseInt(rn,10);r  )
  {
   var x=document.getElementById('myTable').insertRow(r);
   for(var c=0;c<parseInt(cn,10);c  )  
    {
     var y=  x.insertCell(c);
     y.innerHTML="Row-" r " Column-" c; 
    }
   }
}
function repeat(func, times) {
    func;
    times && --times && repeat(func, times);
}
function test() {
  document.querySelector('#content').insertAdjacentHTML(
    'afterbegin',
    `<div  id="combined"   onclick=changeColor('combined')>
      </div>`
    )
}
function add() {
  extra  = 1;
  thingrowx  = 1;
  var combined = "("   thingrowx   ","   thingrowy   ")"
  alert(combined)
  repeat(test(), thingrowy) 
}

CSS:

.button {
  border: none;
  background-color: black;
  padding: 16px 32px;
  text-align: center;
  text-decoration: none;
  display: inline-block;
  font-size: 16px;
  margin: 4px 2px;
  transition-duration: 0.4s;
  cursor: pointer;
  color: white;
}

.button1 {
  background-color: white; 
  color: black; 
  border: 2px solid #4CAF50;
}

.button1:hover {
  background-color: #4CAF50;
  color: white;
}

.button2 {
  background-color: white; 
  color: black; 
  border: 2px solid #008CBA;
}

.button2:hover {
  background-color: #008CBA;
  color: white;
}


.grid-container {
  display: grid;
  grid-template-columns: 50px 50px 50px 50px 0px 50px 50px 50px 0px 50px 50px 50px 50px 50px 50px 50px;
  padding: 10px;
}

.grid-item-white {
  background-color: white;
  border: 1px solid rgba(0, 0, 0, 0.8);
  padding: 20px;
  font-size: 30px;
  text-align: center;
}

.grid-item-black {
  background-color: black;
  border: 1px solid rgba(0, 0, 0, 0.8);
  padding: 20px;
  font-size: 30px;
  text-align: center;
  color: white
}

body {background-color: #cdf1eb ;}

CodePudding user response：

I think it's because all of your divs has the same id ("combined"), so the browser get the top one in the DOM. This can be solved if you give each of them a unique id. This works for me:

`<div  id="combined${extra}" onclick=changeColor('combined${extra}')`

CodePudding user response：

I think the problem is in the test function.

function test() {
  document.querySelector('#content').insertAdjacentHTML(
    'afterbegin',
    `<div  id="combined"   onclick=changeColor('combined')>
      </div>`
    )
}

You're setting the id to "combined" for every new added element. But an id must be unique for each element in the whole html document. May be you intended to do the following.

function test(combined) {
  // use the combined index that is passed from the add function
  document.querySelector('#content').insertAdjacentHTML(
    'afterbegin',
    // -------------------------------vvvvvvvvvvv-- use template literal
    `<div  id="${combined}"   onclick=changeColor('${combined}')>
      </div>`
    )
}
function add() {
  extra  = 1;
  thingrowx  = 1;
  var combined = "("   thingrowx   ","   thingrowy   ")"
  alert(combined)
  repeat(test(combined), thingrowy) // pass the combined index to the test method
}

Your code is very cluttered with poor naming and lots of unused variables and functions, so I've tried to refactor a little!

const contentElement = document.getElementById("content");

contentElement.addEventListener("click", (event) => {
  // here event.target represents the element that has been clicked on
  changeColor(event.target);
});

let rowX = 0;
let rowY = 1;

function changeColor(element) {
  if (element.className === "grid-item-white")
    element.className = "grid-item-black";
  else element.className = "grid-item-white";
}

function add() {
  rowX  = 1;
  const combined = "("   rowX   ","   rowY   ")";

  // alert(combined);

  document
    .querySelector("#content")
    .insertAdjacentHTML(
      "afterbegin",
      `<div  data-id="${combined}"></div>`
    );
}

.button {
  border: none;
  background-color: black;
  padding: 16px 32px;
  text-align: center;
  text-decoration: none;
  display: inline-block;
  font-size: 16px;
  margin: 4px 2px;
  transition-duration: 0.4s;
  cursor: pointer;
  color: white;
}

.button1 {
  background-color: white;
  color: black;
  border: 2px solid #4caf50;
}

.button1:hover {
  background-color: #4caf50;
  color: white;
}

.button2 {
  background-color: white;
  color: black;
  border: 2px solid #008cba;
}

.button2:hover {
  background-color: #008cba;
  color: white;
}

.grid-container {
  display: grid;
  grid-template-columns: 50px 50px 50px 50px 0px 50px 50px 50px 0px 50px 50px 50px 50px 50px 50px 50px;
  padding: 10px;
}

.grid-item-white {
  background-color: white;
  border: 1px solid rgba(0, 0, 0, 0.8);
  padding: 20px;
  font-size: 30px;
  text-align: center;
}

.grid-item-black {
  background-color: black;
  border: 1px solid rgba(0, 0, 0, 0.8);
  padding: 20px;
  font-size: 30px;
  text-align: center;
  color: white;
}

body {
  background-color: #cdf1eb;
}

<!DOCTYPE html>
<html>

<head>
  <meta charset="utf-8" />
  <meta name="viewport" content="width=device-width" />
  <title>Rule 110</title>
  <link href="style.css" rel="stylesheet" type="text/css" />
</head>

<body>
  <center>
    <h1>Rule 110</h1>
  </center>

  <button onclick="add()" > </button>

  <div >
    <main id="content"></main>
  </div>

  <script src="script.js"></script>
</body>

</html>

Here I've used event delegation to listen on click event on any grid items. So if we listen for click events on the #content element and use the event.target then we get the grid item that has been clicked on. Then we can pass that element to the changeColor method.

If you don't understand anything feel free to ask me in the comment. Hope this helps.