Skip to content
This repository was archived by the owner on Dec 18, 2024. It is now read-only.

NW6| Bakhat Begum| MODULE-JS2| Js2 todo list/week 3 | Week-4 #218

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

BakhatBegum
Copy link

@BakhatBegum BakhatBegum commented Jan 23, 2024

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

This is a super handy, super simple to-do list.

You will be given a list of tasks which are "To Do". We call these tasks "ToDos"

Each item in the list should have 2 buttons:

  • One to click when the ToDo has been completed - it will apply a line-through style to the text of the ToDo.
  • A second to delete the ToDo. This could be used to delete completed ToDos from the list or remove ToDos that we are no longer interested in doing.

Questions

I have used my script link at the bottom of the HTML file. Is that correct or do I remove it from there?

Copy link

netlify bot commented Jan 23, 2024

Deploy Preview for cute-gaufre-e4b4e5 ready!

Name Link
🔨 Latest commit afbbe73
🔍 Latest deploy log https://app.netlify.com/sites/cute-gaufre-e4b4e5/deploys/65afd2dd4d3431000828e2c8
😎 Deploy Preview https://deploy-preview-218--cute-gaufre-e4b4e5.netlify.app/week-3/todo-list
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@musayuksel musayuksel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done @BakhatBegum

Comment on lines +8 to +12
let list = document.getElementById("todo-list");
let listValue = list.value;


let li = document.createElement("li")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done @BakhatBegum it is very readable code.
You can improve your code by using const instead of let for variables that don't need to be reassigned. variables like list, listValue, li etc

let container = document.createElement("span")
container.classList.add("layout");
list.appendChild(container);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly recommend using code prettier to make sure your code has proper indentations. It'll make your code look clean and organised. Give it a try ;)

Comment on lines +51 to 57
function deleteAllCompletedTodos(event) {
let todoList = document.getElementById("task-list");
let itemsTocompleted = document.querySelectorAll(".completed");
itemsTocompleted.forEach(function (item) {
todoList.removeChild(item);
});
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look at your code, you had created the deleteAllCompletedTodos function but didn't use it anywhere. You need to call it when the corresponding button-Remove all completed is clicked.
Also, if you look at your preview when you click the "Remove all completed" button, it deletes everything, even uncompleted tasks.
But why?
This happens because the button has a type of "submit", which tries to submit the form and refresh the page.
To prevent this behaviour, you can add an event listener to the button and call the deleteAllCompletedTodos function from there!!!
example:

const removeButton = document.getElementById("remove-button");//Assuming the given Id exists.
removeButton.addEventListener("click", deleteAllCompletedTodos);

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants