Question 301710 in Html Question posted in
Some basic documentation can be found here.

Html – Javascript to do list can't uncheck or delete tasks (except the last one)

I’ve decided to learn Javascript and after watching a few tutorials, I’m training by doing challenges. One of the iconic challenge is doing a to do list. I know that there are examples with answers but I would like to know why my code doesn’t work…

The issue seems to be that I can only delete the last task but not the others. And I think that for the same reason when I check any other boxes than the last one, the text decoration doesn’t appear (line in the middle).

PS: if you have any commentary on my solution don’t hesitate, I’m eager to learn.

Thank you in advance

const newTaskInput = document.getElementById("newTask");
const tasks = document.querySelector(".tasks");
let tasksCheckboxes = tasks.querySelectorAll("input[type='checkbox']");
let taskCount = 0;

//To add the new task in the content section
newTaskInput.addEventListener("keypress", (e) => {
  if (e.key == "Enter" && newTaskInput.value != "" && newTaskInput.value != undefined) {
    tasks.innerHTML += `
        <li id="${taskCount}-section">
            <input type="checkbox"  id="${taskCount}chk">
            <label class="label-for-${taskCount}" for="${taskCount}chk">${newTaskInput.value}</label>
            <input type="button" id="${taskCount}btn" value="x">
        </li>`;
    addListenerToCheckbox(taskCount);
    addListenerToButton(taskCount);
    newTaskInput.value = "";
    ++taskCount;

  }
})

const addListenerToCheckbox = (aTaskCount) => {
  const checkbox = document.getElementById(aTaskCount + "chk");
  const checkLabel = document.querySelector(`.label-for-${aTaskCount}`);

  checkbox.addEventListener("click", () => {
    if (checkbox.checked) {
      checkLabel.style.textDecoration = "line-through";
    } else {
      checkLabel.style.textDecoration = "none";
    }
  })
}

const addListenerToButton = (aTaskCount) => {
  console.log(taskCount);
  const taskBtn = document.getElementById(aTaskCount + "btn");
  const task = document.getElementById(aTaskCount + "-section");

  taskBtn.addEventListener("click", () => {
    console.log("gg");
    task.remove();
  })
}
<h1>To Do List:</h1>

<div class="box">
  <input type="text" id="newTask">
  <div class="content">
    <ul class="tasks"></ul>
  </div>
</div>
Tags:

2

Answers


  1. 0

    When you modify the innerHTML of an element, you lose all of the event handlers that were previously associated with it. So when you write

    tasks.innerHTML+=`
        <li id="${taskCount}-section">
            <input type="checkbox"  id="${taskCount}chk">
            <label class="label-for-${taskCount}" for="${taskCount}chk">${newTaskInput.value}</label>
            <input type="button" id="${taskCount}btn" value="x">
        </li>`;

    You are effectively doing

    tasks.innerHTML = tasks.innerHTML + `
        <li id="${taskCount}-section">
            <input type="checkbox"  id="${taskCount}chk">
            <label class="label-for-${taskCount}" for="${taskCount}chk">${newTaskInput.value}</label>
            <input type="button" id="${taskCount}btn" value="x">
        </li>`;

    But the event handlers are not stored in tasks innerHTML.

    The reason why your delete button only works on the last element is because you add the handler after modifying the HTML of the element.

    You could avoid this by avoiding direct HTML manipulation of the container and using appendChild in conjunction with createElement.

    Your revised code would look like this:

    const task = document.createElement("li");
// Set the ID using DOM API
task.id = `${taskCount}-section`;
// Keep your previous content
task.innerHTML = `<input type="checkbox"  id="${taskCount}chk">
<label class="label-for-${taskCount}" for="${taskCount}chk">${newTaskInput.value}</label>
<input type="button" id="${taskCount}btn" value="x">`;
// This is key to fixing the issue you face here: We're not directly modifying the containers innerHTML, so all of our tasks will maintain their click handlers
tasks.appendChild(task);

    In terms of other commentary your HTML file has a rogue </div> which is not valid HTML.

    See also: addEventListener to innerHTML

    Login or Signup to reply.
  2. 0

    The issue is because within the addListenerToX() methods you’re re-assigining the value of checkbox and taskBtn to the last instance of those elements. Those variables are then used within the event handlers at a later point, so any pre-existing handlers now point to the last created elements.

    You can fix this, and improve your code quality, by using a single delegated event handler for each element. This will enable you to also remove the dynamically generated id/class attributes, which are an anti-pattern.

    You can also use a template element to hold the HTML instead of putting it in to the JS, which should be kept separate as much as possible.

    Lastly, if you wrap the checkbox in the label it removes the need for the dynamic for attribute.

    Here’s an updated working example containing the above improvements:

    const taskInput = document.querySelector('#new-task');
const taskList = document.querySelector('#tasks');
const liTemplate = document.querySelector('#li-template');

const appendNewTask = task => {
  const liClone = liTemplate.content.cloneNode(true).firstElementChild;
  liClone.querySelector('label span').textContent = task;
  taskList.appendChild(liClone);
} 

const checkboxHandler = e => {
  const cb =e.target;
  cb.nextElementSibling.style.textDecoration = cb.checked ? 'line-through' : 'none';
}

const buttonHandler = e => {
  const button = e.target;
  button.closest('li').remove();
}

document.querySelector('form').addEventListener('submit', e => {
  e.preventDefault();
  appendNewTask(taskInput.value);
  taskInput.value = '';
});

taskList.addEventListener('change', e => e.target.tagName === 'INPUT' && checkboxHandler(e));
taskList.addEventListener('click', e => e.target.tagName === 'BUTTON' && buttonHandler(e));
    <h1>To Do List:</h1>

<form>
  <div class="box">
    <input type="text" id="new-task">
    <div class="content">
      <ul id="tasks"></ul>
    </div>
  </div>
</form>

<template id="li-template">
  <li>
    <label>
      <input type="checkbox" />
      <span></span>
    </label>
    <button type="button" class="delete">X</button>
  </li>
</template>
    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search