Skip to content
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

create todo app #807

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

vanya-kalyenichenko
Copy link

src/App.tsx Outdated
Clear completed
</button>
</footer>
<TodosFilter />

Choose a reason for hiding this comment

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

Suggested change
<TodosFilter />
<TodosFilter />

}, [isEdit]);

const handleKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
if (event.key === 'Enter' && editingText) {

Choose a reason for hiding this comment

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

enter is a magic word
Consider will be better if you create a enum for magic words and use it everywhere

const completedFilter = filterTodos.filter((todo: Todos) => todo.completed);

switch (status) {
case Status.all:

Choose a reason for hiding this comment

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

enum's items should be in camelCase or UPPER_CASE

Suggested change
case Status.all:
case Status.All:

completed = '#/completed',
}

export default Status;

Choose a reason for hiding this comment

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

redundant

Suggested change
export default Status;

Comment on lines 1 to 7
type Todos = {
id: number
title: string
completed: boolean
};

export default Todos;

Choose a reason for hiding this comment

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

Suggested change
type Todos = {
id: number
title: string
completed: boolean
};
export default Todos;
export type Todos = {
id: number
title: string
completed: boolean
};

Copy link

@etojeDenys etojeDenys left a comment

Choose a reason for hiding this comment

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

  1. looks different
image image
  1. show clear when completed todo exist
image

<ul className="todo-list" data-cy="todoList">
{filterTodos.map((todo) => (
<TodoItem
key={`${todo.id}`}

Choose a reason for hiding this comment

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

Suggested change
key={`${todo.id}`}
key={todo.id}

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