Skip to content

solution#1265

Open
Nabeyko wants to merge 4 commits intomate-academy:masterfrom
Nabeyko:develop
Open

solution#1265
Nabeyko wants to merge 4 commits intomate-academy:masterfrom
Nabeyko:develop

Conversation

@Nabeyko
Copy link

@Nabeyko Nabeyko commented Jan 29, 2026

Copy link

@Denys-Kravchuk9988 Denys-Kravchuk9988 left a comment

Choose a reason for hiding this comment

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

Good job!

A few things to improve:

className="todoapp__clear-completed"
data-cy="ClearCompletedButton"
disabled={!todoList.some(todo => todo.completed)}
onClick={() => dispatch({ type: 'deleteCompleted' })}

Choose a reason for hiding this comment

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

I would recommend to move deleteCompleted in enum ActionTypes for example

data-cy="ToggleAllButton"
disabled={todoList.length < 0}
onClick={() =>
dispatch({ type: 'toggleTodoAll', payload: !allCompleted })

Choose a reason for hiding this comment

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

I would recommend to move toggleTodoAll in enum ActionTypes for example

type="button"
className="todo__remove"
data-cy="TodoDelete"
onClick={() => dispatch({ type: 'deleteTodo', payload: todo.id })}

Choose a reason for hiding this comment

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

I would recommend to move deleteTodo in enum ActionTypes for example

todoVisible: Todo[];
};

type Action =

Choose a reason for hiding this comment

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

I would recommend to move all these types in enum ActionTypes for example

checked={todo.completed}
onChange={() =>
dispatch({
type: 'toggleTodo',

Choose a reason for hiding this comment

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

I would recommend to move toggleTodo in enum ActionTypes for example

Copy link

@brespect brespect left a comment

Choose a reason for hiding this comment

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

Lgtm 🚀

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.

4 participants