CODEWITHSUNDEEP
javascript
nigel
nigel

Reputation: 23

Simple onclick fails to remove child node

<!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?

Upvotes: 1

Views: 1347

Answers (2)

T.J. Crowder
T.J. Crowder

Reputation: 1075239

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:

  1. 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.

  2. 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.

  3. 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.

  4. 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.

  5. 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.

Upvotes: 1

Pete
Pete

Reputation: 58462

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>

Upvotes: 1

Related Questions