Skip to content

Develop#2155

Open
adadiada wants to merge 6 commits intomate-academy:masterfrom
adadiada:develop
Open

Develop#2155
adadiada wants to merge 6 commits intomate-academy:masterfrom
adadiada:develop

Conversation

@adadiada
Copy link

No description provided.

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.

Checked this link
https://adadiada.github.io/react_todo-app-with-api/
It seems that project wasn't deployed yet. Please, deploy the project and provide DEMO LINK.

Also try to pass all tests

return (
<div
data-cy="ErrorNotification"
className={`notification is-danger is-light has-text-weight-normal ${

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 classNames here

<nav className="filter" data-cy="Filter">
<a
href="#/"
className={`filter__link ${filter === Filter.All ? 'selected' : ''}`}

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 classNames here


<a
href="#/active"
className={`filter__link ${filter === Filter.Active ? 'selected' : ''}`}

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 classNames here

className={`filter__link ${filter === Filter.Active ? 'selected' : ''}`}
data-cy="FilterLinkActive"
onClick={e => {
e.preventDefault();

Choose a reason for hiding this comment

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

I would recommend to extract this code into a separate callback


<a
href="#/completed"
className={`filter__link ${filter === Filter.Completed ? 'selected' : ''}`}

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 classNames here

className="todoapp__clear-completed"
data-cy="ClearCompletedButton"
disabled={!hasCompleted}
onClick={() => {

Choose a reason for hiding this comment

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

I would recommend to shorten click handler like this onClick={onClear}

{todos.length > 0 && (
<button
type="button"
className={

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 classNames here


return (
<section
className={`todoapp__main ${(todos.length === 0 && !tempTodo) || loading ? 'hidden' : ''}`}

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 classNames here

type="checkbox"
className="todo__status"
checked={todo.completed}
onChange={() =>

Choose a reason for hiding this comment

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

It's better to shorten the callback structure onChange={() => handleUpdateTodo({ ...todo, completed: !todo.completed })}

data-cy="HideErrorButton"
type="button"
className="delete"
onClick={() => {

Choose a reason for hiding this comment

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

It's better to shorten click handler like this onClick={() => onCloseError()}

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.

Seems like, you still didnt deploy your solution. Also, check all previous mentors comments and try to pass all tests before requesting the review:
Image

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.

3 participants