leo
leo

Reputation: 71

Eventlisteners not being added in the following js code

I am building a todo app where I am dynamically generating tasks using javascript.[(sample ouput)] I generate following equivalent html from js whenever I click on the add button:

<div class="row datasection">
  <div class="todo">
    <div class="databox col s6 offset-s1 waves-effect">
      <p class="checkglyph1 checkglyph2">Task no 1</p>
      <a>
        <i class="material-icons checkglyph checkglyph1 checkglyph2 glyphcolor">check</i>
      </a>
    </div>
  </div>
</div>

Now what I want is whenever I click event on the task created it should become yellow in colour.I have written the code to this. below.However it works fine only when there is one task created.If there is multiple then the last task works well but actionlistener on the first one does not seem to be working.I am not able to figure out where the code is breaking.

var glyph= document.querySelectorAll(".glyphcolor");

for (var i = 0; i < glyph.length; i++) {
    glyph[i].addEventListener('click', function(event) {
        this.classList.toggle("checkglyph1");
    });
}

The actual snippet

//declaration
    var calendardata = document.getElementById('date1');
    var addbutton = document.querySelector('.addbutton');
    var todo = document.querySelector('.todo');
    addbutton.addEventListener('click', function() {

/* body to return the html */

      if (data.value) {
        var newdiv = document.createElement("div"); // Create a <button> element
        newdiv.classList.add("databox", "col", "s6", "waves-effect");
        //console.log(newdiv);
        todo.appendChild(newdiv);
        //console.log(newdiv.parentNode);
        var newpar = document.createElement("p");
        newpar.classList.add("checkglyph1", "checkglyph2");
        var node = document.createTextNode(data.value + "." + "      " + 
        calendardata.value);
        var newa = document.createElement("a");
        newdiv.appendChild(newa)
        var newglyph = document.createElement("i");
        newglyph.classList.add("material-icons", "checkglyph", "checkglyph1", 
        "checkglyph2", "glyphcolor");
        var node1 = document.createTextNode("check");
        newa.appendChild(newglyph);
        newglyph.append(node1);
        newpar.appendChild(node);
        newdiv.appendChild(newpar);
        data.value = "";
        calendardata.value = "";
        created = true;
      //  console.log("before glyh created");

        //code to perform action on the click of the tick symbol

        var glyph = document.querySelectorAll(".glyphcolor");
        var par = document.getElementsByClassName('checkglyph2');
        for (var i = 0; i < glyph.length; i++) {
    //console.log("Inside the loop");
    glyph[i].addEventListener('click', function.bind(event) {
        this.classList.toggle("checkglyph1");
        //console.log('Inside the click');
        //console.log(i);
    });


}   

      }
    })

Upvotes: 1

Views: 318

Answers (2)

Bruno Lucena
Bruno Lucena

Reputation: 484

The problem with your code is that everytime you click on ".addbutton", you're going through all of the ".glyphcolor"s element's in your DOM and adding a new onclick event listener. The last one created will work, but the others will have multiple event listeners repeated (yes, this is possible). So, when you click on an element that has two events telling it to toggle the "checkglyph1" class, it will do it twice. And of course, it will not change at all. You can easily see this happening, because all the odd elements in your page must work (they will toggle the class by odd times), while the even ones must not.

This can be corrected by adding the event listener directly on the element when you create it on the page. The code below must work fine:

//declaration
var calendardata = document.getElementById('date1');
var addbutton = document.querySelector('.addbutton');
var todo = document.querySelector('.todo');

addbutton.addEventListener('click', function() {
  if (data.value) {
    var newdiv = document.createElement("div"); // Create a <button> element
    newdiv.classList.add("databox", "col", "s6", "waves-effect");
    //console.log(newdiv);
    todo.appendChild(newdiv);
    //console.log(newdiv.parentNode);
    var newpar = document.createElement("p");
    newpar.classList.add("checkglyph1", "checkglyph2");
    var node = document.createTextNode(data.value + "." + "      " + 
    calendardata.value);
    var newa = document.createElement("a");
    newdiv.appendChild(newa)
    var newglyph = document.createElement("i");
    newglyph.classList.add("material-icons", "checkglyph", "checkglyph1", 
    "checkglyph2", "glyphcolor");
    var node1 = document.createTextNode("check");
    newa.appendChild(newglyph);
    newglyph.append(node1);
    newpar.appendChild(node);
    newdiv.appendChild(newpar);
    data.value = "";
    calendardata.value = "";
    created = true;

    newglyph.addEventListener('click', function(event) {
        event.preventDefault(); // Just to prevent non desired effects of click

        newglyph.classList.toggle('checkglyph1');
    }
   }
 }

And just small clarifications: you don't need the "bind" in your actual code.

glyph[i].addEventListener('click', function.bind(event) { // just type function(event)
   this.classList.toggle('checkglyph1');...

Upvotes: 1

Nicolas M. Pardo
Nicolas M. Pardo

Reputation: 762

What's happening that when they other tasks are created they aren't being collected in the Node Collection, thus they have no event listener. What you can do is instead add the event listener to the container and change whichever item that was clicked in:

document.querySelector('ul').addEventListener('click', changeClass);
document.querySelector('#button').addEventListener('click', addLi);

function changeClass(e){
  e.target.closest('li').classList.toggle('checkglyph1');
}

function addLi(e){
  const new_li = document.createElement('li');
  new_li.textContent = document.querySelectorAll('li').length + 1;
  new_li.classList.add('checkglyph1');
  document.querySelector('ul').appendChild(new_li);
}
li:not(.checkglyph1) {
  background: #f00;
}
<button id="button">Add li</button>
<ul>
  <li class="checkglyph1">1</li>
  <li class="checkglyph1">2</li>
  <li class="checkglyph1 red">3</li>
  <li class="checkglyph1">4</li>
  <li class="checkglyph1">5</li>
  <li class="checkglyph1">6</li>
  <li class="checkglyph1">7</li>
  <li class="checkglyph1">8</li>
  <li class="checkglyph1">9</li>
  <li class="checkglyph1">10</li>
</ul>

Upvotes: 2

Related Questions