Onclick event doesn't work inside the product of another function

Question

My event bound to close's class is working on the li that are included on my HTML, but it is not working on the li's I create with my addItem function. I can't figure out why. Though, I add the span which contains the class & the textNode to each of my new item.

function addCross() {
  let myNodeList = document.getElementsByTagName('li');
  for (let i = 0; i < myNodeList.length; i++) {
    let span = document.createElement('span');
    let cross = document.createTextNode('\u00D7');
    span.className = 'close'
    span.appendChild(cross);
    if (myNodeList[i])
      myNodeList[i].appendChild(span);
  }
}
addCross();

//---------------------------------------------

let close = document.getElementsByClassName('close');
for (let i = 0; i < close.length; i++) {
  close[i].addEventListener('click', function() {
    let cross = close[i].parentElement;
    cross.style.display = 'none';


  });

}

//-----------------------------------------------

function addItem() {
  let ul = document.getElementById('list');
  let inputText = document.getElementById('inputText').value;
  let li = document.createElement('li');
  let textNode = document.createTextNode(inputText)
  let span = document.createElement('span');
  let cross = document.createTextNode('\u00D7');
  span.className = 'close'
  span.appendChild(cross);
  li.appendChild(textNode);
  li.appendChild(span);
  if (inputText === '') {
    alert('enter some task, please')
  } else {
    ul.appendChild(li);
  }

}

```

May be because they are still not loaded in the dom ,try `document.ready()` or `document.load()` — Shubh, Oct 04 '19 at 10:56
I would suggest you to make a function for your `close` procedure/variable. The thing is that you are getting all the elements with `close` class, before creating the new elements, than it does not know about the new created elements. If you use a function instead of let, it will get the elements every time that it is executed, and than it should work. — Marcel Kohls, Oct 04 '19 at 11:02
When i wrap my whole javascript into a window.onload = function(){} it tells me that my addItem function is not defined — Peyo33, Oct 04 '19 at 11:03

explorer · Answer 1 · 2019-10-04T11:20:43.403

The problem is here:

let close = document.getElementsByClassName('close');
for (let i = 0; i < close.length; i++) {
  close[i].addEventListener('click', function() {
    let cross = close[i].parentElement;
    cross.style.display = 'none';


  });

}

The click handler will work! But, when it is called the i will be the last value in the iteration.

Try this:

let close = document.getElementsByClassName('close');
for (let i = 0; i < close.length; i++) {

  const handler = ((index) => {
    return function() {
      let cross = close[index].parentElement;
      cross.style.display = 'none';
    };
  })(i);

  close[i].addEventListener('click', handler);

}

That makes a copy of i value in each iteration & uses that in the click handler.

Update:

The above problem is there in the shared code.

The click handler will not work on the li added by addItem. Because, the event registration happens before the new lis added using addItem. You have 2 options now.

Add one event handler on ul tag & manage the close clicks. Called event delegation.
Add event handler in the addItem function as follows.

  let ul = document.getElementById('list');
  let inputText = document.getElementById('inputText').value;
  let li = document.createElement('li');
  let textNode = document.createTextNode(inputText)
  let span = document.createElement('span');
  let cross = document.createTextNode('\u00D7');
  span.className = 'close'
  span.appendChild(cross);
  span.addEventListener('click', function() {
    let cross = span.parentElement;
    cross.style.display = 'none';
  });
  li.appendChild(textNode);
  li.appendChild(span);
  if (inputText === '') {
    alert('enter some task, please')
  } else {
    ul.appendChild(li);
  }

}

The issue shouldn't be with `i` as they're using `let i = ...` not `var i = ...` — Nick Parsons, Oct 04 '19 at 11:08
Not sure i totally understand your answer. Though, it's not worcking either when i try to use it. Thanks anyway — Peyo33, Oct 04 '19 at 11:10

score 0 · Accepted Answer · edited Oct 04 '19 at 11:10

0

the problem lies in the time it is called if I am not mistaken additem is a function called by an HTML tag correct if so. here is what is happening:

opening the page
loaded all the li and gave them the function
you added the new li

they have the same class name but the function on click for them is not connected to anything solution:

function addclosefunction() {
  let close = document.getElementsByClassName('close');
  for (let i = 0; i < close.length; i++) {
    close[i].addEventListener('click', function() {
      let cross = close[i].parentElement;
      cross.style.display = 'none'
    });
  }
}
addclosefunction();

function addItem() {
  let ul = document.getElementById('list');
  let inputText = document.getElementById('inputText').value;
  let li = document.createElement('li');
  let textNode = document.createTextNode(inputText)
  let span = document.createElement('span');
  let cross = document.createTextNode('\u00D7');
  span.className = 'close'
  span.appendChild(cross);
  li.appendChild(textNode);
  li.appendChild(span);
  if (inputText === '') {
    alert('enter some task, please')
  } else {
    ul.appendChild(li);
  }
  addclosefunction();
}

edited Oct 04 '19 at 11:10

Nick Parsons

31,322
6
25
44

answered Oct 04 '19 at 11:08

Jamal S.

16
3

Won't this register the event on existing elements, which would cause the event handlers to be executed number of the times + 1 times `addclosefunction` is called? – explorer Oct 04 '19 at 11:17
This is the correct solution. I also tried to copy the eventListener's onclick code inside the addItem function, instead of transforming it into a function like you did, and it worcked too. Thanks – Peyo33 Oct 04 '19 at 11:19
@explorer no cause you are overwriting the function for the click button so no problem will appear, although yes it will go through them each time but let be honest the time taken for such an event to go is a fraction of a second so no problem will appear, – Jamal S. Oct 04 '19 at 12:42
@Peyo33 yes it could work inside if you call additem at the loading but if not the defaulted items (that appear at the beginning ) will not have this function embedded – Jamal S. Oct 04 '19 at 12:42

Onclick event doesn't work inside the product of another function

2 Answers2