Homework 1/yulia #18
Homework 1/yulia #18
Conversation
src/ex1/script.js
Outdated
| document.querySelector('ul').addEventListener('click', deleteHandler) | ||
| document.getElementById('clear-all').addEventListener('click', clearAllHandler) | ||
| document.querySelector('ul').addEventListener('click', alertMessage) | ||
| /*document.getElementById('new-task').addEventListener('keydown', preventSpace)*/ |
There was a problem hiding this comment.
please avoid leaving those comments in the code :)
src/ex1/script.js
Outdated
| event.preventDefault() //Preventing page refresh on form submission | ||
| const newTaskInput = document.querySelector('#new-task') | ||
| listGif.style.display= "none" | ||
| if (newTaskInput.value != ''){ |
There was a problem hiding this comment.
in general its a better practice in js to use instead of == (on in this case !== instead of !=)
this way you check comparison of both the value and the type and not just the value
src/ex1/script.js
Outdated
| } | ||
|
|
||
| //Prevents "space" click on keyboard | ||
| /*function preventSpace(event){ |
There was a problem hiding this comment.
same here - you should keep the code clean when its ready for review without those comments
also - let me know if you need help with the what you wanted to do here :)
src/ex1/script.js
Outdated
| doneTasksCounter = 0 | ||
| } | ||
| item.classList.add('task-delete-animation') | ||
| tasksCompleted.innerText = `You have completed ${doneTasksCounter} / ${tasksCounter} tasks` |
There was a problem hiding this comment.
there are multiple places where you create this text, consider extracting to function. this way you have 1 source of truth, and if you will need to change the text - you will need to do this just in 1 place and not everywhere
src/ex1/script.js
Outdated
| tasksCounter = 0 | ||
| doneTasksCounter = 0 |
There was a problem hiding this comment.
this part should not be inside the loop, it can happen just once when calling this function
| </form> | ||
| <div class="tasks-container"> | ||
| <ul></ul> | ||
| <img id="list-gif" src="list_gif.gif" alt="list" style="display: flex;" height="200px" width="200px"> |
There was a problem hiding this comment.
its a better practice to add class and then add the style in css. try to avoid using style attribute explicitly :)
| function checkTodo(event){ | ||
| let item = event.target.parentNode | ||
| //When clicking on the check button after it was already checked as done, the line through style will be removed | ||
| if(item.style.textDecoration === 'line-through'){ |
There was a problem hiding this comment.
consider using early return to make the code more clear
if (something) {
....
return;
}
// else logic in here
src/ex1/script.js
Outdated
| @@ -1,0 +1,125 @@ | |||
| document.querySelector('form').addEventListener('submit', submitButtonHandler) | |||
There was a problem hiding this comment.
you are missing ; everywhere. dont forget to add it at the end of any new line in js :)
|
|
||
| /*** Task Done Checkbox */ | ||
| function checkHandler(event){ | ||
| if(event.target.name === 'task-complete'){ |
There was a problem hiding this comment.
this event is triggered in every ul selector but relevant only for task-compelte.
consider adding it specifically for the relevant selector when creating it
No description provided.