JS To Do List: how to toggle 'done'?

Taking a crack at a JS todo list and I’m at a stuck point. I want each list item to have an eventListener that toggles a class that will change the style. i.e. click a list item, and it’s crossed out. click it again, and it reverts to it’s original styling.

With the code I have, clicking any of the list items will change the style of the last item added, but no others. On the other hand todoItem object toggles from true to false just fine per each list item. This has really got me stumped. Any ideas?

See snippet below or the full version here.

function addTodoItem(x) {
  //ignore empty strings
  if (newTask.value == "") {
      console.log('Please enter a task');
  } else {
  //create todoItem object
  let todoItem = {
    text: `${newTask.value}`,
    complete: false,
  }
    todoArr.push(todoItem);
    listItem = document.createElement("LI");
    listItem.classList.add("taskToDo");
    //set list item text equal to input field text.
    listItem.innerHTML = newTask.value;

//~~~~ONLY TOGGLES LATEST LIST ITEM
    listItem.addEventListener("click", function completeTask() { 
        if (todoItem.complete == false) {
          todoItem.complete = true;
          listItem.classList.toggle("taskComplete");
          listItem.classList.toggle("taskToDo");
          console.log(todoItem.complete);
        } else {
          todoItem.complete = false;
          listItem.classList.toggle("taskComplete");
          listItem.classList.toggle("taskToDo");
          console.log(todoItem.complete);
        }
      });
    taskList.appendChild(listItem);
  }
  //clear input field
  newTask.value = "";
}

Did you try that without naming the callback function? It should be anonymous.

Anonymous function as in:
listItem.addEventListener("click", function() {...}

I did try the above and got the same results unfortunately.

What you’re essentially trying to do is event delegation:

I’d read through that + event delegation and then refactor your code accordingly. You’ll also probably want to create a ‘key’ or ‘id’ for each element.

I will say, if you’re going through the JS courses, once you hit the React framework this becomes super trivial. Always good to learn the vanilla, though! :slight_smile:

Edit:

I went ahead and whipped up a version of what you’ve done, with some similarities, but I left it different since you’ll want to figure out the best way to do it for you.

Just a light explanation of what event delegation/event bubbling is is that the parent container will listen for events that happen on it’s children (the event bubbles up to the event listener). In this case when I click an LI the UL element sees that and then we appropriately target the child LI since we’re using the EVENT argument, which allows us to change the text color of the clicked LI.

I should also mention that you’ll notice you can click on the screen to the right of the LI item and it will change–something you need to be aware of when using event delegation is to watch for what containers your targeting. In this case the LI being inline element extends the whole width of the page (even though the text doesn’t). I mitigated this by changing the display of the UL element to inline-block.

Also with classList.toggle, I believe you’ll only need one because toggle will essentially add a class if it’s not detected on the element, if the class is there it will remove it–just like a toggle :stuck_out_tongue:. So you can probably just make one class that is like your “completion CSS” and then just toggle that CSS on or off.

1 Like

What a joy to see come up, again. Hopefully this time it gains traction.

1 Like

The logic of delegation exemplifies itself; the footprint is so much smaller since it is one instead of many.

When we register a listener, JS now has to request an interrupt from the OS. It won’t be denied, obviously, but it will add to the load on system performance. The more we add, the greater the load. One listener is better than many listeners, at any length.


It is easy to detect an event, bubbled up or not. It’s a event, that is clear. If it’s the parent object then the rest is mechanics. The event has a window attribute that is updated sixty times a second. When we interrupt this flow we get a snapshot of the Event Object.

That object happens to have a which property, as in, ‘which element has event focus(i.e., target) ?’ It becomes moot that the parent detected the event; we now have focus on that particular event.

The target object gives us a context, and with that we can interact.

2 Likes

Thank you for the time and detail you put into this! Very helpful! I had not heard of event delegation before and only recently heard of bubbling. I’ve taken what you’ve said and was able to get the styles to toggle the way I want. Update code here.

In doing so, the true/false value of the object.complete property now toggles on only the latest list item. A familiar issue :smiley:

Would the solve for this be adding an event listener to each todoItem object? I got the sense that adding an event listener to each list item was not advised…

      <ul id="taskList">
      </ul>

The listener should be attached to that (parent) element. That way when new items are added, or items are removed there is no change to the listener necessary. Events on the list items will bubble up to the parent where they can be detected and trigger the handler.

It is in the handler that the delegation takes place. The event object has a target attribute which points to the node where the event took place.

1 Like

I believe I have attached the listener to the parent (it’s on the last line of code, copied below). Unless I’m missing something…?

document.querySelector("#taskList").addEventListener('click', completeTask);

Looks alright. Is it working as you expect?

In part, yes. The style changes on click as expected, but after that success, another problem was introduced. When I began this project, I made it so that when the user adds an item to the list, an object is created and added to an array. This new object has a property todoItem.complete with a boolean value that signifies whether or not the task is complete. This property only toggles on the latest list item added whenever any of the list items are clicked for some reason. Full code here

The physical list is like an array, so it is not really necessary to maintain a separate one. That just adds to the work. All your handler needs to do is insert a new LI into the DOM with the input data as text content.

Say you have a cached node:

const tasklist = document.querySelector("#taskList");

The LIs are child elements of that node so will be in the node tree (an array like collection).

For simplicity we can use the global event, onclick to register the listener/handler.

tasklist.onclick = completeTask;

Be sure the handler is in the code above the listener.

If you want it easy to identify the tree nodes without using subscripts, give each new element a data attribute (or id) and compute the number from the length of the tasklist then write it to the new LI. That way, you can access the data on the event target,

target.data = Array.from(tasklist).length

Interesting! I will have to take another swing at this after I fully digest your reply (I’m still a JS greenhorn). Thank you for your guidance!

1 Like