Develop #1579
Develop #1579
Conversation
There was a problem hiding this comment.
Good job 👍
Let's improve your code
-
It is bad practice to write all logic in one component, you need to separate the logic into different components, for example
Header, Footer, TodoItem, TodoList -
The edit mode looks strange, you need to fix styles
- Empty todo should be deleted
src/App.tsx
Outdated
| setChangeTodoId(todoItem.id); | ||
| }; | ||
|
|
||
| const ClearCompleted = () => { |
There was a problem hiding this comment.
| const ClearCompleted = () => { | |
| const clearCompleted = () => { |
There was a problem hiding this comment.
The edit mode looks strange, you need to fix styles - не знаю як це зробити, підскажіть будь-ласка, витратив купу часу і так і не зрозумів, чому стилі не спрацьовують
src/App.tsx
Outdated
| <header className="todoapp__header"> | ||
| <button | ||
| type="button" | ||
| // className="todoapp__toggle-all active" |
There was a problem hiding this comment.
Remove all comments
| // className="todoapp__toggle-all active" |
src/App.tsx
Outdated
| <button | ||
| type="button" | ||
| // className="todoapp__toggle-all active" | ||
| className={`todoapp__toggle-all ${allCompleted ? 'active' : ''}`} |
There was a problem hiding this comment.
Use the classnames library for add classes with condition, fix it everywhere
src/App.tsx
Outdated
| .then(todosList => { | ||
| setTodos(todosList); | ||
| }) |
There was a problem hiding this comment.
| .then(todosList => { | |
| setTodos(todosList); | |
| }) | |
| .then(setTodos) |
src/componens/ErrorMessage.tsx
Outdated
| return ( | ||
| <div | ||
| data-cy="ErrorNotification" | ||
| className={`notification is-danger is-light has-text-weight-normal ${errorMessage ? '' : 'hidden'}`} |
There was a problem hiding this comment.
Use the classnames library for add classes with condition, fix it everywhere
src/componens/Footer.tsx
Outdated
| return ( | ||
| <footer className="todoapp__footer" data-cy="Footer"> | ||
| <span className="todo-count" data-cy="TodosCounter"> | ||
| {todos.filter(tod => !tod.completed).length} items left |
There was a problem hiding this comment.
Move this logic from jsx to the helper variable and use it here
src/componens/Footer.tsx
Outdated
| <a | ||
| href="#/" | ||
| className={classNames('filter__link', { | ||
| selected: filter === FilterType.All, | ||
| })} | ||
| data-cy="FilterLinkAll" | ||
| onClick={() => setFilter(FilterType.All)} | ||
| > | ||
| All | ||
| </a> | ||
|
|
||
| <a | ||
| href="#/active" | ||
| className={classNames('filter__link', { | ||
| selected: filter === FilterType.Active, | ||
| })} | ||
| data-cy="FilterLinkActive" | ||
| onClick={() => setFilter(FilterType.Active)} | ||
| > | ||
| Active | ||
| </a> | ||
|
|
||
| <a | ||
| href="#/completed" | ||
| className={classNames('filter__link', { | ||
| selected: filter === FilterType.Completed, | ||
| })} | ||
| data-cy="FilterLinkCompleted" | ||
| onClick={() => setFilter(FilterType.Completed)} |
There was a problem hiding this comment.
Use Object.values(your created enum) and render these options with map() method
src/componens/Header.tsx
Outdated
| }) => { | ||
| const inputRef = useRef<HTMLInputElement | null>(null); | ||
| const allCompleted = todos.every(tod => tod.completed); | ||
| const ToggleAllButton = () => { |
There was a problem hiding this comment.
| const ToggleAllButton = () => { | |
| const toggleAllButton = () => { |
src/componens/TodoItem.tsx
Outdated
| {isLoadingIds.includes(todoItem.id) && ( | ||
| <div data-cy="TodoLoader" className="modal overlay is-active"> | ||
| <div className="modal-background has-background-white-ter" /> | ||
| <div className="loader" /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
| {isLoadingIds.includes(todoItem.id) && ( | |
| <div data-cy="TodoLoader" className="modal overlay is-active"> | |
| <div className="modal-background has-background-white-ter" /> | |
| <div className="loader" /> | |
| </div> | |
| )} | |
| <div | |
| data-cy="TodoLoader" | |
| className={cn('modal overlay', { 'is-active': isLoadingIds.includes(todoItem.id)})} | |
| > | |
| <div className="modal-background has-background-white-ter" /> | |
| <div className="loader" /> | |
| </div> |
src/componens/Footer.tsx
Outdated
|
|
||
|
|
||
|
|
||
|
|
| const [todo, setTodo] = useState<string>(''); | ||
| const [errorMessage, setErrorMessage] = useState<string>(''); |
There was a problem hiding this comment.
it's redundant to specify types for primitives
DEMO LINK