Simple onclick fails to remove child node

Question

<!DOCTYPE HTML>
<html>
<head>
<meta charset="utf-8">
<title></title>
<script>
window.onload = function() {
  var oBtn = document.getElementById('btn1');
  var oTxt = document.getElementById('txt1');
  var oUl = document.getElementById('ul1');
  var aA = [];

  oBtn.onclick = function() {
    var oLi = document.createElement('li');
    var aLi = document.getElementsByTagName('li');

    // add <a> to li
    oLi.innerHTML = oTxt.value + ' <a href="javascript:;">delete</a>';

    if (aLi.length > 0) {
      oUl.insertBefore(oLi, aLi[0]);
    } else {
      oUl.appendChild(oLi);
    }

    // everytime add an li, add <a> which is in <li> to aA
    aA.push(oLi.children[0]);
  };

  for (var i = 0; i < aA.length; i++) {
    aA[i].onclick = function() {
      oUl.removeChild(this.parentNode);
    }
  }
}
</script>
</head>
<body>
  <input id='txt1' type="text" />

  <!-- Add an li element -->
  <input id="btn1" type="button" value="Add li" />
  <ul id="ul1">
  </ul>
</body>
</html>

Hi everyone, I'm a javascript beginner, and I have a trouble in click event, following is my problem:

When I click oBtn.onclick, it create an li element to ul, then innerHTML of li contains tag "a" and be pushed to an array named aA.

However, in aA[i].onclick, it doesn't work, so what is the problem in my code?

There's no attempt to remove any elements in your click handler. The loop that contains a `removeChild` call will never call it, as at that point `aA` is empty, so the body of the loop never gets executed. — T.J. Crowder, Sep 05 '16 at 15:12
It's not at all clear what you're trying to achieve. State clearly: What code is supposed to do what, what you're doing, what results you're seeing, and what results you expected to see instead. — T.J. Crowder, Sep 05 '16 at 15:15
@Pete: I just completely misread that loop, missed the `onclick` entirely (I never use it). — T.J. Crowder, Sep 05 '16 at 15:19

score 1 · Answer 1 · answered Sep 05 '16 at 15:16

You need to bind the anchor click event after the anchor is created - you try to bind it before they are created

window.onload = function() {
  var oBtn = document.getElementById('btn1');
  var oTxt = document.getElementById('txt1');
  var oUl = document.getElementById('ul1');
  var aA = [];

  oBtn.onclick = function() {
    var oLi = document.createElement('li');
    var aLi = document.getElementsByTagName('li');

    // add <a> to li
    oLi.innerHTML = oTxt.value + ' <a href="javascript:;">delete</a>';

    if (aLi.length > 0) {
      oUl.insertBefore(oLi, aLi[0]);
    } else {
      oUl.appendChild(oLi);
    }

    // everytime add an li, add <a> which is in <li> to aA
   // aA.push(oLi.children[0]);                         <- don't think you need this array anymore
    
    // bind your anchor click here
    oLi.children[0].onclick = function() {
      oUl.removeChild(this.parentNode);
    }
  };
}

  <input id='txt1' type="text" />

  <!-- Add an li element -->
  <input id="btn1" type="button" value="Add li" />
  <ul id="ul1">
  </ul>

Thank for your response! It works! But why oLi.children[0].onclick is contained in oBtn.onclick, they are triggered by different button? — nigel, Sep 05 '16 at 15:26
@nigel, because you create the `oLi.children[0]` inside the `oBtn` so you need to bind the click event when you have created the child - you can't bind it to something that hasn't been created which is why your original failed — Pete, Sep 06 '16 at 07:35

score 1 · Accepted Answer · edited May 23 '17 at 12:08

You're never doing anything to connect an event handler on the a element's click event. See comments:

window.onload = function() {
  var oBtn = document.getElementById('btn1');
  var oTxt = document.getElementById('txt1');
  var oUl = document.getElementById('ul1');
  var aA = [];

  oBtn.onclick = function() {
    var oLi = document.createElement('li');
    var aLi = document.getElementsByTagName('li');

    // Create deletion link and hook it up
    var a = document.createElement('a');
    a.href = "javascript:;";
    a.innerHTML = "delete";
    a.onclick = deleteOnClick;

    // add text and <a> to li
    // note the use of createTextNode, in case the user types something with < in it
    oLi.appendChild(document.createTextNode(oTxt.value + " "));
    oLi.appendChild(a);

    // Add at the beginning of the list
    oUl.insertBefore(oLi, oUl.firstChild);

    // everytime add an li, add <a> which is in <li> to aA
    aA.push(oLi.children[0]);
  };
  
  // Called by click handler to delete the clicked' element's parent
  function deleteOnClick() {
    var parent = this.parentNode;
    parent.parentNode.removeChild(parent);
  }
};

<input id='txt1' type="text" />

  <!-- Add an li element -->
  <input id="btn1" type="button" value="Add li" />
  <ul id="ul1">
  </ul>

Some other notes:

onclick is quick and easy, but it doesn't play nicely with others. Look at using modern event handling (addEventListner). If you need to support obsolete browsers like IE8, see this answer for a function that works even when addEventListener is missing.
Loading scripts in head (without using async or defer) is poor practice. Barring a good reason to do something else, put them at the end of the document body, just before the closing </body> tag.
The load event on window happens very late in the page load process, waiting until after all images and stylesheets and other external resources have finished loading. Barring a good reason to wait that long, hook up handlers earlier. If you put your script at the end of the body (see #2), you can do it right there in the top level of the script.
Wrapping your code in a scoping function like this:
```
(function() {
    // ...your code here...
})();
```
will save you a fair bit of heartache, since otherwise all of your top-level variables and functions are globals. The global namespace is already far too crowded, making conflicts a problem.
Look into the idea of event delegation for your delete handlers. You can handle the click on the ul element, and then check to see if it passed through your a element en route when bubbling. This lets you not worry about hooking it up later when you create the delete link.

Thank for your response! In your code, it seems that there are no order problem in code! I learn a lot from your code, thank you! — nigel, Sep 05 '16 at 15:33

Simple onclick fails to remove child node

2 Answers2