0

I have a code:

<ul class='mates'>
  <li class='m' id='1'>Jakub</li>
  <li class='f' id='2'>Vinnie</li>
  <li class='m' id='3'>David</li>
</ul>

This script selects one of the 'li' elements, according to users input:

<script>
var mates = document.getElementsByClassName('mates')[0];
for (var i=0; i< mates.childNodes.length; i++){
    if(mates.children[i].innerHTML == 'Vinnie') alert("Got you! ID "+mates.children[i].id)
}
</script>

And I need to remove this element:

<script>
var mates = document.getElementsByClassName('mates')[0];
for (var i=0; i< mates.childNodes.length; i++){
    if(mates.children[i].innerHTML == 'Vinnie') {
        alert("Got you! ID "+mates.children[i].id);

        parent = document.getElementsByClassName('mates');
        mateToDelete = mates.children[i];

        parent.removeChild(mateToDelete);
    }
}
</script>

This is what I tried in several different ways but I always got error, e.g. " Cannot call method 'removeChild' of undefined". Any ideas?

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
Jakub Zak
  • 1,212
  • 6
  • 32
  • 53
  • 1
    Do note that `childNodes` includes text nodes, such as newlines and indentation. Perhaps you should use `mates.children` instead. – Asad Saeeduddin May 23 '13 at 16:12
  • If you can use jQuery, when you find the element, just use element.remove() – DmitryK May 23 '13 at 16:13
  • 1
    note the "s" in `getElementsByClassName` this indicates that it returns a node list not a single node. So when you call `parent.removeChild(mateToDelete)` you are attempting the remove on a list. – scrappedcola May 23 '13 at 16:15

4 Answers4

4

You already have the parent node from your original getElementsByClassName, and you have your child through the loop that you've just performed.

As such, it's simple:

for (var i=0; i< mates.childNodes.length; i++){
  if(mates.children[i].innerHTML == 'Vinnie'){
    alert("Got you! ID "+mates.children[i].id)
    mates.removeChild(mates.children[i]);
    break;
  }
}

For the sake of completeness (and to prevent further arguing in comments :P), if you are in fact potentially deleting multiple "Vinnie"'s from your list, then it would be better to make a list of those children you want to delete, then delete them after like so:

var toDelete=[],
    i;

for (i=0; i< mates.childNodes.length; i++){
  if(mates.children[i].innerHTML == 'Vinnie'){
    alert("Got you! ID "+mates.children[i].id)
    toDelete.push(mates.children[i]);
  }
}

for (i=0; i<toDelete.length; i++){
  mates.removeChild(toDelete[i]);
} 
Doug
  • 3,312
  • 1
  • 24
  • 31
  • @Asad That's why the break is there after the delete. – slinky2000 May 23 '13 at 16:16
  • @slinky2000 Yes, I've already deleted my comment. – Asad Saeeduddin May 23 '13 at 16:17
  • The break may not be required. What if the case arises that there are two "mates" named Vinnie? Suggest adding a comment to your code to say that it will only match on one occurrence of "Vinnie". – Michael Coxon May 23 '13 at 16:19
  • @MichaelCoxon In that case, the identified nodes should be pushed into an array and deleted after the loop. – Asad Saeeduddin May 23 '13 at 16:20
  • @MichaelCoxon The question isn't about that. – slinky2000 May 23 '13 at 16:21
  • Haha I wasn't trying to be a pain, just assumed the OP was beginner JS so that the break keyword in a for may have confused him. Just thought it to be a good idea to explain what was going on there is all. Just noticed an issue that tymeJV has pointed out, you will need to change your childNodes to children in the for statement. – Michael Coxon May 23 '13 at 16:26
1

You don't need that parent variable. Delete it using this:

mates.removeChild(mateToDelete);

Fiddle: http://jsfiddle.net/3XeM5/2/

I also modified your for-loop to use:

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

The length of this (children.length) is 3, the length of childNodes is 7, so if nothing is found the loop will break!

Edit: If you want to delete multiple iterations of a specific element, remove the break; in the if-logic. If you're only looking for the first, leave the break.

tymeJV
  • 103,943
  • 14
  • 161
  • 157
  • It seems it does work. But how so? MDN says parent node is needed here: https://developer.mozilla.org/en-US/docs/Web/API/Node.removeChild?redirectlocale=en-US&redirectslug=DOM%2FNode.removeChild – Jan Turoň May 23 '13 at 16:26
  • You already have a variable that contains the children of what you want. There is no need to declare a second variable. – tymeJV May 23 '13 at 16:27
0

Use This:

mates.removeChild(mates.children[i]);

Example: http://jsfiddle.net/t9nCT/1/

slinky2000
  • 2,663
  • 1
  • 15
  • 12
-1

You're retrieving a collection, so do,

parent[0].removeChild(mateToDelete);
painotpi
  • 6,894
  • 1
  • 37
  • 70
  • There is no need to use the parent object created because it already exists as "mates". – Michael Coxon May 23 '13 at 16:16
  • How's my answer wrong though? I will/would edit it at some point, just getting a quick answer in – painotpi May 23 '13 at 16:18
  • 2
    I am only a beginner at SO, so correct me if I am wrong, but isn't good practice and solid code in the first place better than "just getting a quick answer in"? – Michael Coxon May 23 '13 at 16:33
  • @MichaelCoxon well you are correct, partly my mistake as well, didn't read the code completely. But, generally, people get an answer in and tend to improve on that :) Also, only wrong answers are downvoted afaik, partly right answers are given comments to improve on – painotpi May 23 '13 at 17:06