-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
need help #1485
base: master
Are you sure you want to change the base?
need help #1485
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- show the loader on all todos that we need to update/delete immediately
Screen.Recording.2024-10-21.at.10.52.13.mov
- remove all comments
src/components/TodoList.tsx
Outdated
// update: (todoToUpdate: Todo) => void; | ||
// errorFunction: (message: string) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove all comments
src/components/TodoHeader.tsx
Outdated
className={classNames('todoapp__toggle-all', { | ||
active: checkActiveTodos(), | ||
})} | ||
onClick={() => handleToggleAll()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onClick={() => handleToggleAll()} | |
onClick={handleToggleAll} |
src/api/todos.ts
Outdated
export const getActiveTodos = () => { | ||
return getTodos().then(todos => todos.filter(todo => !todo.completed)); | ||
}; | ||
|
||
export const getCompletedTodos = () => { | ||
return getTodos().then(todos => todos.filter(todo => todo.completed)); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to make separate requests for active or completed todos, just filter the todos state in the component
src/api/todos.ts
Outdated
}; | ||
|
||
export const createTodo = ({ userId, title, completed }: Omit<Todo, 'id'>) => { | ||
return client.post<Todo>(`/todos`, { userId, title, completed }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return client.post<Todo>(`/todos`, { userId, title, completed }); | |
return client.post<Todo>('/todos', { userId, title, completed }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
The app always shows an error after deleting the todo (if the user deletes the title and presses enter)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix this case(you should delete the todo in this case, but you try to update it)
https://github.com/user-attachments/assets/892759a6-7a65-4885-b19a-64ccd16e8f44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job
https://Shevchuchka.github.io/react_todo-app-with-api/