Develop #1489
Develop #1489
Conversation
src/hooks/useTodos.ts
Outdated
| const handleErrorMessage = (error: ErrorMessages) => { | ||
| setErrorMessage(error); | ||
| setTimeout(handleErrorReset, 3000); | ||
| }; |
src/hooks/useTodos.ts
Outdated
| updateTodo(id, updatedTodo) | ||
| .then(todo => { | ||
| setTodos(currentTodos => | ||
| currentTodos.map(t => (t.id === id ? { ...t, ...todo } : t)), |
There was a problem hiding this comment.
[CHECKLIST] A variable name should describe what is stored in it (it's about your variable t)
src/hooks/useTodos.ts
Outdated
| setIsReceivingAnswer(true); | ||
| setLoadingTodoIds(currentIds => [ | ||
| ...currentIds, | ||
| ...completedTodos.map(todo => todo.id), | ||
| ]); |
There was a problem hiding this comment.
your function handleTodoDelete already has this logic
src/hooks/useTodos.ts
Outdated
| setIsReceivingAnswer(true); | ||
| setLoadingTodoIds(currentIds => [ | ||
| ...currentIds, | ||
| ...todosToUpdate.map(todo => todo.id), | ||
| ]); |
There was a problem hiding this comment.
your function
handleTodoDeletealready has this logic
same, but this time handleTodoUpdate
src/components/Header/Header.tsx
Outdated
| {todos.length > 0 && <button | ||
| type="button" | ||
| className={cn('todoapp__toggle-all', { | ||
| active: areAllCompleted, | ||
| })} | ||
| data-cy="ToggleAllButton" | ||
| onClick={onToggleAll} | ||
| />} |
There was a problem hiding this comment.
wrap <button /> in brackets for better readability
src/hooks/useTodos.ts
Outdated
| useEffect(() => { | ||
| getTodos() | ||
| .then(setTodos) | ||
| .catch(() => handleErrorMessage(ErrorMessages.UNABLE_TO_LOAD_TODO)); | ||
| }, []); |
There was a problem hiding this comment.
Move the useEffect to the end of code
Placing useEffect at the end improves readability by letting developers first grasp the main logic before seeing side effects. This promotes clearer structure and consistency in the codebase.
src/hooks/useTodos.ts
Outdated
| useEffect(() => { | ||
| getTodos() | ||
| .then(setTodos) | ||
| .catch(() => handleErrorMessage(ErrorMessages.UNABLE_TO_LOAD_TODO)); | ||
| }, []); |
There was a problem hiding this comment.
It's better to move the logic inside the useEffect hook to a separate function, as it improves code readability, reusability, and makes testing easier. It also helps keep useEffect focused on side effects.
DEMO LINK