Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript

Posted by ali mur
.a{fill-rule:evenodd;}techdegree
ali mur
UX Design Techdegree Student 23,377 Points

Remove Multiple Elements

Hi,

I have created a to-do list app. Everything is working properly. Except when I try to remove multiply items, not all of them are removed. For example: if i select three items, and click the button remove, and only the three items are removed. There are three seperate files. HTML, CSS, and JS.

<head>
</head>
<link rel="stylesheet" href="style.css">
<body>
  <div id="todo-list">
      <ul id="todo-ul">
        <input type="text" id="input"/>
        <button id="add-btn">Add</button>
        <button id="rmv-btn">Remove</button>
        <button id="edit-btn">Edit</button>
        <button id="done-btn">Done</button>

      </ul>
  </div><!--todolist ends here-->

  <div id="done-list">

        <ul id="done-ul">
        </ul>

  </div><!--endlist ends here-->



  <script src="app.js"></script>
</body>
</html>
--------------
#todo-list {
  width:420px;
  height:45%;
  margin:auto;
  border:1px solid black;
  padding:5px;

}
#done-list {
  width:420px;
  height:45%;
  margin:auto;
  border:1px solid black;
  padding:5px;
}
ul{
  padding:5px;
  margin:5px;
  list-style: none;
}
li {
  border:1px solid black;
  margin:2px;
  padding:2px;
}
--------------
const tododiv=document.getElementById('todo-list');
const donediv=document.getElementById('done-list');
const todoul=document.getElementById('todo-ul');
const doneul=document.getElementById('done-ul');
const input=document.getElementById('input');



todoul.addEventListener('click',(e)=>{
  const target1=e.target.id;

            if (target1==="add-btn"){
              const checkbox=document.createElement('input');
              checkbox.setAttribute('type','checkbox');
              checkbox.setAttribute('class','checkbox');
              //here we created a span item to make sure we can edit it later
              const span=document.createElement('span');
              const todoli=document.createElement('li');
              todoli.setAttribute('class','todo-li');
              span.textContent=input.value;
              todoli.appendChild(span);
              todoli.appendChild(checkbox);
              todoul.appendChild(todoli);
            }//if statement ends here

            else if (target1==="rmv-btn"){
              const checkbox=document.getElementsByClassName('checkbox');
            for (var i=0;i<checkbox.length; i++){
              if(checkbox[i].checked){
                            const checkboxparent=checkbox[i].parentElement;
              checkboxparent.parentElement.removeChild(checkboxparent);
              }//if statement ends here
            }//for loop ends here
          }//else if ends here



          else if(target1==="edit-btn"){
            const checkbox=document.getElementsByClassName('checkbox');
            const editbutton=document.getElementById('edit-btn')
            for (var i=0;i<checkbox.length; i++){
              if (checkbox[i].checked){
                const li=document.getElementsByClassName('todo-li')
                const span= document.getElementsByTagName('span')
                const input=document.createElement('input');
                input.type='text'
                input.setAttribute('id','edit-input');
                li[i].insertBefore(input,span[i]);
                li[i].removeChild(span[i])

                input.value="" ;
                editbutton.textContent='Save';
                editbutton.id="save-btn";
              }//if statement ends here
            }//for loop ends here
          }//else if ends here

                else if(target1==="save-btn"){
                  const checkbox=document.getElementsByClassName('checkbox');
                  for (var i=0;i<checkbox.length; i++){
                      if (checkbox[i].checked){
                  const li=document.getElementsByClassName('todo-li')
                  const input=document.getElementById('edit-input')
                  const span=document.createElement('span');

                  span.textContent=input.value;
                  li[i].insertBefore(span,input);
                  li[i].removeChild(input);
                  document.getElementById('save-btn').textContent='Edit';
                  document.getElementById('save-btn').id="edit-btn";
                }//if statement ends here
              }//for loop ends here
            }//else if ends here


            else if(target1==="done-btn"){
              const checkbox=document.getElementsByClassName('checkbox');
            for (var i=0;i<checkbox.length; i++){
              if(checkbox[i].checked){
                  const checkboxparent=checkbox[i].parentElement;
                  doneul.appendChild(checkboxparent);
            }//if statement ends here
          }//for loop ends here
        }//else if ends here
},false)```

1 Answer

Steven Parker
Steven Parker
242,191 Points
Steven Parker
Steven Parker
242,191 Points

You're removing items from the collection you are iterating.

I assume you meant to say ".. all three items are not removed" instead of "... only the three items are removed".

But the issue is a result of taking items from a live HTMLCollection while you are iterating that same collection. A live HTMLCollection is automatically updated when the underlying document is changed. This causes your loop to skip over elements.

One fix for that would be to iterate in the opposite direction:

rmv-btn.js 
    for (let i = checkbox.length - 1; i >= 0 ; i--) {
ali mur
.a{fill-rule:evenodd;}techdegree
ali mur
UX Design Techdegree Student 23,377 Points

Steven, yes you have understood it correctly, not all of the items are removed. I have tried your code it works, but as always it is not very easy to understand your methods. I did not know that you could reverse loop. I think the Treehouse team should add this loop to their tutorials.

Steven Parker
Steven Parker
242,191 Points
Steven Parker
Steven Parker
242,191 Points

Just to be clear, my suggestion is just one of several ways to resolve this issue. There are many other ways that would not require a loop with decrementing index. This was just the first thing that came to mind that only required changing a single line.