completed the task#2195
Conversation
Denys-Kravchuk9988
left a comment
Denys-Kravchuk9988
left a comment
There was a problem hiding this comment.
Good job!
A few things to improve:
- By clicking on select all caret button all tasks should display loading icon (loading state)
| className="todoapp__clear-completed" | ||
| data-cy="ClearCompletedButton" | ||
| disabled={!todosFromServer?.some(todo => todo.completed === true)} | ||
| onClick={() => { |
There was a problem hiding this comment.
I would recommend to shorten here expression onClick={clearCompleted}
| htmlFor={`checkbox-${tempTodo.id}`} | ||
| className="todo__status-label" | ||
| > | ||
| {''} |
| type="button" | ||
| className="todo__remove" | ||
| data-cy="TodoDelete" | ||
| onClick={() => { |
There was a problem hiding this comment.
It's better to shorten onClick={() => deleteTodo(todo.id, todo)}
| htmlFor={`checkbox-${todo.id}`} | ||
| className="todo__status-label" | ||
| > | ||
| {''} |
| filtrationTodos(selectedFilter); | ||
| }, [todosFromServer]); | ||
|
|
||
| const clearCompleted = useCallback(() => { |
There was a problem hiding this comment.
It's better to add async/await here
const clearCompleted = useCallback(async () => {
const todosToDelete = todosFromServer
.filter(todo => todo.completed)
.map(todo => deleteTodo(todo.id, todo));
await Promise.all(todosToDelete);
}, [todosFromServer, deleteTodo]);
etojeDenys
left a comment
etojeDenys
left a comment
There was a problem hiding this comment.
loader is not working properly at the moment
-
after trying to change the status of several todos in a short period of time, the loader only appears on the last pressed todo.
-
after pressing the 'toggle all' button, the loader should appear on every todo that will change its status. instead, it appears on every todo
loader should not appear in the first two items in this situation
Anton-Kuchmasov
left a comment
Anton-Kuchmasov
left a comment
There was a problem hiding this comment.
Great job you did!
Check out comments below - they might be helpful
| setTodosFormServer(serverTodos); | ||
| }) | ||
| .catch(() => { | ||
| setError('Unable to load todos'); |
There was a problem hiding this comment.
Better approach is creating custom type or enum (f.e., ErrorMessage.ts) and re-use it to avoid possible hard-coded mistakes
| setTimeout(() => { | ||
| setError(''); | ||
| }, 3000); |
There was a problem hiding this comment.
This function can be splitted in the single helper and could be re-used
| /> | ||
| )} | ||
|
|
||
| {todosFromServer && todosFromServer.length > 0 && ( |
There was a problem hiding this comment.
| {todosFromServer && todosFromServer.length > 0 && ( | |
| {todosFromServer && !!todosFromServer.length && ( |
4 participants
DEMO LINK