NW6 | Rabia Avci | JS2 Module | [TECH ED] Build todo-list app | Week 4#222
NW6 | Rabia Avci | JS2 Module | [TECH ED] Build todo-list app | Week 4#222RbAvci wants to merge 4 commits intoCodeYourFuture:mainfrom
Conversation
✅ Deploy Preview for cute-gaufre-e4b4e5 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
|
||
| function addItem(task) { | ||
| if (!task){ | ||
| alert("To-do is required!"); |
Ara225
left a comment
There was a problem hiding this comment.
I think this looks great and works really well. Well done!
| </div> | ||
| </form> | ||
| </section> | ||
| </container> |
There was a problem hiding this comment.
I love your use of semantic HTML and the consistant naming conventions 👍
|
|
||
| function addItem(task) { | ||
| if (!task){ | ||
| alert("To-do is required!"); |
| const classList = item.classList; | ||
| classList.contains("done") | ||
| ? classList.remove("done") | ||
| : classList.add("done"); |
There was a problem hiding this comment.
Could use .toggle() here if you wanted next time - it's a helpful function to save writing extra code https://stackoverflow.com/questions/18880890/how-do-i-toggle-an-elements-class-in-pure-javascript
| const doneItems = list.querySelectorAll(".done"); | ||
| for (const item of doneItems) { | ||
| list.removeChild(item); | ||
| } |
There was a problem hiding this comment.
All in all, some solid functional code here. It could probaly benefit from a refactor and some comments but we're not exspecting perfection here - it works that's the main thing. Good work!
| } | ||
|
|
||
| body { | ||
| background-image: url(assets/pattern_squares-2_1_4_0-0_180_2__ee6d4d_293241_e0fbfc_3d5a80_98c1d9.png); |
| flex-direction: column; | ||
| justify-content: center; | ||
| align-items: center; | ||
| } |
| <button | ||
| type="button" | ||
| id="remove-all-completed" | ||
| class="btn btn-primary mb-3" |
There was a problem hiding this comment.
Doesn't look like you're using these class names in the CSS. They look like classes from Bootstrap CSS. When copy pasting always make sure you tidy up the code so that you can be certain to understand why it works :)
| color: white; | ||
| padding: 10px 25px; | ||
| text-align: center; | ||
| } |
There was a problem hiding this comment.
Some really nice styles, consistant theming etc, no issues I can see except that the delete and completed buttons might be a bit nicer with a bit of spacing
| <title>Todo List</title> | ||
| <link | ||
| rel="stylesheet" | ||
| href="https://maxcdn.bootstrapcdn.com/font-awesome/4.7.0/css/font-awesome.min.css" |
There was a problem hiding this comment.
I like how you've included font awesome for the icons - icons are always a nice touch!
Learners, PR Template
Self checklist
Changelist
Completed a to-do list app populating a "to-do list" element with tasks and associated completion buttons. Have functions to add new tasks, mark tasks as completed, and delete completed tasks.
Questions
Ask any questions you have for your reviewer.