-
-
Notifications
You must be signed in to change notification settings - Fork 40
NW6| Bakhat Begum| MODULE-JS2| Js2 todo list/week 3 | Week-4 #218
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cute-gaufre-e4b4e5 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
musayuksel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done @BakhatBegum
| let list = document.getElementById("todo-list"); | ||
| let listValue = list.value; | ||
|
|
||
|
|
||
| let li = document.createElement("li") |
There was a problem hiding this comment.
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); | ||
|
|
There was a problem hiding this comment.
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 ;)
| function deleteAllCompletedTodos(event) { | ||
| let todoList = document.getElementById("task-list"); | ||
| let itemsTocompleted = document.querySelectorAll(".completed"); | ||
| itemsTocompleted.forEach(function (item) { | ||
| todoList.removeChild(item); | ||
| }); | ||
| } |
There was a problem hiding this comment.
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);
Learners, PR Template
Self checklist
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:
Questions
I have used my script link at the bottom of the HTML file. Is that correct or do I remove it from there?