-
Notifications
You must be signed in to change notification settings - Fork 2
Aliaksandr_Zakharevich #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Conversation
|
ок, разберем про события на занятии |
| background-color: #f9f9f9; | ||
| width: 100%; | ||
| color: black; | ||
| } |
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.
add empty line plaese between styles
|
|
||
| const pushTodoList = () =>{ | ||
| const main = document.querySelector('.main'); | ||
| if(todoArray.length > 0) { |
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.
const main = document.querySelector('.main');|
// empty line
if/* space */ (todoArray.length > 0) {
| </div> | ||
| </div> | ||
| `; | ||
| main.append(div); |
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.
Changing dom in for is a bab idea. The most freezeing operation in js at all.
Do all changes in one tag. Add everything. Ad the apped result in DOM
| const label = document.querySelector(`#labelTodo${i}`); | ||
| const data = document.querySelector(`#dateTodo${i}`); | ||
| const checkBtn = document.querySelector(`#btnCompleteTodo${i}`); | ||
| label.textContent = todoArray[i].todoTheme; |
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.
div.querySelector()
| } | ||
|
|
||
| const addTodo = () =>{ | ||
| let todoName = document.querySelector('#inputEnter'); |
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.
Better to up thi variable and store in func globaly after append main layout
document.querySelector() also not so fast. Beteer to move it iutside this func
| let todoName = document.querySelector('#inputEnter'); | ||
| if(todoName.value){ | ||
| const data = new Date().toLocaleString(); | ||
| todoArray.push({todoTheme: `${todoName.value}`, data: data, isCompleted: false}) |
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.
In english better to name it as "topic" not a "theme". Theme is more about appearance
| const deleteAllTodo = () =>{ | ||
| const allTodo = todoArray.length; | ||
| for (let i = 0; i<allTodo; i++) { | ||
| deleteLastTodo(); |
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 call func before declaration. Move please deleteLastTodo aboce this func.
- Why to hust set empty array and reset all data? It's musch faster
Avoid multiply document change. Always try to make one change
| const lastTodo = document.querySelector(`.todo${num}`); | ||
| lastTodo.remove(); | ||
| if(todoArray[num].isCompleted){ | ||
| isCompleted(num); | ||
| } | ||
| todoArray.splice(num, 1); | ||
| countTodo(); |
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.
Looks ok
| const checkBtn = e.target; | ||
| const todo = document.querySelector(`.todo${checkBtn.id[checkBtn.id.length-1]}`) |
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.
Hard to understand(
| const countTodo = () =>{ | ||
| const labelAllTodo = document.querySelector('#labelAll'); | ||
| labelAllTodo.textContent = 'All: ' + todoArray.length; | ||
| } | ||
| const countCompleteTodo = () =>{ | ||
| const completedTodo = document.querySelector('#labelCompleted'); | ||
| completedTodo.textContent = 'Completed: ' + countCompetedTodo; | ||
| } | ||
|
|
||
| const isCompleted = (index)=>{ | ||
| const elem = todoArray[index]; | ||
| elem.isCompleted ? countCompetedTodo-- : countCompetedTodo++; | ||
| elem.isCompleted = !elem.isCompleted; | ||
| countCompleteTodo() | ||
| } |
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.
Generaly ok
Попробовал реализовать функицонал кнопок. Почти все получилось, но не знаю как правильно сделать работу кнопки удаления на конкретном todo и отображения всех/выполненных todo