0

I'm building a to do list, and adding a remove and completed functionality to the list. However, once the remove/completed button is pressed, it is applied to the all the li elements. Furthermore, if one adds more items to the list, the buttons lose their functionality.

I've checked the JS Console, and there are no errors. I've built a snippet to recreate the bug:

$(document).ready(function() {
    var removeImg = '<img src= "https://www.icon2s.com/wp-content/uploads/2013/07/ios7-trash-icon.png" width = "22px" class = "removeicon">';
    var completeImg = '<img src = "https://www.shareicon.net/data/128x128/2016/11/28/857777_circle_512x512.png" class = "completeicon" width = "22px">';
    document.getElementById("add").addEventListener('click', function(){
        var value = document.getElementById("addtask_TD").value;
        var lookat = document.getElementById("addtask_TD");
        if (value) {
            addItemTodo(value);
            lookat.focus();
            lookat.select();
        }

    });
    function addItemTodo(text){
        var list = document.getElementById('todo');

        var item = document.createElement('li');
        item.innerText = text;
        
        var buttons = document.createElement('div');
        buttons.classList.add('buttons');
        
        var remove = document.createElement('button');
        remove.classList.add('remove');
        remove.classList.add('btn');
//  $(".remove").attr("id","delete");
        remove.innerHTML = removeImg;
        
        var complete = document.createElement('button');
        complete.classList.add('complete');
        complete.classList.add('btn');
// $(".complete").attr("id","done");
        complete.innerHTML = completeImg;
        buttons.appendChild(remove);
        buttons.appendChild(complete);
        item.appendChild(buttons);
        list.appendChild(item);
 document.getElementsByClassName("remove")[0].addEventListener('click', function(){
            $(item).fadeOut(1000);
 });
        document.getElementsByClassName("complete")[0].addEventListener('click', function(){
            $(".completeicon").attr("src", "https://png.icons8.com/cotton/50/000000/checkmark.png");
        });
    }
});

    
header{
 width:100% !important;
 height: 80px !important;
 position:fixed !important;
 top:0 !important;
 
 left:0 !important;
 z-index:5 !important;
 background: #25b99a !important;
 box-shadow:0px 2px 4px rgba(44,62,80,0.15) !important;
 padding:15px;
}

#addtask_TD {
 width:100% !important;
 height:50px !important;
 float:left !important;
 border-radius: 5px !important;
 border: 0px;
 box-shadow:none !important;
 outline:none;
 color:#fff;
 font-size:15px;
 font-weight:400px;
 text-indent:16px;
 border-top-right-radius:25px !important;
 border-top-left-radius:5px !important;
 border-bottom-right-radius:25px !important;
 border-bottom-left-radius:5px !important;
 text-indent:10px !important;

 background-color:rgba(0, 0, 0,0.22) !important; 
}

header input:: -webkit-input-placeholder{
 color:rgba(255,255,255,.75);
}

header input:: -moz-input-placeholder{
 color:rgba(255,255,255,.75);
}
header input:: -o-input-placeholder{
 color:rgba(255,255,255,.75);
}
header input:: -ms-input-placeholder{
 color:rgba(255,255,255,.75);
}
header button {
 width:50px;
 height:50px;
 position:absolute;
 top: 15px;
 right:15px;
 z-index:2;
 border-radius:25px;
 background:8px;
 box-shadow:none;
 outline: none; 
 background-color:white;
 border:none;
  -o-appearance:none !important;
 -ms-appearance:none !important;
 -moz-appearance:none !important;
 -webkit-appearance:none !important;
 appearance:none !important;
 
}

header button svg{
 width:16px;
 height:16px;
 position:absolute;
 top:50%;
 left:50%;
 margin: -8px 0 0 -8px;
}

header button svg .fill{
 fill: #25b99a;
}

ul.todo{
 width:100%;
 float:left;
  
}
.st0{fill:none;}
.st1{fill:#C0CECB;}
ul.todo li {
 width:108%;
 float:left;
 height:50px;
 position:relative;
 background:darkblue;
 border-radius:5px;
 box-shadow:0px 1px 2px rgba(44,62,80,0.10);
 bottom: -16px;
 padding: 0 100px 0 0;
 /**margin: -20px;**/
 margin: 0 0 15px -20px;
}

.containtodolist{
 width:100%;
 float:left;
 padding: 15px;
}

ul#todo li:last-of-type{
/** margin:0;**/
}

ul#todo li .buttons {
 width:100px;
 height:50px;
 position:absolute;
 top:0;
 right:0;
}

ul#todo li .buttons button{
 width:50px;
 height:50px;
 float:left;
 background:none;
 border:0px;
 box-shadow:none;
 outline:none;
 position:relative;
 
 -o-appearance:none !important;
 -ms-appearance:none !important;
 -moz-appearance:none !important;
 -webkit-appearance:none !important;
 appearance:none !important;
 
}

ul#todo li .buttons button:last-of-type:before{
 content:'';
 width:1px;
 height:30px;
 position:absolute;
 top:10px;
 left: 0;
 background:#edf0f1;
 
}

.buttons{
 display:block;
}

.noFill{
 fill:none;
}

ul#todo li .buttons button svg{
 width:22px;
 height:22px;
 position:absolute;
 top:50%;
 left:50%;
 margin: -11px 0 0 -11px;
}

li{
 background-color:lightblue;
 margin: 65px 0 -70px 0;
 padding-top:7px;
 padding-bottom:24px;
 padding-left:50px;
 
 border-top: 6px solid #79afaf;
  right: 40px;
}

.remove,.complete{
 margin: 10px;

}

.remove,.complete{
  padding-top: 6px !important;
}
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/materialize/0.100.2/css/materialize.min.css">
<script type="text/javascript" src="https://code.jquery.com/jquery-2.1.1.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/materialize/0.100.2/js/materialize.min.js"></script>

<div class = "containtodolist">
    <header>
        <input id = "addtask_TD" type = "text" placeholder = "Add task">
        <button id = "add"><svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"viewBox="0 0 16 16" style="enable-background:new 0 0 16 16;" xml:space="preserve"><g><path class="fill" d="M16,8c0,0.5-0.5,1-1,1H9v6c0,0.5-0.5,1-1,1s-1-0.5-1-1V9H1C0.5,9,0,8.5,0,8s0.5-1,1-1h6V1c0-0.5,0.5-1,1-1s1,0.5,1,1v6h6C15.5,7,16,7.5,16,8z"/></g></svg></button>
    </header> 
    <div class = "container_TD" id = "todo">
<!-- <ul class = "todo" id = "completed">
            <li> WHouha 
                    <div class = "buttons">
                            <button class = "remove"> <img src="https://png.icons8.com/ios/50/000000/delete.png" width = "22px"></button>
                            <button class = "complete"><img src = "https://www.shareicon.net/data/128x128/2016/11/28/857777_circle_512x512.png" width = "22px"> </button>
                    </div>
            </li>
    </ul>-->
    </div>  
    <ul class = "todo" id = "completed"></ul>
</div>
  
cela
  • 2,352
  • 3
  • 21
  • 43
DarkRunner
  • 449
  • 4
  • 15
  • You only add the event listener to `[0]`, so just the first button of each type. – Barmar Apr 05 '18 at 00:59
  • 1
    Why are you mixing jQuery and plain JS like this? If you're using jQuery, see https://stackoverflow.com/questions/203198/event-binding-on-dynamically-created-elements for how to bind events one time for all the dynamic elements. – Barmar Apr 05 '18 at 01:03

2 Answers2

2

If you're using jquery, you should use jquery to select elements and to apply click events, dont mix in vanilla js's getElementsByClassName and addEventListener. It will make your code harder to read and maintain in the future.

Issue 1:

once the remove/completed button is pressed, it is applied to the all the li elements

this is because when you do

      $(".completeicon").attr("src", "https://png.icons8.com/cotton/50/000000/checkmark.png")

That is the jquery equivalent of selecting all elements of the class .completeicon, so clicking one triggers all of them. That's why everything completes.

Everything removing happens for a different reason:

 document.getElementsByClassName("remove")[0].addEventListener('click', function() {
      $(item).fadeOut(1000);
    });

Whenever you add a todo, you only ever assign the remove event to the first remove button in the first todo (because it is document.getElementsByClassName("remove")[0]). But, each time you call addEventListener, item represents the latest item added to the list. So clicking the first remove, will successively remove itself and each other item on the list.

This also explains issue 2:

Furthermore, if one adds more items to the list, the buttons lose their functionality.

That is because again, you are only ever applying the events to the first item on the list (the [0]th item).

To solve it, instead of using document.getElementsByClassName to select the DOM elements apply events to, you can just apply events directly to the variables of your various DOM elements, which is handy and IMHO easier to read:

Just before this:

 buttons.appendChild(remove);
 buttons.appendChild(complete);
 item.appendChild(buttons);

 list.appendChild(item);

You can do

$(remove).click(function(){
    $(item).fadeOut(1000);
});

$(complete).click(function(){
    $(this).find('.completeicon').attr('src',"https://png.icons8.com/cotton/50/000000/checkmark.png")
});

Which just targets the latest remove button and complete button that is about to be added. Then just remove the addEventListener code completely.

chiliNUT
  • 18,989
  • 14
  • 66
  • 106
1

simply replace

document.getElementsByClassName("remove")[0].addEventListener('click', function(){
    $(item).fadeOut(1000);
});

with

remove.addEventListener('click', function(){
    $(item).fadeOut(1000);
});

you don't need to query the dom, just append the handler to your remove variable.

EDIT

OOPS, I had forgotten about the completion part

replace

document.getElementsByClassName("complete")[0].addEventListener('click', function(){
    $(".completeicon").attr("src", "https://png.icons8.com/cotton/50/000000/checkmark.png");
});

with

complete.addEventListener('click', function(){
    $(this).children('.completeicon').attr('src', "https://png.icons8.com/cotton/50/000000/checkmark.png")
});

too

$(document).ready(function() {
var removeImg = '<img src= "https://www.icon2s.com/wp-content/uploads/2013/07/ios7-trash-icon.png" width = "22px" class = "removeicon">';
 var completeImg = '<img src = "https://www.shareicon.net/data/128x128/2016/11/28/857777_circle_512x512.png" class = "completeicon" width = "22px">';
 document.getElementById("add").addEventListener('click', function(){
  
  var value = document.getElementById("addtask_TD").value;
  var lookat = document.getElementById("addtask_TD");
  if(value) {
   addItemTodo(value);
   lookat.focus();
   lookat.select();
  }
  
 });
 
 function addItemTodo(text){
  var list = document.getElementById('todo');
  
  var item = document.createElement('li');
  item.innerText = text;
  
  var buttons = document.createElement('div');
  buttons.classList.add('buttons');
  
  var remove = document.createElement('button');
  remove.classList.add('remove');
  remove.classList.add('btn');
//  $(".remove").attr("id","delete");
  remove.innerHTML = removeImg;
  
  var complete = document.createElement('button');
  complete.classList.add('complete');
  complete.classList.add('btn');
 // $(".complete").attr("id","done");

  complete.innerHTML = completeImg;
  
  buttons.appendChild(remove);
  buttons.appendChild(complete);
  item.appendChild(buttons);
  
  list.appendChild(item);
 
 
remove.addEventListener('click', function(){
 $(item).fadeOut(1000);
 });

complete.addEventListener('click', function(){
 $(this).children('.completeicon').attr('src', "https://png.icons8.com/cotton/50/000000/checkmark.png")

});
 
 }
});
header{
 width:100% !important;
 height: 80px !important;
 position:fixed !important;
 top:0 !important;
 
 left:0 !important;
 z-index:5 !important;
 background: #25b99a !important;
 box-shadow:0px 2px 4px rgba(44,62,80,0.15) !important;
 padding:15px;
}

#addtask_TD {
 width:100% !important;
 height:50px !important;
 float:left !important;
 border-radius: 5px !important;
 border: 0px;
 box-shadow:none !important;
 outline:none;
 color:#fff;
 font-size:15px;
 font-weight:400px;
 text-indent:16px;
 border-top-right-radius:25px !important;
 border-top-left-radius:5px !important;
 border-bottom-right-radius:25px !important;
 border-bottom-left-radius:5px !important;
 text-indent:10px !important;

 background-color:rgba(0, 0, 0,0.22) !important; 
}

header input:: -webkit-input-placeholder{
 color:rgba(255,255,255,.75);
}

header input:: -moz-input-placeholder{
 color:rgba(255,255,255,.75);
}
header input:: -o-input-placeholder{
 color:rgba(255,255,255,.75);
}
header input:: -ms-input-placeholder{
 color:rgba(255,255,255,.75);
}
header button {
 width:50px;
 height:50px;
 position:absolute;
 top: 15px;
 right:15px;
 z-index:2;
 border-radius:25px;
 background:8px;
 box-shadow:none;
 outline: none; 
 background-color:white;
 border:none;
  -o-appearance:none !important;
 -ms-appearance:none !important;
 -moz-appearance:none !important;
 -webkit-appearance:none !important;
 appearance:none !important;
 
}

header button svg{
 width:16px;
 height:16px;
 position:absolute;
 top:50%;
 left:50%;
 margin: -8px 0 0 -8px;
}

header button svg .fill{
 fill: #25b99a;
}

ul.todo{
 width:100%;
 float:left;
  
}
.st0{fill:none;}
.st1{fill:#C0CECB;}
ul.todo li {
 width:108%;
 float:left;
 height:50px;
 position:relative;
 background:darkblue;
 border-radius:5px;
 box-shadow:0px 1px 2px rgba(44,62,80,0.10);
 bottom: -16px;
 padding: 0 100px 0 0;
 /**margin: -20px;**/
 margin: 0 0 15px -20px;
}

.containtodolist{
 width:100%;
 float:left;
 padding: 15px;
}

ul#todo li:last-of-type{
/** margin:0;**/
}

ul#todo li .buttons {
 width:100px;
 height:50px;
 position:absolute;
 top:0;
 right:0;
}

ul#todo li .buttons button{
 width:50px;
 height:50px;
 float:left;
 background:none;
 border:0px;
 box-shadow:none;
 outline:none;
 position:relative;
 
 -o-appearance:none !important;
 -ms-appearance:none !important;
 -moz-appearance:none !important;
 -webkit-appearance:none !important;
 appearance:none !important;
 
}

ul#todo li .buttons button:last-of-type:before{
 content:'';
 width:1px;
 height:30px;
 position:absolute;
 top:10px;
 left: 0;
 background:#edf0f1;
 
}

.buttons{
 display:block;
}

.noFill{
 fill:none;
}

ul#todo li .buttons button svg{
 width:22px;
 height:22px;
 position:absolute;
 top:50%;
 left:50%;
 margin: -11px 0 0 -11px;
}

li{
 background-color:lightblue;
 margin: 65px 0 -70px 0;
 padding-top:7px;
 padding-bottom:24px;
 padding-left:50px;
 
 border-top: 6px solid #79afaf;
  right: 40px;
}

.remove,.complete{
 margin: 10px;

}

.remove,.complete{
  padding-top: 6px !important;
}
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/materialize/0.100.2/css/materialize.min.css">
 <script type="text/javascript" src="https://code.jquery.com/jquery-2.1.1.min.js"></script>
 <script src="https://cdnjs.cloudflare.com/ajax/libs/materialize/0.100.2/js/materialize.min.js"></script>

 <div class = "containtodolist">
  <header>
  <input id = "addtask_TD" type = "text" placeholder = "Add task">
   <button id = "add">
    
    
        <svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"viewBox="0 0 16 16" style="enable-background:new 0 0 16 16;" xml:space="preserve"><g><path class="fill" d="M16,8c0,0.5-0.5,1-1,1H9v6c0,0.5-0.5,1-1,1s-1-0.5-1-1V9H1C0.5,9,0,8.5,0,8s0.5-1,1-1h6V1c0-0.5,0.5-1,1-1s1,0.5,1,1v6h6C15.5,7,16,7.5,16,8z"/></g></svg>
   </button>
  </header>
  
  
  <div class = "container_TD" id = "todo">
 <!-- <ul class = "todo" id = "completed">
   <li> WHouha 
    <div class = "buttons">
     <button class = "remove"> <img src="https://png.icons8.com/ios/50/000000/delete.png" width = "22px"></button>
     <button class = "complete"><img src = "https://www.shareicon.net/data/128x128/2016/11/28/857777_circle_512x512.png" width = "22px"> </button>
    </div>
   </li>
  </ul>-->
  </div>
  
<ul class = "todo" id = "completed">
   </ul>
user229044
  • 232,980
  • 40
  • 330
  • 338
Scaramouche
  • 3,188
  • 2
  • 20
  • 46
  • Wow, I did not expect the solution to be so simple (I added [0], that's why). However, I tried the same tactic for replacing the check image, however, the issue remains. – DarkRunner Apr 05 '18 at 01:18