Skip to content

add task solution#2418

Open
ratushnjakivanko-spec wants to merge 1 commit into
mate-academy:masterfrom
ratushnjakivanko-spec:develop
Open

add task solution#2418
ratushnjakivanko-spec wants to merge 1 commit into
mate-academy:masterfrom
ratushnjakivanko-spec:develop

Conversation

@ratushnjakivanko-spec

Copy link
Copy Markdown

No description provided.

@2pasha 2pasha 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.

good job! 👏

take a look on comments below ⬇️

Comment thread src/App.tsx

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

separate your App to components

Comment thread src/App.tsx
} from './api/todos';
import { Todo } from './types/Todo';

type FilterStatus = 'all' | 'active' | 'completed';

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 to separate file into types

Comment thread src/App.tsx
setErrorMsg('');
getTodos()
.then(setTodos)
.catch(() => showError('Unable to 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.

move all your error messages to enum

Comment thread src/App.tsx
}

const filteredTodos = todos.filter(todo => {
if (filter === '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.

it's better to use enum here

Comment thread src/App.tsx
Comment on lines +247 to +250
onSubmit={e => {
e.preventDefault();
handleEditSave(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 to separate fn

Comment thread src/App.tsx
Comment on lines +337 to +362
<a
href="#/"
className={`filter__link ${filter === 'all' ? 'selected' : ''}`}
data-cy="FilterLinkAll"
onClick={() => setFilter('all')}
>
All
</a>

<a
href="#/active"
className={`filter__link ${filter === 'active' ? 'selected' : ''}`}
data-cy="FilterLinkActive"
onClick={() => setFilter('active')}
>
Active
</a>

<a
href="#/completed"
className={`filter__link ${filter === 'completed' ? 'selected' : ''}`}
data-cy="FilterLinkCompleted"
onClick={() => setFilter('completed')}
>
Completed
</a>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

DRY
you may add all your filter values to kind of array and map through it

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