CODEWITHSUNDEEP
javascriptlocal-storage
Mohammed
Mohammed

Reputation: 103

Changing values from localstorage on click event isn't always working

I'm doing a to-do list application and I want it show the tasks on the page and also save them in local-storage so it's there when the page is refreshed. I added the option to delete and mark tasks as complete which change the element color and change the value of the 'done' property that is stored in local-storage.

By default the 'done' property is supposed to be false and that changes when you click on the element. One time it works and you can see the value changing on the browser tools, and other times when you click on different elements or delete them it doesn't.

The error that I'm getting is:

TypeError: items[index] is undefined. [line 73 in JS Fiddle]

https://jsfiddle.net/84rkq0yj/1/

This is the function that I used but I couldn't identify the reason items[index] becomes undefined.

function completeList(e) {
  const targetLi = e.target.parentElement;
  const lisArr = Array.from(list.children);
  const index = lisArr.indexOf(targetLi);
  const taskLi = document.querySelectorAll('li');
  taskLi.forEach(li => {
    if(li.contains(e.target)){
      li.classList.toggle('done');
      items[index].done = !items[index].done;
      populateList(items, list);
      localStorage.setItem('items', JSON.stringify(items));
    }
  });
};
list.addEventListener('click', completeList);

Upvotes: 1

Views: 234

Answers (1)

antonku
antonku

Reputation: 7665

The issue may happen when the click event fires on a li element instead of a span.

The following code assumes that event is fired on span element that is a child of li element.

function completeList(e) {
  const targetLi = e.target.parentElement;
  ...
}

When the event is fired for li element, the wrapping div is assigned to targetLi variable and that breaks things.

Adding a check for the type of element in the event handler should help:

function completeList(e) {
  const targetLi = e.target.tagName === 'LI' ? e.target : e.target.parentElement;
  ...
}

Upvotes: 2

Related Questions