Question 241265 in Html Question posted in
Some basic documentation can be found here.

Html – getElementsByClassName() not working as expected

I want the following code to create a close button and append it to each list item. But since I have more than one list, I want it to only apply to the list with a specific class name, for example class="ToDo-List".

I am creating a To Do list where the user can add and delete items.

I tried using getElementsByClassName() but it doesn’t seem to work. I don’t know if I’m applying it the wrong way or the problem is in the HTML.

Below is the code I currently have:

HTML (where should I place the class, in each li element?):

<div id="myDIV" class="to-do-header">
    <h2 class="table-title">To Do</h2>
    <input type="text" id="myInput" placeholder="What do you want to do?">
    <span onclick="newElement()" class="addBtn">Add</span>
  </div>
  
  <ul id="myUL">
    <li>Wake up</li>
    <li>Meeting with the team</li>
    <li>Have an amazing lunch</li>
    <li>Finsish the project</li>
    <li>Get some pizza</li>
    <li>Go to sleep</li>
  </ul>

JavaScript (the code I’m referring to is the first section but I didn’t know if the rest might have something to do with the problem so I included all the code for that list):

// Create a "close" button and append it to each list item
var myNodelist = document.getElementsByTagName("LI");
var i;
for (i = 0; i < myNodelist.length; i++) {
  var span = document.createElement("SPAN");
  var txt = document.createTextNode("u00D7");
  span.className = "close";
  span.appendChild(txt);
  myNodelist[i].appendChild(span);
}

// Click on a close button to hide the current list item
var close = document.getElementsByClassName("close");
var i;
for (i = 0; i < close.length; i++) {
  close[i].onclick = function() {
    var div = this.parentElement;
    div.style.display = "none";
  }
}

// Add a "checked" symbol when clicking on a list item
var list = document.querySelector('ul');
list.addEventListener('click', function(ev) {
  if (ev.target.tagName === 'LI') {
    ev.target.classList.toggle('checked');
  }
}, false);

// Create a new list item when clicking on the "Add" button
function newElement() {
  var li = document.createElement("li");
  var inputValue = document.getElementById("myInput").value;
  var t = document.createTextNode(inputValue);
  li.appendChild(t);
  if (inputValue === '') {
    alert("You must write something!");
  } else {
    document.getElementById("myUL").appendChild(li);
  }
  document.getElementById("myInput").value = "";

  var span = document.createElement("SPAN");
  var txt = document.createTextNode("u00D7");
  span.className = "close";
  span.appendChild(txt);
  li.appendChild(span);

  for (i = 0; i < close.length; i++) {
    close[i].onclick = function() {
      var div = this.parentElement;
      div.style.display = "none";
    }
  }
}
Tags:

2

Answers


  1. 0

    Problem you have is document.getElementsByTagName selected every tag on the page. If you want to limit it to just the one list, you need to either select the list with getElementById first OR use querySelectorAll()

    const list = document.getElementById("myUL");
const listItems = list.getElementsByTagName("li");

// or better option

const listItems = document.querySelectorAll("#myUL li");

    You have a lot of repetitive code. Pull it out into reusable functions so you are not copy and pasting things. Simplify your events with event delegation.

    function addCloseBtn (li) {
  const button = document.createElement("button");
  const txt = document.createTextNode("u00D7");
  button.className = "close";
  button.appendChild(txt);
  li.appendChild(button);
}

function createLi (ul, text) {
  const li = document.createElement("li");
  const txt = document.createTextNode(text);
  li.appendChild(txt);
  addCloseBtn(li);
  ul.appendChild(li);
}

// Loop over the existing li elements and add the close button
document.querySelectorAll("#myUL li").forEach(addCloseBtn);

// find your list
const myList = document.querySelector("#myUL");

// add a click handler to the elemenet and we will use event delegation
myList.addEventListener("click", function (event) {
  // look if the what was clicked was our button
  const btn = event.target.closest('button.close');
  
  // if e have the button remove it
  if (btn) btn.closest('li').remove();
});


function newElement(){

  // get the text
  const text = document.querySelector('#myInput').value.trim();
  
  /// verify they added text
  if (!text) {
    alert('error');
    return;
  }
  
  // create your li
  createLi(myList, text);
}
    .close {
  margin-left: 1em;
}
    <div id="myDIV" class="to-do-header">
  <h2 class="table-title">To Do</h2>
  <input type="text" id="myInput" placeholder="What do you want to do?">
  <button onclick="newElement()" class="addBtn">Add</button>
</div>

<ul id="myUL">
  <li>Wake up</li>
  <li>Meeting with the team</li>
  <li>Have an amazing lunch</li>
  <li>Finsish the project</li>
  <li>Get some pizza</li>
  <li>Go to sleep</li>
</ul>
    Login or Signup to reply.
  2. 0

    The code you’ve provided does seem to work on my machine (using Firefox), at least on its own. Adding a second duplicate list section seems to portray the problem you’re describing, where it’s only adding items to a single list.

    I would recommend using a similar technique you did with your removal events. In newElement(), use the event given to you (making your function newElement(event) instead) and operating on the appropriate elements relative to that event’s target.

    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search