Local storage - wrong list items appear on refresh

Question

I've created a to-do list with local storage. If you create three list items and delete the second one, the deleted list item will reappear in place of the third item on refresh.

Edit: I'm not sure whether it's to do with local storage or with the original todo array. In the code below, I'm trying to remove the relevant value from the array, but I suspect this isn't working (logging the array to the console produces no result).

Although it has nothing to do with local storage, I think the issue lies with the following code:

function removeItem() {
  let item = this.parentNode.parentNode;
  let parent = item.parentNode;
  let id = parent.id;
  console.log(id)
  let value = parent.textContent;
     todo.splice(todo.indexOf(value, 1));
  this.parentNode.parentNode.removeChild(this.parentNode);    
  saveTodos();
}

Edit: Here is the code I used to store the list items:

function saveTodos() {
  let jsonstr = JSON.stringify(todo);
  localStorage.setItem('todo', jsonstr);
}

    function getTodos() {
      localStorage.getItem('todo')
      let jsonstr = localStorage.getItem("todo");
      todo = JSON.parse(jsonstr);
      if (!todo || !todo.length) {
        todo = [];
      }
      else {
        renderTodoList();

} }

Here is a link to the codepen: https://codepen.io/david-webb/pen/yLeqydK

Can you help?

Please include a [mcve] _in the question itself_ ([preferable as snippet `<>`](https://meta.stackoverflow.com/questions/358992/ive-been-told-to-create-a-runnable-example-with-stack-snippets-how-do-i-do)) and not only a link to an external resource that might not be reachable (for whatever reason) -> [How do I ask a good question?](https://stackoverflow.com/help/how-to-ask) — Andreas, Sep 25 '20 at 09:22
None of the code that you have included in the question itself has anything to do with local storage whatsoever — Aluan Haddad, Sep 25 '20 at 09:24
looks good , local storage works fine for me with your code pen .... to see what is stored go to dev tools -- > Application --> Local Storage on left , while removing get the selected index and splice accordingly which you are doing wrong ... — KcH, Sep 25 '20 at 09:24
@Aluan Haddad Thanks for your comment, you're right. I've added more detail to try and explain the issue better — David Webb, Sep 25 '20 at 09:34
I believe the problem is that you don't update the array after you remove the item. Every function that modifies the array should return the updated array. Functions that return values and their e consistent consumption makes code much easier to follow and reason about — Aluan Haddad, Sep 25 '20 at 09:54

score 2 · Accepted Answer · answered Sep 25 '20 at 10:01

This is because the current code seems to be removing the wrong item.

See scenario:

Localstorage: ["t","1", "2"];

-> Remove item #2 ("t")

Localstorage: ["t", "1"];

Output:

As you can see, the output shows ["t", "2"] thought the localstorage array is ["t", "1"].

This is because of the flawed logic in the removeItem function.

Try with this, instead.

//remove list item on click
function removeItem() {
  const item = this.parentNode;
  const value = this.parentNode.lastChild.textContent;
  
  todo = todo.filter(t => t !== value);
  this.parentNode.parentNode.removeChild(item);   
  saveTodos();
}

fiddle:

<input type = "text" style="font-size:25px;" id = "input" placeholder="Write here">

<button id = "addBtn" >Add item</button>

<ul id = "myUL">
</ul>

<script>
let todo = [];

renderTodoList();

document.getElementById('addBtn').addEventListener('click', function () {
  let value = document.getElementById('input').value;
  if (value) {
    todo.push(value);
    saveTodos()
    addInput(value);
  }
});

input.addEventListener("keypress", function(event) {
// Number 13 is the "Enter" key on the keyboard
if (event.keyCode === 13) {
  // Trigger the button element with a click
  document.getElementById("addBtn").click();
}
});


function addInput(text) {
  //add list item on click
  let listItem = document.createElement('li');
  let list = document.getElementById('myUL');
  let input = document.getElementById('input').value;
  let textNode = document.createTextNode(text);

  //create and append remove button
  let removeBtn = document.createElement("BUTTON");
  list.appendChild(removeBtn);
  removeBtn.className = "removeBtn";
  removeBtn.innerHTML = "Remove item";
  listItem.appendChild(removeBtn);
  list.appendChild(listItem);
  listItem.appendChild(textNode);
  document.getElementById("input").value = "";
  removeBtn.addEventListener('click', removeItem);
  console.log(todo);
}

//remove list item on click
function removeItem() {
  const item = this.parentNode;
  const value = this.parentNode.lastChild.textContent;
  
  todo = todo.filter(t => t !== value);
  this.parentNode.parentNode.removeChild(item);   
  saveTodos();
}

function renderTodoList() {
  if (!todo) return
  for (let i = 0; i < todo.length; i++) {
    let value = todo[i];
    addInput(value);
    console.log(value);
  }
}

function saveTodos() {
  let jsonstr = JSON.stringify(todo);
  localStorage.setItem('todo', jsonstr);
}

function getTodos() {
  localStorage.getItem('todo')
  let jsonstr = localStorage.getItem("todo");
  todo = JSON.parse(jsonstr);
  if (!todo || !todo.length) {
    todo = [];
  }
  else {
    renderTodoList();
  }
}



//cross out text on click
/*document.addEventListener('click', function (ev) {
  if (ev.target.tagName === 'LI') {
    ev.target.classList.toggle('checked');

  }
});*/

//renderTodoList();
getTodos();
</script>

score 0 · Answer 2 · answered Sep 25 '20 at 10:13

0

I think the problem is the usage of splice and indexOf.

For splice -- pass index, how may delete, new item

var todo = ["a", "b", "c"];
var value = "b"

// your code
todo.splice(todo.indexOf(value, 1));
console.log(todo)


var todo = ["a", "b", "c"];
var value = "b"
// correct way to delete
todo.splice(todo.indexOf(value), 1);

console.log(todo)

answered Sep 25 '20 at 10:13

Siva K V

10,561
2
16
29

thanks for pointing out the issue with the splice method. Although it wasn't the only issue, it was certainly one of them! – David Webb Sep 25 '20 at 12:06

score -1 · Answer 3 · answered Sep 25 '20 at 10:04

This line has the error todo.splice(todo.indexOf(value, 1));

The reason is when you apply let item = this.parentNode.parentNode; you the UL element in the variable.

Fix:

While adding the item in addInput() create a span and put the text inside the span rather than creating textNode.

when removing from todo then you should use the innerText inside SPAN tag todo.splice(todo.indexOf(value, 1));

In the value variable you should have the todo item name.

Local storage - wrong list items appear on refresh

3 Answers3