how to remove DOM element from HTML to-do list using splice

I am attempting to implement the splice method to remove items from a JavaScript to do list. With my current code, an item is clearly removed from the list when the remove button is clicked.

However, clicking a remove button also appears to convert the list into a simple array with the default commas. Are there any recommendations on how to set up the remove function properly with the splice method?

Much Appreciated!

var array = [];

function add() {
  var task = document.getElementById("task").value;
  array.push(task);
  var text = document.createTextNode(task);
  var li = document.createElement("li");
  var btn = document.createElement("button");
  btn.appendChild(document.createTextNode("x"));
  btn.setAttribute("onclick", 'remove()');
  li.appendChild(text);
  li.appendChild(btn);
  document.getElementById("myUl").appendChild(li);
}

function remove() {
  array.splice(1, 1);
  document.getElementById("myUl").innerHTML = array;
}

<input id="task">
<button onclick="add()">add</button>
<ul id="myUl"></ul>

Solution

You are mixing up an array of values and the list of elements in the DOM. Your array contains a list of text values of tasks:

var task = document.getElementById("task").value;
array.push(task);

This is simply a list of strings from the input value, but it is not related to the list of elements in your page.

When you are trying to remove a task:

function remove() {
  array.splice(1, 1);
  document.getElementById("myUl").innerHTML = array;
}

You use array.splice(1,1) which according to the docs means:

you are updating your array
you are removing 1 element starting at index 1

Basically every time you call remove() you delete the second item in the array. Definitely not what you want.

Also, you set the array of strings as the innerHTML of the div:

document.getElementById("myUl").innerHTML = array;

This essentially replaces the entire contents of your ul with a list of strings, which is the behavior you are observing. What you want here is to remove the corresponding element from the DOM.

If you want to store an array of tasks at the same time, you have several options. You could keep an array with the list of tasks, this would be a copy of the tasks added and you would need to maintain this list together with your HTML elements. Maintaining two copies of your list of tasks is not robust because you need to manually keep them updated, so it would be safer to derive one of the lists from the other:

when needed, generate your task array from the elements in the dom
have your function update your array, and generate your DOM elements from it.

Depending on your use case, one option might be better than the other, and you might even want to just keep the duplicated array if you are willing to pay the maintainability price in exchange of performance.

Here's a working example, where I use the DOM elements to get the current list of tasks:

function getTasks() {
  return Array.from(document.querySelectorAll('li')).map(l => l.dataset.taskName);
}

function onAddTaskClick() {
 addTask(document.getElementById("task").value);
 console.log(getTasks());
}

function addTask(task) {
  const text = document.createTextNode(task);
  const li = document.createElement("li");
  li.dataset.taskName = task;
  const btn = document.createElement("button");
  btn.appendChild(document.createTextNode("x"));
  // Instead of maintaining an array of elements and using splice,
  // you can use a closure to add an event listener that will remove 
  // itself from the DOM.
  btn.addEventListener('click', () => {
    li.remove();
    console.log(getTasks());
  });
  li.appendChild(text);
  li.appendChild(btn);
  document.getElementById("myUl").appendChild(li);
}

addTask('sampleTask1');
addTask('sampleTask2');
addTask('sampleTask3');
console.log(getTasks());

<input id="task">
<button onclick="onAddTaskClick()">add</button>
<ul id="myUl"></ul>

A few changes I made to the code:

used const for any variables that were not reassigned.
used addEventListener instead of onclick which allows you to use a closure.
used a closure instead of an array, which allows the event handler to have a reference to the li that needs to be removed.
since you don't need the array, I've removed it