CODEWITHSUNDEEP
javascript
David
David

Reputation: 67

can't delete custom todo list item from todo list

I made a todo list with some predefined todo list items and I also made it possible to set your own todos inside the list through the prompt command. My problem with this code is, when I try to delete the items that I created, I can't do it. Though I can delete the predefined items. Here's the code:

let addTodo = document.getElementById('add');
let delTodo = document.getElementById('delete');
let ul = document.querySelector('ul');
let li = document.querySelectorAll('li');

addTodo.addEventListener('click', () => {
  let add = prompt('Add a new todo');

  if (add.length >= 1) {
    let newLi = document.createElement('li');
    let newLiText = document.createTextNode = add;
    newLi.append(newLiText);
    ul.appendChild(newLi);
  } else {
    alert('An error has occurred. Please try again.');
  }
});

delTodo.addEventListener('click', () => {
  let deleteTodo = prompt('Which todo do you want to delete?');
  let firstTodo = document.querySelector('#first');
  let secondTodo = document.querySelector('#second');
  let thirdTodo = document.querySelector('#third');

  if (deleteTodo === 'Study') {
    ul.removeChild(firstTodo);
  } else if (deleteTodo === 'Eat') {
    ul.removeChild(secondTodo);
  } else if (deleteTodo === 'Watch') {
    thirdTodo.style.display = 'none';
  } else {
    alert('error occurrred');
  }
});
body {
  margin: 0;
  padding: 0;
  background-color: coral;
  font-family: 'Playfair Display', cursive;
  letter-spacing: 0.1em;
  color: firebrick;
}

.wrapper {
  margin: 10px auto;
  border: 3px solid firebrick;
  max-width: 300px;
  text-align: center;
}

.table {
  margin: 20px auto;
  padding: 10px;
}

.table ul {
  list-style-type: none;
}

.table li {
  text-align: left;
  margin: 20px 0px 20px -40px;
  border: 1px solid red;
  padding: 10px;
}

.table h3 {
  text-align: left;
}

button {
  margin: 10px 10px;
  padding: 5px;
  font-family: 'Playfair Display';
  background-color: coral;
  color: firebrick;
  border: 2px solid firebrick;
}
<!DOCTYPE html>
<html>

<head>
  <link href="https://fonts.googleapis.com/css?family=Playfair+Display&display=swap" rel="stylesheet">
</head>

<body>
  <div class="wrapper">
    <h1>Todo List</h1>
    <div class="table">
      <h3>List the todos here</h3>
      <ul>
        <li id="first">Study Math</li>
        <li id="second">Eat Breakfast</li>
        <li id="third">Watch a Movie</li>
      </ul>
    </div>
    <button id="add">Add new todo</button>
    <button id="delete">Delete todo</button>
  </div>
  <script src="sandbox.js"></script>
</body>

</html>

Upvotes: 2

Views: 322

Answers (3)

Akshay Bande
Akshay Bande

Reputation: 2587

To delete the todo you have to first identify it. For that add id attribute to the li element.

The below solution can fail if two todos have the same name because your IDs must be unique.

For that, you have to find a way to uniquely identify an element(li) while creating it dynamically.

let addTodo = document.getElementById('add');
let delTodo = document.getElementById('delete');
let ul = document.querySelector('ul');
let li = document.querySelectorAll('li');

addTodo.addEventListener('click', () => {
  let add = prompt('Add a new todo');

  if (add && add.length >= 1) {
    let newLi = document.createElement('li');
    newLi.id = add.trim();
    let newLiText = document.createTextNode(add);
    newLi.append(newLiText);
    ul.appendChild(newLi);
  } else {
    alert('TODO name cannot be empty');
  }
});

delTodo.addEventListener('click', () => {
  let deleteTodo = prompt('Which todo do you want to delete?');

  let toDeleteId = deleteTodo.toLowerCase().trim();
  let toDeleteNode = document.querySelector(`#${toDeleteId}`);
  if (toDeleteNode) {
    ul.removeChild(toDeleteNode)
  } else {
    alert("TODO not found")
  }

});
body {
  margin: 0;
  padding: 0;
  background-color: coral;
  font-family: 'Playfair Display', cursive;
  letter-spacing: 0.1em;
  color: firebrick;
}

.wrapper {
  margin: 10px auto;
  border: 3px solid firebrick;
  max-width: 300px;
  text-align: center;
}

.table {
  margin: 20px auto;
  padding: 10px;
}

.table ul {
  list-style-type: none;
}

.table li {
  text-align: left;
  margin: 20px 0px 20px -40px;
  border: 1px solid red;
  padding: 10px;
}

.table h3 {
  text-align: left;
}

button {
  margin: 10px 10px;
  padding: 5px;
  font-family: 'Playfair Display';
  background-color: coral;
  color: firebrick;
  border: 2px solid firebrick;
}
<!DOCTYPE html>
<html>

<head>
  <link href="https://fonts.googleapis.com/css?family=Playfair+Display&display=swap" rel="stylesheet">
</head>

<body>
  <div class="wrapper">
    <h1>Todo List</h1>
    <div class="table">
      <h3>List the todos here</h3>
      <ul>
        <li id="first">Study Math</li>
        <li id="second">Eat Breakfast</li>
        <li id="third">Watch a Movie</li>
      </ul>
    </div>
    <button id="add">Add new todo</button>
    <button id="delete">Delete todo</button>
  </div>
</body>

</html>

Upvotes: 1

onecompileman
onecompileman

Reputation: 940

You are not doing it dynamically, what I would do is get all li and find a text containing the text user inputted in the prompt, and remove the element afterwards.

let addTodo = document.getElementById('add');
let delTodo = document.getElementById('delete');
let ul = document.querySelector('ul');
let li = document.querySelectorAll('li');

addTodo.addEventListener('click', () => {
  let add = prompt('Add a new todo');

  if (add.length >= 1) {
    let newLi = document.createElement('li');
    let newLiText = document.createTextNode = add;
    newLi.append(newLiText);
    ul.appendChild(newLi);
  } else {
    alert('An error has occurred. Please try again.');
  }
});

delTodo.addEventListener('click', () => {
  let deleteTodo = prompt('Which todo do you want to delete?');
  const listItems = document.querySelectorAll('li');
  let listItemToRemove = null;
  for (let li of listItems) {
     if (li.innerText === deleteTodo) {
        listItemToRemove = li;
     }
  }
  if (listItemToRemove) {
    listItemToRemove.remove();
  }
});
body {
  margin: 0;
  padding: 0;
  background-color: coral;
  font-family: 'Playfair Display', cursive;
  letter-spacing: 0.1em;
  color: firebrick;
}

.wrapper {
  margin: 10px auto;
  border: 3px solid firebrick;
  max-width: 300px;
  text-align: center;
}

.table {
  margin: 20px auto;
  padding: 10px;
}

.table ul {
  list-style-type: none;
}

.table li {
  text-align: left;
  margin: 20px 0px 20px -40px;
  border: 1px solid red;
  padding: 10px;
}

.table h3 {
  text-align: left;
}

button {
  margin: 10px 10px;
  padding: 5px;
  font-family: 'Playfair Display';
  background-color: coral;
  color: firebrick;
  border: 2px solid firebrick;
}
<!DOCTYPE html>
<html>

<head>
  <link href="https://fonts.googleapis.com/css?family=Playfair+Display&display=swap" rel="stylesheet">
</head>

<body>
  <div class="wrapper">
    <h1>Todo List</h1>
    <div class="table">
      <h3>List the todos here</h3>
      <ul>
        <li id="first">Study Math</li>
        <li id="second">Eat Breakfast</li>
        <li id="third">Watch a Movie</li>
      </ul>
    </div>
    <button id="add">Add new todo</button>
    <button id="delete">Delete todo</button>
  </div>
  <script src="sandbox.js"></script>
</body>

</html>

Upvotes: 0

Josh Wulf
Josh Wulf

Reputation: 4877

You haven't done anything wrong, you just haven't written the code to delete any TODOs.

I'm going to describe it for you, but you will need to research it and write the code.

When you add a new TODO, you need to set the id attribute on it.

You probably want to use the user-supplied name of the TODO with the spaces stripped out.

Then in your delete code, you strip the spaces out of the user input and look for an element with that as its id.

Don't do a massive if block. Just look for the element by the id, and if there is one that matches, remove it. If there is not one, do an alert to tell the user it is not found.

For the next level, put "__todo-" in front of the id when you create the TODOs, and show a drop-down box on the page to delete them, or otherwise (better UX), put a button on the TODO to allow the user to delete it.

Upvotes: 1

Related Questions