Search code examples
javascripthtmlcssdom-manipulation

Issue with Adding New Element in the DOM with Javascript


I am new to javascript and I am trying to create a shopping list with HTML, CSS and Javascript. Basically, there is already a pre existing list on it but you can add a new list just by typing it to the input field and delete it by pressing the delete button.

I am trying to add a function(emptyList()) which adds an h2 element with the content "Empty List" when the list is empty. But if you have added more than 1 item to the list beforehand then deletes it afterwards, depending on how many list you've added, the h2 elements that were added is also the same amount with that of the list.

I called the emptyList() function in the deleteListElement() function for now. I have tried calling it outside of it, as well as on the createListElement() function.

Expectation: enter image description here

Actual: enter image description here

Disregard the CSS for now.

Here are my codes:

var addButton = document.getElementById("addButton");
var input = document.getElementById("userinput");
var ul = document.querySelector("ul");
var li = document.getElementsByTagName("li");
var deleteButton = document.getElementsByClassName("deleteButton");
var i = document.getElementsByTagName("i");
var secondSection = document.querySelector('.second-section');

function inputLength() {
    return input.value.length;
}


function deleteElement() {
    var h2 = document.querySelector("h2");
    if (li.length > 0){
        h2.parentNode.removeChild(h2);
        
    }
}

// for (let index = 0; index == li.length; index++) {
//  if (li.length == 0){
//      h2[index].remove();
//  }
// }


function createListElement() {
    //add list
    var li = document.createElement("li");
    li.append(document.createTextNode(input.value));
    li.classList.add("item", "zone");
    li.addEventListener("click", crossOutListWhenClicked);
    ul.appendChild(li);
    input.value = ""; //resets the input field
    addDeleteButtonToList();
    deleteElement();

    function addDeleteButtonToList(){
        var button = document.createElement("deleteButton");
        var i = document.createElement("i");
        i.classList.add("fa-solid", "fa-trash-can");
        button.classList.add("deleteButton");
        button.appendChild(i);
        button.addEventListener('click', deleteListElement);
        li.appendChild(button);
    }
    
}

function deleteListElement(){
    var deletes = document.querySelectorAll('.deleteButton');
    // Iterate all nodes
    deletes.forEach(btn => {
        // Attach event listener. Note to preserve the button this-reference
        // by using the non-shorthand function
        btn.addEventListener('click', function() {
            var li = this.parentNode
            li.remove();
            EmptyList();
            
        })
    })
    
}

function addListAfterClick() {
    if (inputLength() > 0) {
        createListElement();
    }
}

function addListAfterKeypress(event) {
    if (inputLength() > 0 && event.keyCode === 13) {
        createListElement();
    }
}


function crossOutListWhenClicked(){
    var li = document.querySelectorAll("li");
    // for (let index = 0; index < li.length; index++) {
    //  li[index].addEventListener("click", function(){
    li.forEach(x =>
        x.addEventListener("click", function(){
            x.classList.toggle('done');
        }))
    }

function EmptyList(){
    var h2 = document.createElement("h2");
    if (li.length == 0){
        secondSection.appendChild(h2);
        h2.append(document.createTextNode("Empty List"));
        console.log(h2);
    }
}

deleteListElement();
crossOutListWhenClicked();
addButton.addEventListener("click", addListAfterClick);
input.addEventListener("keypress", addListAfterKeypress);
body {
  margin: auto 20rem;
  height: 100%;
  
}

.container {
  background-color: #E38B29;
  margin: 0 auto;
  
}
.zone {
  display: flex;
  cursor: pointer;
  background-color: white;
  font-size: 1em;
  border: 0;
  transition: all 0.3s linear;
  

}

#main-head {
  color: #FDEEDC;
  display: flex;
  justify-content: center;
  align-items: center;
  border: none;
  border-radius: 0;
  height: 10vh;
}

.first-section {
  display: flex;
  align-items: center;
  justify-content: center;
  font-size: 2em;
  height: 10vh;
  border: none;
  border-radius: 0;
}

.second-section {
  height: 80vh;
  border-radius: 20px 20px 0px 0px;
  border-color: white;
  border: 2px solid #E38B29;
}

ul{
  display: flex;
  justify-content: flex-start;
  align-items: flex-start;
  flex-wrap: wrap;
  flex-direction: column;
  height: 100%;
  overflow: auto;
  margin-top: 15px;
  background-color: #E38B29;
  list-style: none;
  padding: 0;
}

ul > .zone{
  border: none;
}

h2{
  margin: 0 auto;
}

li{
  margin-left: 20px;
}


.gradient {
  background-color: #E38B29;
}


.done {
  text-decoration: line-through;
}
<!DOCTYPE html>
<html>
<head>
    <title>Javascript + DOM</title>
    <link rel="stylesheet" type="text/css" href="style.css">
    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.1.2/css/all.min.css">
</head>
<body>
    <div class="container">
        <header class="zone gradient" id="main-head">
            <h1>Shopping List</h1>
        </header>

        
        <section class="zone first-section gradient">
            <!-- <p id="first">Get it done today</p> -->
            <input id="userinput" type="text" placeholder="enter items">
            <button id="addButton">Enter</button>
        </section>
        
        <section class="zone second-section">
            <ul class="zone">
                <li class="item zone">Notebook
                    <button class="deleteButton"><i class="fa-solid fa-trash-can"></i></button>
                </li>
                <li class="item zone">Apple
                    <button class="deleteButton"><i class="fa-solid fa-trash-can"></i></button>
                </li>
            </ul>
        </section>
            <script type="text/javascript" src="script.js"></script>
    </div>

</body>
</html>


Solution

  • The problem is deleteListElement. This function is called on page load to attach click handlers on all existing delete buttons.

    But then, you also use this function as event handler on new delete buttons. An event handler that itself adds event handlers is often a code smell. And also here, it is wrong. This means that when the user adds a new item, all exisiting delete buttons also get a new click handler. These buttons thus get more than one click handler, and this explains why you get these multiple "Empty List" titles.

    So split this function into two:

    • One function to just handle the click of one button.
    • Another function to deal with the buttons that are already present on page load.

    Unrelated, but there was also a problem in deleteElement: it should check whether the h2 element was present. If not, it should do nothing.

    I have put comments where the code was changed. I did not test for other problems though... I just focused on the problem you raised and the one that immediately caused an error when I added an item (the one I mentioned about a missing h2 element).

    var addButton = document.getElementById("addButton");
    var input = document.getElementById("userinput");
    var ul = document.querySelector("ul");
    var li = document.getElementsByTagName("li");
    var deleteButton = document.getElementsByClassName("deleteButton");
    var i = document.getElementsByTagName("i");
    var secondSection = document.querySelector('.second-section');
    
    function inputLength() {
        return input.value.length;
    }
    
    function deleteElement() {
        var h2 = document.querySelector("h2");
        // Add a condition to see H2 exists
        if (h2 && li.length > 0){
            h2.parentNode.removeChild(h2);
        }
    }
    
    function createListElement() {
        var li = document.createElement("li");
        li.append(document.createTextNode(input.value));
        li.classList.add("item", "zone");
        li.addEventListener("click", crossOutListWhenClicked);
        ul.appendChild(li);
        input.value = "";
        addDeleteButtonToList();
        deleteElement();
    
        function addDeleteButtonToList(){
            var button = document.createElement("deleteButton");
            var i = document.createElement("i");
            i.classList.add("fa-solid", "fa-trash-can");
            button.classList.add("deleteButton");
            button.appendChild(i);
            // Call common function to attach ONE handler
            activateDeleteAction(button);
            li.appendChild(button);
        }   
    }
    
    // New function. Common for 2 use cases:
    // - for existing delete buttons, on page load
    // - for newly added delete buttons, on user action
    function activateDeleteAction(btn) {
        btn.addEventListener('click', function() {
            var li = this.parentNode;
            li.remove();
            EmptyList();
        })
    };
    
    // Renamed this function:
    function activateAllDeleteActions(){
        var deletes = document.querySelectorAll('.deleteButton');
        // Use other function to attach handler
        deletes.forEach(activateDeleteAction);
    }
    
    function addListAfterClick() {
        if (inputLength() > 0) {
            createListElement();
        }
    }
    
    function addListAfterKeypress(event) {
        if (inputLength() > 0 && event.keyCode === 13) {
            createListElement();
        }
    }
    
    
    function crossOutListWhenClicked(){
        var li = document.querySelectorAll("li");
        li.forEach(x =>
            x.addEventListener("click", function(){
                x.classList.toggle('done');
            }))
        }
    
    function EmptyList(){
        var h2 = document.createElement("h2");
        if (li.length == 0){
            secondSection.appendChild(h2);
            h2.append(document.createTextNode("Empty List"));
        }
    }
    
    activateAllDeleteActions(); // Calling renamed function
    crossOutListWhenClicked();
    addButton.addEventListener("click", addListAfterClick);
    input.addEventListener("keypress", addListAfterKeypress);
    body {
      margin: auto 20rem;
      height: 100%;
      
    }
    
    .container {
      background-color: #E38B29;
      margin: 0 auto;
      
    }
    .zone {
      display: flex;
      cursor: pointer;
      background-color: white;
      font-size: 1em;
      border: 0;
      transition: all 0.3s linear;
      
    
    }
    
    #main-head {
      color: #FDEEDC;
      display: flex;
      justify-content: center;
      align-items: center;
      border: none;
      border-radius: 0;
      height: 10vh;
    }
    
    .first-section {
      display: flex;
      align-items: center;
      justify-content: center;
      font-size: 2em;
      height: 10vh;
      border: none;
      border-radius: 0;
    }
    
    .second-section {
      height: 80vh;
      border-radius: 20px 20px 0px 0px;
      border-color: white;
      border: 2px solid #E38B29;
    }
    
    ul{
      display: flex;
      justify-content: flex-start;
      align-items: flex-start;
      flex-wrap: wrap;
      flex-direction: column;
      height: 100%;
      overflow: auto;
      margin-top: 15px;
      background-color: #E38B29;
      list-style: none;
      padding: 0;
    }
    
    ul > .zone{
      border: none;
    }
    
    h2{
      margin: 0 auto;
    }
    
    li{
      margin-left: 20px;
    }
    
    
    .gradient {
      background-color: #E38B29;
    }
    
    
    .done {
      text-decoration: line-through;
    }
    <!DOCTYPE html>
    <html>
    <head>
        <title>Javascript + DOM</title>
        <link rel="stylesheet" type="text/css" href="style.css">
        <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.1.2/css/all.min.css">
    </head>
    <body>
        <div class="container">
            <header class="zone gradient" id="main-head">
                <h1>Shopping List</h1>
            </header>
    
            
            <section class="zone first-section gradient">
                <!-- <p id="first">Get it done today</p> -->
                <input id="userinput" type="text" placeholder="enter items">
                <button id="addButton">Enter</button>
            </section>
            
            <section class="zone second-section">
                <ul class="zone">
                    <li class="item zone">Notebook
                        <button class="deleteButton"><i class="fa-solid fa-trash-can"></i></button>
                    </li>
                    <li class="item zone">Apple
                        <button class="deleteButton"><i class="fa-solid fa-trash-can"></i></button>
                    </li>
                </ul>
            </section>
                <script type="text/javascript" src="script.js"></script>
        </div>
    
    </body>
    </html>