add task P1#2403
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:
| export const App: React.FC = () => { | ||
| const [todos, setTodos] = React.useState<Todo[]>([]); | ||
| const [errorMessage, setErrorMessage] = React.useState(''); | ||
| const [filter, setFilter] = React.useState('all'); |
There was a problem hiding this comment.
I would recommend to extract 'all' into a separate enum and use it here
| const activeTodos = todos.filter(todo => !todo.completed); | ||
| const visibleTodos = todos.filter(todo => { | ||
| switch (filter) { | ||
| case 'active': |
There was a problem hiding this comment.
I would recommend to extract 'active' into a separate enum and use it here
| case 'active': | ||
| return !todo.completed; | ||
|
|
||
| case 'completed': |
There was a problem hiding this comment.
I would recommend to extract 'completed' into a separate enum and use it here
| <div | ||
| key={todo.id} | ||
| data-cy="Todo" | ||
| className={todo.completed ? 'todo completed' : 'todo'} |
There was a problem hiding this comment.
It's better to use classNames here
| <a | ||
| href="#/" | ||
| data-cy="FilterLinkAll" | ||
| className={ |
There was a problem hiding this comment.
It's better to use classNames here
| className={ | ||
| filter === 'active' ? 'filter__link selected' : 'filter__link' | ||
| } | ||
| onClick={() => setFilter('active')} |
There was a problem hiding this comment.
I would recommend to extract 'active' into a separate enum and use it here
| className={ | ||
| filter === 'all' ? 'filter__link selected' : 'filter__link' | ||
| } | ||
| onClick={() => setFilter('all')} |
There was a problem hiding this comment.
I would recommend to extract 'all' into a separate enum and use it here
| ? 'filter__link selected' | ||
| : 'filter__link' | ||
| } | ||
| onClick={() => setFilter('completed')} |
There was a problem hiding this comment.
I would recommend to extract 'completed' into a separate enum and use it here
| <a | ||
| href="#/completed" | ||
| data-cy="FilterLinkCompleted" | ||
| className={ |
There was a problem hiding this comment.
I would recommend to use classNames here
| <div | ||
| data-cy="ErrorNotification" | ||
| className="notification is-danger is-light has-text-weight-normal" | ||
| className={`notification is-danger is-light has-text-weight-normal ${ |
There was a problem hiding this comment.
I would recommend to use classNames here
Denys-Kravchuk9988
left a comment
Denys-Kravchuk9988
left a comment
There was a problem hiding this comment.
Good job!
Try to pass all tests.
I think it can be caused because this caret is selected by default
|
Can I copy the all SRC folder into next part? |
Anton-Kuchmasov
left a comment
Anton-Kuchmasov
left a comment
There was a problem hiding this comment.
Well done!
Can I copy the all SRC folder into next part?
not a whole src/ folder, but several components - in the next chap there should be some new functional, so be careful
3 participants
https://ArtemNosachenko.github.io/react_todo-app-loading-todos/