-2

I am trying to add a delete option, where you can click on list elements you have added and delete them. I cannot get this to work though. When I try to add delete functionality to the app, I cannot add items to list either. I do not understand why the deleteitem function is not working.

<html langauge="en">
    <head>
        <meta charset="UTF-8">
        <meta name="viewport" content="width-device-width, initial-scale-1.0">
        <link rel="stylesheet" href="todolistcss.css">
        <title>Todolist</title>
        
    </head>

    <body>

        <h1>To Do List</h1>
        <div class="container">
            <input id="inputfield" type="text"><button id="addToDo" onclick="addnew()">Add</button>
            <div class="tO" id="toDoContainers">
                <ul id="todolistitems">
                </ul>
            </div>
        </div>

    <script>


       const newcontainer = document.getElementById("toDoContainers");


       //gets number of list items in todolist

       //adds list item to list

       function deleteItem(paramitem){

        var element = document.getElementById(paramitem);
        element.remove();
       }

       function addnew(){
            let numb = document.getElementById("todolistitems").childElementCount;
            var listitem = document.createElement("li");
            var reallist = document.getElementById("todolistitems");
            var inputvar = document.getElementById("inputfield").value;
            var node = document.createTextNode(inputvar);
            let numbvar = numb +1;

           


            listitem.appendChild(node);
            listitem.setAttribute('id', numbvar);
            listitem.addEventListener('click', deleteItem(listitem));
            reallist.appendChild(listitem);  
            var inputvar = document.getElementById("inputfield").value="";
        //  node.appendChild(document.createTextNode(inputvar));
        /// document.getElementById("toDoContainers").innerHTML=inputvar;
        

            

       }
       
  

    </script>

    </body>
</html>
  • Do you get any errors on the console? What happens? One observation: [`remove()` does not take any parameters](https://developer.mozilla.org/en-US/docs/Web/API/Element/remove), not sure if that would cause it to fail outright, your dev console will show you. – Don't Panic Nov 18 '22 at 22:38
  • 1
    It would be easier for ppl to help if we could see the relevant HTML. And how do you fire those functions, do you have some event handlers somewhere? See https://stackoverflow.com/help/mcve. – Don't Panic Nov 18 '22 at 22:42
  • too many obvious errors. put more work into your question. You have a handful of typos and some messed up logic – DCR Nov 18 '22 at 22:48
  • just edited it. Added HTML and corrected some syntax. – Joseph Foley Nov 19 '22 at 09:25

1 Answers1

2

If you run your code, and look at your browser's devtools console, you will see that trying to add a single item to your list fails:

Uncaught TypeError: Cannot read properties of null (reading 'remove')",

The only remove in the code is the one in the deleteItem() function. How can that function be running when you are adding an item!? Well, you can deduce that this line is not just defining a hander for future action, it is actually running that function immediately:

listitem.addEventListener('click', deleteItem(listitem));

That's not what we want. In fact, the correct way to define an event listener is to specify the callback or function by name, without the () which immediately inovkes it:

listitem.addEventListener('click', deleteItem);

According to the docs, the callback receievs a single parameter, an Event, describing the event which occurred. An Event includes a property target which tells us which element was clicked. This will be useful for us in the next step, let's use it:

function deleteItem(event) {
    event.target.remove();
}

If we can identify the element that was clicked, there's no need to add IDs to try to find them, so we can get rid of all that code.

Putting it all together and simplifying a bit:

function deleteItem(event) {
    event.target.remove();
}

function addnew() {
    var reallist = document.getElementById("todolistitems");
    var inputvar = document.getElementById("inputfield").value;

    var listitem = document.createElement("li");
    var node = document.createTextNode(inputvar);

    listitem.appendChild(node);
    listitem.addEventListener('click', deleteItem);
    reallist.appendChild(listitem);
    document.getElementById("inputfield").value = "";
}
<input id="inputfield" type="text">
<button id="addToDo" onclick="addnew()">Add</button>
<div class="tO" id="toDoContainers">
    <ul id="todolistitems"></ul>
</div>

If you think about it, since we can identify whatever was clicked with the Event, we don't need a handler for every single TODO item. So you can go a step further in simplifying the code, and use just single listener to handle all clicks, instead of adding new ones dynamically every time a TODO is added:

var reallist = document.getElementById("todolistitems");

reallist.addEventListener('click', deleteItem);

function deleteItem(event) {
    event.target.remove();
}

function addnew() {
    var inputvar = document.getElementById("inputfield").value;
    var listitem = document.createElement("li");
    var node = document.createTextNode(inputvar);

    listitem.appendChild(node);
    reallist.appendChild(listitem);
    document.getElementById("inputfield").value = "";
}
<input id="inputfield" type="text">
<button id="addToDo" onclick="addnew()">Add</button>
<div class="tO" id="toDoContainers">
    <ul id="todolistitems"></ul>
</div>

One final bonus point: it is generally considered better practice to separate your Javascript and your HTML. That means using Javascript to add event handlers, rather than adding inline onclick()s. I'd suggest removing your onclick="addnew()", and replacing it with a handler in your JS:

// Add handler
var button = document.getElementById("addToDo");
button.addEventListener('click', addItem);

// Delete handler
var reallist = document.getElementById("todolistitems");
reallist.addEventListener('click', deleteItem);

And of course remove the handler from your HTML:

<button id="addToDo">Add</button>
Don't Panic
  • 13,965
  • 5
  • 32
  • 51