Skip to content

solution#2236

Open
andriy-stetsula wants to merge 7 commits into
mate-academy:masterfrom
andriy-stetsula:develop
Open

solution#2236
andriy-stetsula wants to merge 7 commits into
mate-academy:masterfrom
andriy-stetsula:develop

Conversation

@andriy-stetsula

Copy link
Copy Markdown

No description provided.

@dothearts dothearts left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please provide DEMO LINK.

  • PR description
  • README.md

Also, check the comments and fix them.

Many thanks!

Comment thread src/App.tsx
getTodos()
.then(data => setTodo(data))
.catch(() => setError(LOAD_ERROR.LOAD_TODOS));
}, []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add linter ignore for this line

Comment thread src/App.tsx Outdated
}, [error]);

const filterTodo = todo.filter(todos => {
if (selected === 'active') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need use ENUM for statuses

Comment thread src/App.tsx Outdated
}
}, [error]);

const filterTodo = todo.filter(todos => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can create a pure function and move it to a separate file (helpers.ts for example)

Comment thread src/App.tsx Outdated
setTitle('');
})

.catch(() => setError('Unable to add a todo'))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have ENUM for this, move error in there and use in this place.

@andriy-stetsula

andriy-stetsula commented Jun 2, 2026

Copy link
Copy Markdown
Author

@andriy-stetsula andriy-stetsula requested a review from dothearts June 2, 2026 13:52

@dothearts dothearts left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix some minority comments.

Many thanks!

Comment thread src/App.tsx Outdated
setLoadingIds(prev => [...prev, id]);
deleteTodo(id)
.then(() => setTodo(prev => prev.filter(t => t.id !== id)))
.catch(() => setError('Unable to delete a todo'))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this to Error enum

Comment thread src/App.tsx Outdated
)
.catch(() => setError('Unable to update a todo'))
.finally(() => {
setLoadingIds(prev => prev.filter(Ids => Ids !== currentTodo.id));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Ids not ids?
As i see you don't need capitalize in this case.

Comment thread src/App.tsx Outdated
const handleDelete = (id: number) => {
setLoadingIds(prev => [...prev, id]);
deleteTodo(id)
.then(() => setTodo(prev => prev.filter(t => t.id !== id)))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix all one letter notation in callbacks.

@dothearts dothearts left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request Changes*

@andriy-stetsula andriy-stetsula requested a review from dothearts June 2, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants