Delete function targets wrong id?

So i am making my to-do app and i have encountered this weird problem where my removeToDo function targets the ID of an item in an array like this:
clickedItemID + amount of times i clicked on an list item(any in the array)
If i click on item with id 1, it deletes fine. Then i click item with id 3, deletes item with id 4 (adds 1 to id because of 1 xclick), then let’s say i click item with id 5 and it deletes item with ID 7 (as of 2 x click ?) I may be wrong but that’s how it looks like … here’s some code :


	<div class='app'>
		<div class='header'>
			<img src="./images/header.jpg"/>
			<div id="date">DATA</div>
			<div id="time"></div>
		<div id='listTop'>
			<i class="fas fa-sync-alt" id="clear"></i>
			<p id='pClear'>Clear All</p>
		<ul id='list'>

		<div class="footer">
			<i class="fas fa-plus-circle" aria-hidden="true" id='addButton'></i>
			<input type="text" id='itemInput' placeholder="Add a to-do" />
<script src="./app.js"></script>


const clear = document.querySelector("#clear");
const date = document.getElementById("date");
const list = document.getElementById("list");
const input = document.getElementById("itemInput");

const check = "fa-check-circle";
const uncheck = "fa-circle";
const lineThrough = "lineThrough";

let id;

//get the item from the local storage
let data = localStorage.getItem('TODO');

//check if data is not empty
if(data) {
	LIST = JSON.parse(data)
	id = LIST.length; // set the list id to the last in the array
	loadList(LIST); // load all the items in the array to the UI 
} else {
	//if data is empty
	LIST = [];
	id = 0;

function loadList(array) {
	array.forEach(item => {
		addToDo(,, item.done, item.trash);

function addToDo(toDo, id, done, trash) {
	// if trash is true do not execute the code below
	if (trash) {return ;}
	const DONE = done ? check : uncheck;
	const LINE = done ? lineThrough : "";
	const text =`
	<li class="item">
		<i class="far ${DONE}" id='${id}'></i>
			<div class="description ${LINE} wrap">${toDo}</div>
		<i class="fas fa-trash-alt" id='${id}'></i>
	const position = "beforeend";
	list.insertAdjacentHTML(position, text);		

// remove to-do
function removeToDo(element) {
	LIST[].trash = true;
	LIST.splice(, 1);  // <----- I think that is the problem ?

// click listener for job complete and job delete
list.addEventListener('click', e => {
	const element =;
	if( == "fas fa-trash-alt" ){
	}else if( == "far fa-circle") {
	}else if( == "far fa-check-circle"){



//add a task with "enter" key
document.addEventListener("keyup", (event) => {
	if(event.keyCode == 13){
		const toDo = input.value;
			if(toDo) {
				addToDo(toDo, id, false, false);
						name: toDo,
						id: id,
						done: false,
						trash: false
				localStorage.setItem('TODO', JSON.stringify(LIST));
				input.value = '';

Hello there,

You are 100% correct, this is your problem:

LIST.splice(, 1);

Think about it:

  1. You have an array of length x
  2. You remove 1 element. Array becomes length x-1.

The use of the ids for the first splice makes sense, as they are the same as the indices, but the minute you alter the array, the indices and ids are no longer the same.

Hope this helps

So does the array need an update after every removal ? Or should i use something else than splice ?

Also is it possible that

list.addEventListener('click', e => {
	const element =;
	if( == "fas fa-trash-alt" ){
	}else if( == "far fa-circle") {
	}else if( == "far fa-check-circle"){



is stacking click events on itself ?

You have a few options commonly used:

  1. Use the indexOf() method.
  2. Loop through the array, and splice if element has id id.

Hope this helps.

Why do you think that is stacking event listeners? Is that code snippet inside of a loop? What you might want to do, is, before splicing the element, use removeEventListener on the element.

I think it’s stacking event listeners because even if i get the index of id it was still deleting wrong id further within the app. Like if i removed one item, it counts as firing event listener one time, so if i would try to remove three items, it would fire it three times in one click after that, resulting in index +2 ? It’s a bit of a speculation but i will try to add removeEventListener in a sec to see if it works.


So removeEventListener doesn’t do much, i didn’t observe any change…
But i have changed the code earlier on a bit, am i getting any closer to solving this ? :slightly_smiling_face:

// remove to-do
function removeToDo(element) {
	let newList = [...LIST]
		let children = element.parentNode.childNodes;
		for (i = 0; i < children.length; i++) {
		  if (element == children[i]) break;
	newList[i].trash = true;  
	newList.splice(i, 1);
	LIST = newList;


What is the purpose of this:

for (i = 0; i < children.length; i++) {
  if (element == children[i]) break;

It was suppose to iterate through the array to get index of the clicked item ? I am guessing it’s wrong :laughing: Sorry i have spent most of the day trying to figure it out and i guess at the end i started adding stuff i found on google that looked like it may work :frowning_face: it returns correct id values of a clicked item though

No need to apologise. I just ask, because the for loop loops through the array, until the element is the same as the child. It is just an inefficient way to do indexOf(). I suggest you research the use of indexOf.

Ahh fair enough, i tried indexOf as well but probably haven’t tried it very well. Anyway the problem still persists even if i am getting the right values when i click on the item, anything else i could try ?

EDIT: Ok i made indexOf to work properly, its getting right id values, but problem with the wrong items in the array getting removed persists :frowning:

Would you be able to share an up-to-date CodePen, or JSFiddle, or similar of your code?

If not, could you paste the current code you have?


Never shared a CodePen yet so let me know if it works

It looks a bit rough, but it seems to function as it was

Thank your, for doing that. The functionality works as I would expect…
I can see that the alert logs -1 for any items deleted, if they were created after deleting a previous item. But, the correct item still gets deleted.

Have you changed anything?

That is weird, i was getting that -1 but that was AFTER i deleted few items (and sorry i forgot to actually mention that), and that’s how it works in Studio code right now. Just checked the code and everything seems exactly the same ?

EDIT: Ok hang on it doesn’t now O_o
EDIT2: And now it does again, what the hell…
EDIT3… : Ok so anything created after deleting any item, gets the index-1. If you do “clear all” it deletes all items in the array and clears the storage, so could it be that the local storage is cause of the index problem ?

I have never used localStorage before, and am confused about the alert, considering the console logs the array with the correct ids.

Just did a small test - had numbers 1 - 8 with ID’s 0 - 7.
Clicked to delete item with number 4 (id = 3), deleted it right from the array and console logged id number 3, which was right. Then clicked to delete item with number 5 (id = 4), deleted number 6 instead with id = 5, but the console logged id 4 ?

Instead of having a top-level id variable that you are passing around and incrementing, why not generate a unique id using something like uuid and filter the array using that id.

LIST = LIST.filter(todo => !==;

You are also not updating localStorage in removeToDo.

@lasjorg Well i am a complete beginner, that uuid thing seems a bit complicated ? I am still learning basic javascript so i don’t want to confuse my brain with some libraries …
As for updating localStorage, that was the plan when i sort out problem with id’s as i need to learn how to do that first, but might be worth doing it now …

It’s just a function you call that gives you a guaranteed unique string, you call it uuidv4() and it returns a string like this “71b05b68-7e94-40fd-bcfe-97c925cd7b48”.

Using it is in fact much less complicated then what you are doing now. Each todos id will be guaranteed to have a fixed unique id that you can use in an array filter.

I didn’t really test this code much but just to show what I mean.

@lasjorg You have fixed all my problems there, good Sir! It is much simpler and efficient, it doesn’t look like my Visual studio code recognizes the uuidv4() though, do i need to install something ? Also could i actually achieve what i wanted to with the way i was doing this ?

You can just link to the script file in the HTML

I’m sure you might. But there is really no reason to have a “floating dynamic value” (i.e. a value being passed around and being changed) that you need to keep track of when all you need is just a fixed unique value that never changes.

Also, filtering an array based on a unique id is a very common way of doing this. You will see it done like this everywhere in other applications.