Add task solution #1497
Add task solution #1497
Conversation
There was a problem hiding this comment.
Good Job 👍
let's improve your solution
Watch the attached video and fix the bug
https://www.loom.com/share/351ebf9f477e46f0bbe0d5437efdb54d
src/App.tsx
Outdated
| return <UserWarning />; | ||
| } | ||
| const [todos, setTodos] = useState<Todo[]>([]); | ||
| const [error, setError] = useState<string | null>(null); |
There was a problem hiding this comment.
| const [error, setError] = useState<string | null>(null); | |
| const [error, setError] = useState<ErrorMessages>(ErrorMessages.DEFAULT); |
src/App.tsx
Outdated
| const [isEditingId, setIsEditingId] = useState<number | null>(null); | ||
|
|
||
| const handleAddTodo = useCallback(async (): Promise<void> => { | ||
| if (title.trim() === '') { |
There was a problem hiding this comment.
Create a variable for trimmed title as u use it more than once.
src/App.tsx
Outdated
| } catch { | ||
| setError(ErrorMessages.ADDING_TODOS); | ||
| setTempTodo(null); | ||
| setTimeout(() => setError(null), 3000); |
There was a problem hiding this comment.
Consider clearing timeouts when the component unmounts to avoid potential memory leaks and ensure smooth handling of state updates. This will improve the reliability of your code and prevent errors in case the DOM element is no longer available.
src/App.tsx
Outdated
| const fetchTodos = async () => { | ||
| try { | ||
| const data = await getTodos(); | ||
|
|
||
| setTodos(data); | ||
| } catch (err) { | ||
| setError(ErrorMessages.LOAD_TODOS); | ||
| const timer = setTimeout(() => setError(null), 3000); | ||
|
|
||
| return () => clearTimeout(timer); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Move the fetch Todos function out from useEffect
src/components/Footer.tsx
Outdated
| return ( | ||
| <footer className="todoapp__footer" data-cy="Footer"> | ||
| <span className="todo-count" data-cy="TodosCounter"> | ||
| {`${activeTodos} items left`} |
There was a problem hiding this comment.
| {`${activeTodos} items left`} | |
| {activeTodos} items left |
src/components/ErrorNotification.tsx
Outdated
| onHideError: () => void; | ||
| } | ||
|
|
||
| export const ErrorNotification: React.FC<ErrorNotificationProps> = ({ |
There was a problem hiding this comment.
| export const ErrorNotification: React.FC<ErrorNotificationProps> = ({ | |
| export const ErrorNotification: FC<ErrorNotificationProps> = ({ |
src/components/Footer.tsx
Outdated
| </span> | ||
|
|
||
| <nav className="filter" data-cy="Filter"> | ||
| {Object.values(FilterStates).map(state => ( |
There was a problem hiding this comment.
Create a variable for this Object.values(FilterStates)
src/components/TodoItem.tsx
Outdated
| await onDelete(todo.id); | ||
| } catch { | ||
| setError(ErrorMessages.DELETING_TODOS); | ||
| setTimeout(() => setError(null), 3000); |
There was a problem hiding this comment.
Move the setTimeout to the useEffect and add the array dependency with ErrorMessages
src/components/TodoItem.tsx
Outdated
| }; | ||
|
|
||
| return ( | ||
| <div data-cy="Todo" className={`todo ${todo.completed ? 'completed' : ''}`}> |
There was a problem hiding this comment.
it's better to use classnames in cases like that, it allows you to dynamically toggle classes based on state or props without manually concatenating strings, making the code more maintainable and easier to read.
https://WatarJoy.github.io/react_todo-app-with-api/