Delete Button on a To Do List

I have created a To Do List but there’s a problem with delete button that when I try to delete items it doesn’t work and also it doesn’t show any errors on Console Log ?
https://codepen.io/beh4r/pen/ZZJBjy

You have clear variable which the buttons are bound but you are not adding any event listeners to it like this.

btn2.addEventListener('click', addToDoItem)

You need to add the removeEvent function as a listener on those items.

Array.from(clear).forEach(btn => {
  btn.addEventListener('click', removeEvent);
});

Also, you’ll probably need to adjust your removeEvent function because right now it tries to delete e.target from it’s parent, but e.target is just the icon, not the whole todo item

Added this “list.addEventListener(‘click’, removeEvent)”
And it shows alert when I click on delete button but it doesn’t delete it because it shows some other errors?
@dev-nicolaos I added eventListener but it shows me some errors “Failed to execute ‘removeChild’ on ‘Node’: The node to be removed is not a child of this node.”

I often find it helpful in situations like this to place console.log() statements in the code so you can see the value of variables when a function runs (if you’re comfortable using the browsers dev tools you could try setting breakpoints too, but that’s a bit more advanced).

Try logging e.target in your event listener to see what element is actually being used.

Hi @behari97, instead of create a variable with e.target.parentElement you can use directly into the removeChild method

list.removeChild(e.target.parentElement);

or change the name of the var
let list = e.target.parentElement;
with other, something like this: let list2 = e.target.parentElement;,
because you created a global var with the same name and that generate an strange behaviour, i know that you are using let and that kind of var works with his own scope but in this case maybe the removeChild method gets confuse about witch one is.

1 Like

Easy fix.

--ul --------------------
| ---li -----------------
| | text   trashicon |
| -----------------------
-------------------------

when you click the trashcan, e.target becomes the trashcan

get closest li when your target is trashicon.
find the parent of that li ( ul )
remove that li.

if (confirm('Are you sure?')) {
   let li = e.target.closest('li');
   li.parentElement.removeChild(li);
}

Thank you it worked as it removed it :wink:
Just a question about this
“list.removeChild(e.target.parentElement);
list.removeChild(list);”
Why i have to use 2 times list.removeChild ?
If i would use “list.removeChild(e.target.parentElement)”, would that remove parentElement and no need to add another line or i’m wrong?

Anyone can take a look at my code and see why background of items that I add doesn’t change when I click icon of Checked so it would change background color of item?

  1. marked is a HTMLCollection not a single element. You would need to iterate over all of them and attach event listeners. But that won’t work. You have to do the same as what you are doing with remove otherwise it won’t work for new items added. Remove marked and just use the list.

  2. The element tagName is not list it is "I". Also, I’m not sure you really want to change the color of the list. Is it not the list item that needs the color changed?

I think it is better to use a class so you can toggle it.

.item-checked {
  background: yellow !important;
}
function removeEvent(e) {
  if (e.target.classList.contains("fa-trash")) {
    list.removeChild(e.target.parentElement);
  } else if (e.target.tagName === "I") {
    e.target.parentElement.classList.toggle("item-checked");
  }
}

Edit: you should probably rename the function to something else if you do want it to do both the delete and check handling. Maybe handleListItemActions or something.

Just as an aside here is two ways to iterate marked (just to show it)

Summary
const marked = document.getElementsByClassName('fa-check');
Array.from(marked).forEach(el => el.addEventListener('click', itemChecked))

Or using querySelectorAll.

const marked = document.querySelectorAll('.fa-check');
marked.forEach(el => el.addEventListener('click', itemChecked))

it works with classList.toggle but any idea why color of item won’t change when I remove !important from CSS, because with !important some other elements could be affected ?
And about this one (e.target.tagName === “I”)
Can you tell me why list wouldn’t work like (e.target.tagName === list) but “I” it works and does “I” means increment because i am not so good in javascript to be honest

  1. Because the other selectors that set the background color have higher specificity. Using !important should not be a problem for what/how you are using it.

  2. First off tagName gives a string, second the target is an <i> element so the tagName is “I”.

1 Like

Oh got it thought “I” was referring to iterration maybe and was confused.
Thanks for explaining :slight_smile: