Develop #848
Develop #848
Conversation
There was a problem hiding this comment.
Good work, but some comments on the code and the application itself:
- If we try to create a todo with an empty name, we will see an error, but after that the input becomes inactive and we will not be able to add anything new, only after reloading the page:
https://gyazo.com/bc0b7e5aaf5f416faa31f3a517b631d1 - If we exit the edit mode using the Escape key, the new name will not be saved, but the next time we enter the edit mode, we will see the next state from the previous entry into the edit mode:
https://gyazo.com/1f59971ebddccd3694d2814a556551d0
src/App.tsx
Outdated
| /* eslint-disable max-len */ | ||
| /* eslint-disable jsx-a11y/control-has-associated-label */ |
There was a problem hiding this comment.
Try to rewrite the code so that it is not necessary to disable the linter
src/App.tsx
Outdated
| completed: false, | ||
| }); | ||
|
|
||
| // eslint-disable-next-line consistent-return |
There was a problem hiding this comment.
Try to rewrite the code so that it is not necessary to disable the linter
src/App.tsx
Outdated
| todos | ||
| .filter(todo => todo.completed) | ||
| .forEach(todo => handleTodoDelete(todo.id)); |
There was a problem hiding this comment.
No need to do extra filtering, this condition can be transferred to the middle of the forEach method
src/App.tsx
Outdated
| todos | ||
| .filter(todo => !todo.completed) | ||
| .forEach(handleTodoToggle); |
src/App.tsx
Outdated
| if (isEveryCompleted) { | ||
| todos.forEach(handleTodoToggle); | ||
| } else { | ||
| todos |
There was a problem hiding this comment.
| if (isEveryCompleted) { | |
| todos.forEach(handleTodoToggle); | |
| } else { | |
| todos | |
| if (isEveryCompleted) { | |
| return todos.forEach(handleTodoToggle); | |
| } | |
| todos |
src/App.tsx
Outdated
| {todos.length > 0 && ( | ||
| <footer className="todoapp__footer" data-cy="Footer"> | ||
| <span className="todo-count" data-cy="TodosCounter"> | ||
| {`${activeTodosCounter} items left`} |
There was a problem hiding this comment.
If there is one item, there should not be a plural in the text.
src/components/TodoHeader.tsx
Outdated
| @@ -0,0 +1,81 @@ | |||
| /* eslint-disable jsx-a11y/control-has-associated-label */ | |||
There was a problem hiding this comment.
Try to rewrite the code so that it is not necessary to disable the linter
src/components/TodoHeader.tsx
Outdated
| .then(() => { | ||
| setTodoTitle(''); | ||
| }) | ||
| .finally(() => { |
src/components/TodoRow.tsx
Outdated
| if (todoTitle.trim()) { | ||
| if (todoTitle.trim() !== todo.title) { | ||
| onTodoRename(todoTitle.trim()); |
There was a problem hiding this comment.
You call todoTitle.trim() three times - you should put it in a separate variable.
src/components/TodoRow.tsx
Outdated
| </button> | ||
| </> | ||
| )} | ||
| {/* overlay will cover the todo while it is being updated */} |
There was a problem hiding this comment.
Do not leave unnecessary comments.
DEMO LINK