geo
geo

Reputation: 1101

Delete function with checkbox and JS doesn't work properly

currently I am making my own project and I faced a problem. I made a code like below to Select/Unselect/Delete checkbox(include its grandparent Element(.layout)).

However, somehow It only works 50%. When I create a lot of checkboxes and try to select or unselect them, it works perfectly fine. But if I try to delete them after selecting them all, it only deletes half of them, and I click it again, and it deletes half of them remain and do this over and over until only one checkbox is left. This is not the only problem I have. When there is only a single checkbox, I can not select, unselect, or delete it with buttons I made. could you tell me what is wrong with my code and how to solve it, please?

//P.S below code is just part of my code to summarize code so I can show you the exact part of my problem. If you need a whole code send me message I will show you :)

thx.

const selectAll = document.querySelector(".select");
const unselectAll = document.querySelector(".unselect");
const deleteIndex = document.querySelector(".delete");

selectAll.addEventListener("click", selectAllindex);
unselectAll.addEventListener("click", unselectAllindex);
deleteIndex.addEventListener("click", deleteDiaryIndex);

function selectAllindex(i) {
  i.preventDefault();
  for (i = 0; i < testform.checkboxchild.length; i++) {
    console.log(testform.checkboxchild.length);
    testform.checkboxchild[i].checked = true;
  }
}
function unselectAllindex(i) {
  i.preventDefault();
  for (i = 0; i < testform.checkboxchild.length; i++) {
    testform.checkboxchild[i].checked = false;
  }
}

function deleteDiaryIndex(i) {
  i.preventDefault();
  for (i = 0; i < testform.checkboxchild.length; i++) {
    if (testform.checkboxchild[i].checked) {
      removeDiv(testform.checkboxchild[i]);
    }
  }
}

function removeDiv(d) {
  console.log(d.parentElement.parentElement);
  d.parentElement.remove();
}
.trash{
  display: none;
}
<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Document</title>
    <link rel="stylesheet" href="./test.css" />
  </head>
  <body>
    <form action="" name="testform" class="test">
      <div class="divs">
        <div class="layout">
          <div class="checkboxs">
            <input type="checkbox" name="checkboxchild" id="" />
          </div>
        </div>
        <div class="layout">
          <div class="checkboxs">
            <input type="checkbox" name="checkboxchild" id="" />
          </div>
        </div>
        <div class="layout">
          <div class="checkboxs">
            <input type="checkbox" name="checkboxchild" id="" />
          </div>
        </div>
        <div class="layout">
          <div class="checkboxs">
            <input type="checkbox" name="checkboxchild" id="" />
          </div>
        </div>
      </div>
      <div class="buttons">
        <button class="selector select" value="selectall">Select All</button>
        <button class="selector unselect" value="unselectall">
          Unselect All
        </button>
        <button class="selector delete" value="delete">Delete</button>
      </div>
    </form>

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

Upvotes: 1

Views: 482

Answers (1)

MikeM
MikeM

Reputation: 13631

if I try to delete them after selecting them all, it only deletes half of them, and I click it again, and it deletes half of them remain and do this over and over until only one checkbox is left.

This happens because you are mutating a live list of DOM nodes, testform.checkboxchild, as you iterate through it.

function deleteDiaryIndex(i) {
  i.preventDefault();

  console.log(testform.checkboxchild instanceof HTMLCollection);  // false
  console.log(testform.checkboxchild instanceof NodeList);        // true
  console.log(Object.prototype.toString.call(testform.checkboxchild));
  // '[object RadioNodeList]' (in both Chrome and Firefox, strangely)

  for (i = 0; i < testform.checkboxchild.length; i++) {
    if (testform.checkboxchild[i].checked) {
      removeDiv(testform.checkboxchild[i]);
    }
  }
}

This table tries to show what happens in the above function when iterating through the checkboxes when all are checked.

i = 0 i = 1 i = 2
1st checkBox is checkboxchild[0] removed
2nd checkBox is checkboxchild[1] checkboxchild[0] checkboxchild[0]
3rd checkBox is checkboxchild[2] checkboxchild[1] removed
4th checkBox is checkboxchild[3] checkboxchild[2] checkboxchild[1]
checkboxchild.length 4 3 2

Only two of the checkboxes get removed because the length and indices of testform.checkboxchild are automatically updated after each checkbox is removed from the DOM, so that what was the index of a removed checkbox becomes the index of the next checkbox in the collection.

For example, the second checkbox doesn't get removed because on the second iteration of the loop, when i = 1, checkboxchild[1], points to the third checkBox, not the second!

Notice also that the loop ends when i becomes 2, as that is not less than the length, which is 2 after two of the checkboxes have been removed.

One solution to the above is to convert the live collection to a normal Array before iterating through it, for example

const checkboxes = Array.from(testform.checkboxchild);

Another solution is not to use testform.checkboxchild at all, but instead to use querySelectorAll, which returns a static list of the nodes rather than a live one.

const checkboxes = document.querySelectorAll('input[type=checkbox]');
for (let i = 0; i < checkboxes.length; i++) {
    if (checkboxes[i].checked) {
      removeDiv(checkboxes[i]);
    }
  }
}

or

const checkedboxes = document.querySelectorAll('input[type=checkbox]:checked');
for (const cb of checkedboxes) {
  removeDiv(cb);
}

You may also want to consider using the display: none or visibility: hidden styles as an easily-reversible alternative to removing the checkboxes from the document altogether.

When there is only a single checkbox, I can not select, unselect, or delete it with buttons I made

This is another issue with using testform.checkboxchild to reference the checkboxes.

If only one checkbox remains, then testform.checkboxchild will point to that single checkbox rather than to a list of checkboxes, so it will not have a length property and be iterable.

One simple solution is to check for the existence of the length property first. For example

function selectAllindex(event) {
  event.preventDefault();
  if (testform.checkboxchild.length !== undefined) {
     for (let i = 0; i < testform.checkboxchild.length; i++) {
       testform.checkboxchild[i].checked = true;
     }  
  } else {
    testform.checkboxchild.checked = true;
  }
}

or

function selectAllindex(event) {
  event.preventDefault();
  const checkboxes = document.querySelectorAll('input[type=checkbox]');
  for (const cb of checkboxes) {
    cb.checked = true;
  }
}

My recommendation is: don't use testform.checkboxchild to reference the checkboxes.

Upvotes: 2

Related Questions