Using removeChild() properly

Question

I have a function that appends a "div" child into a parent node, then I need to delete this child using the removeChild() method, but it doesn't work.

This is my code:

function ColorCards()
        {for (i=0; i<numerocaselle; i++)
            {document.getElementsByClassName("MemoryCards")[i].style.border="none"
            var esempiocolore=document.createElement('div')
            esempiocolore.style="position: relative; height: 80px; width: 130px; background-image: url('img/backcard"+cartaesempio+".png'); background-size: cover;"
            document.getElementsByClassName("MemoryCards")[i].appendChild(esempiocolore)
            }
        }

        function CleanColorCards()
        {for (i=0; i<numerocaselle; i++)
            {document.getElementsByClassName("MemoryCards")[i].style.border="dashed 3px #02A494"
            document.getElementsByClassName("MemoryCards")[i].removeChild(document.getElementsByTagName("div"))
            }
        }

Does somebody have any suggestion on how to make it work?

There is not any HTML because I create all my using another JS function that assigns a class (MemoryCards) to these — Simone Pinto, May 05 '19 at 19:53
There will still be some DOM elements, which you can render the HTML of here, so we can understand the structure — ADyson, May 05 '19 at 19:58

trincot · Answer 1 · 2019-05-05T20:55:32.507

0

You are passing an NodeList to removeChild, while you should pass a single node. Secondly, document.getElementsByTagName("div") is going to also find elements that are not children of the parent you are trying to remove a child from.

So do it like this:

// Avoid repetition of code, and store the result in a variable:
var nodelist = document.getElementsByClassName("MemoryCards");
for (var i=0; i<numerocaselle; i++){
    const parent = nodelist[i];
    parent.style.border="dashed 3px #02A494";
    // Perform `getElementsByTagName` on the parent node only, and take first match:
    parent.removeChild(parent.getElementsByTagName("div")[0]);
}

Note that querySelector is designed for getting one node-result, so that last line can read:

    parent.removeChild(parent.querySelector("div"));

edited May 05 '19 at 20:55

answered May 05 '19 at 19:54

trincot

317,000
35
244
286

Why in the world do people believe that scanning the DOM for all matches, making a node list and then throwing that node list away just to use the first match is a good idea?[Just use `.querySelector()`](https://stackoverflow.com/questions/54952088/how-to-modify-style-to-html-elements-styled-externally-with-css-using-js/54952474#54952474). – Scott Marcus May 05 '19 at 20:14
@ScottMarcus, Sure, `querySelector` avoids creating a node list (added to answer). It is likely from the OP's code, that there is only one child `div` anyway, so the gain (relative to the cost of accessing the DOM) will be very limited. – trincot May 05 '19 at 20:57
It doesn't matter how many matches there are, it matters how big and deep the DOM is, so if you are scanning the entire DOM for just the first matching element, the extra work could be much more than limited. – Scott Marcus May 05 '19 at 21:11
That is true for the OP's code, which did not work anyway. But in my answer only the parent node is searched which suppsedly only has one child. – trincot May 05 '19 at 21:41
But, if the parent node will be completely searched. `.getElementsByTagName()` isn't going to stop at the first match. If the parent node is deep, you've got the same problem. – Scott Marcus May 05 '19 at 21:50

score 0 · Answer 2 · answered May 11 '19 at 22:00

Just a couple notes:

Using a for loop is unnecessary. Having a variable to hold the count of the length of .MemoryCards will leave room for errors. Instead, I recommend an Array function such as .forEach() to iterate through your elements.
The bulk of your element styles should be handled with classes in CSS. By doing this your function will be more concise and easier to manage.
And, to answer your question:
1. To remove all child nodes for each .MemoryCards, I would recommend using a loop and the node.removeChild() method as it will perform faster than setting node.innerHTML=''.
2. See the comments in this post as why this method would be best.

let cartaesempio = 10;
ColorCards = () =>
  Array.from(document.getElementsByClassName("MemoryCards"))
  .forEach(e => {
    e.classList.add('borderNone');
    let esempiocolore = document.createElement('div');
    esempiocolore.style.backgroundImage = `url('https://cdn.dribbble.com/users/285803/screenshots/1066705/dribbble.gif')`;
    e.appendChild(esempiocolore);
  });

CleanColorCards = () =>
  Array.from(document.getElementsByClassName("MemoryCards"))
  .forEach(e => {
    e.classList.add('boderDashed');
    while (e.firstChild) {
      e.removeChild(e.firstChild);
    }
  });

ColorCards();
CleanColorCards();

/* Children of the .MemoryCards nodes */

.MemoryCards div {
  position: relative;
  height: 80px;
  width: 130px;
  background-size: cover;
}

.borderNone {
  border: none;
}

.boderDashed {
  border: dashed 3px #02A494;
}

<div class='MemoryCards'></div>

Hope this helps,

score -1 · Answer 3 · answered May 05 '19 at 19:55

-1

just check your error ! document.getElementsByTagName("div") return an array of div that's basic js meaning you've to search more by ourself. Use a manual, like the one at w3schools or a book whatever :-)

answered May 05 '19 at 19:55

jean3xw

121
7

score -1 · Answer 4 · answered May 05 '19 at 21:01

-1

You are passing removeChild all the divs in the document. Try replacing this line:

document.getElementsByClassName("MemoryCards")[i].removeChild(document.getElementsByTagName("div"))

With:

var memoryCardsEl = document.getElementsByClassName("MemoryCards")[i];
memoryCardsEl.removeChild(memoryCardsEl.getElementsByTagName("div")[0]);

answered May 05 '19 at 21:01

benkol

351
1
13

what if there are some other tags and the divs I dont want to remove? – mars328 May 06 '19 at 01:40
@fengo Then you should use something else and not getElementsByTagName("div") – benkol May 06 '19 at 11:27

Using removeChild() properly

4 Answers4