solved task #1486
solved task #1486
Conversation
| setLoadingTodosCount, | ||
| setTodos, | ||
| setInputValue, | ||
| } = useTodos(setErrorMessage); |
There was a problem hiding this comment.
In my opinion, it's rather confusing. Part of the logic associated with todo is in the custom hook, and the other part is spread across components. It is better to either put everything in the hook or not use it at all.
src/App.tsx
Outdated
|
|
||
| Promise.all( | ||
| todosForDeleting.map(id => | ||
| deleteTodos(id) |
There was a problem hiding this comment.
I'd rather think about how to avoid repeating the code. It uses the same logic as deleting a single element, so maybe you should think about how to reuse the already existing code?
This applies to deletion as well as any updates.
| onSubmit, | ||
| onInputChange, | ||
| }) => { | ||
| const isLoading = loadingTodos.length > 0; |
There was a problem hiding this comment.
| const isLoading = loadingTodos.length > 0; | |
| !!loadingTodos.length |
src/components/TodoComponent.tsx
Outdated
| onTodoDelete: (todoId: number) => void; | ||
| onTodoToggle: (todoId: number, currentCompletedStatus: boolean) => void; | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| onUpdateTodo: any; |
There was a problem hiding this comment.
| onUpdateTodo: any; | |
| onUpdateTodo: Promise<Todo>; |
Wouldn't that be better? Or Promise<unknown> at least
| setLoadingTodosCount, | ||
| setTodos, | ||
| setInputValue, | ||
| } = useTodos(setErrorMessage); |
src/components/Footer.tsx
Outdated
| return ( | ||
| <footer className="todoapp__footer" data-cy="Footer"> | ||
| <span className="todo-count" data-cy="TodosCounter"> | ||
| {todos.length - completedTodos.length} items left |
There was a problem hiding this comment.
Create a variable for this calculation
src/components/TodoComponent.tsx
Outdated
| if (isEditing && inputRef.current) { | ||
| inputRef.current.focus(); | ||
| } | ||
| }, [isEditing, inputRef]); //? |
src/utils/updateTodo.ts
Outdated
| ? operation(id, { completed: newStatus }) | ||
| : deleteOperation | ||
| ? deleteOperation(id) | ||
| : operation(id); |
There was a problem hiding this comment.
The nested ternary operator can be hard to read. Consider refactoring that section for clarity.
src/App.tsx
Outdated
| useEffect(() => { | ||
| if (errorMessage) { | ||
| const timer = setTimeout(() => { | ||
| setErrorMessage(ErrorMessages.NO_ERRORS); | ||
| }, 3000); | ||
|
|
||
| return () => clearTimeout(timer); | ||
| } | ||
| }, [errorMessage]); |
|
It's strange, that you have such behaviour, because from my side it works perfectly.🤨 |
DEMO LINK