-
-
Notifications
You must be signed in to change notification settings - Fork 41
NW6 | Sabella Fisseha | JS2 Module-To Do-List App | WEEK 4 #217
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. |
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.
Great work so far, some improvements are needed
<ul id="todo-list"></ul> | ||
<form> | ||
<div> | ||
<input type="text" placeholder="New todo..." id="textboxinput" /> |
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.
you should have the deadline date right here, it should be set after the user enter the todo name
|
||
|
||
add.addEventListener("click", () => { | ||
clickCount++; |
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.
instead of using the number of counts to determine the status of the todo. you can make use of the boolean
todo completed
status on each todo which you can toggle. That is s cleaner way to do it.
function deleteAllCompletedTodos() { | ||
// Write your code here... | ||
const del = document.getElementById("remove-all-completed"); |
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.
this function isnt implemented correctly, when clicked, it removes all todos on the board. You should rewrite this function to filter out todos that has their completed
status as true
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.