addEventListener() not working on list of elements

moonshineOutlaw picture moonshineOutlaw · Dec 27, 2013 · Viewed 8.1k times · Source

I have an array of paragraph elements. When each <p> is clicked on I want that element removed from the DOM. The code I've written only works when an element is specified by its id. I've tried using the this keyword, as well as looping through the elements without any success as the code below shows.

What am I doing wrong here ? How can I overcome it ?

<div class="section" id='section2'>
<a name="section-two" href="#top">Section Two</a>

        <p id='tester'>Text here <span><img src='images/remove.png' height='20px'/></span></p>
        <p>Text here. <span><img src='images/remove.png' width='20px'/></span></p>
        <p>Text here. <span><img src='images/remove.png' height='20px'/></span></p>
        <p>Text here. <span><img src='images/remove.png' height='20px'/></span></p>
    </div>

var section2 = document.getElementById('section2').getElementsByTagName('p');

for(var i = 0; i < section2.length; i++) {

  section2[i].addEventListener('click', function() {
  section2[i].parentNode.removeChild(section2[i]);
  }, false);
}

Answer

guessimtoolate picture guessimtoolate · Dec 27, 2013

I'd attach a listener to the parent, instead of the children, and check what was clicked. That's a bit more efficient than attaching n-listeners, especially on items you'd remove from the DOM anyway.

var parent = document.getElementById('section2');

parent.addEventListener('click', function (event) {
    if (event.target.tagName === 'P') {
        parent.removeChild(event.target);
    }
}, false);

I made a fiddle to illustrate how I would go on about doing something like that.

For completeness sake: The code you've written won't also work, because you're referencing i variable in a callback. In your case, when the callback would be executed, i would always be equal to the its last value assigned by the loop. You'd need to reassign i to a local variable in the callback to do that (that way you'd have a 'copy' and not a reference). Also if you care for IE lt 8 you'd have to rewrite that to use legacy IE attachEvent api, but I assumed (based on nothing in particular) that you don't need that.