Develop #821
Develop #821
Conversation
There was a problem hiding this comment.
Good job, but there is quite a bit of work to be done.
Now, after pressing Enter, it does not update there (but it should according to the task):
https://gyazo.com/4b857711b0dde3b29be9273e13ae73fd
src/App.tsx
Outdated
| @@ -1,24 +1,317 @@ | |||
| /* 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
|
|
||
| export const App: React.FC = () => { | ||
| const [todos, setTodos] = useState<Todo[]>([]); | ||
| const [isLoadig, setIsLoading] = useState(false); |
There was a problem hiding this comment.
| const [isLoadig, setIsLoading] = useState(false); | |
| const [isLoading, setIsLoading] = useState(false); |
src/App.tsx
Outdated
|
|
||
| <div className="todoapp__content"> | ||
| <header className="todoapp__header"> | ||
| {/* this buttons is active only if there are some active todos */} |
There was a problem hiding this comment.
Do not leave unnecessary comments.
src/App.tsx
Outdated
| /> | ||
| )} | ||
|
|
||
| {/* Add a todo on form submit */} |
There was a problem hiding this comment.
Do not leave unnecessary comments.
src/App.tsx
Outdated
| handleClearCompleted={handleClearCompleted} | ||
| /> | ||
| )} | ||
|
|
| return ( | ||
| <footer className="todoapp__footer" data-cy="Footer"> | ||
| <span className="todo-count" data-cy="TodosCounter"> | ||
| {`${countActiveTodos} items left`} |
There was a problem hiding this comment.
If there is one item, there should not be a plural in the text.
There was a problem hiding this comment.
If there is one item, there should not be a plural in the text.
I can't change it because the tests failed
src/App.tsx
Outdated
| setIsLoading(false); | ||
| }) | ||
| .catch(error => { | ||
| // eslint-disable-next-line no-console | ||
| console.warn(error); | ||
| setTodoError(ErrorType.GetData); | ||
| setIsLoading(false); |
There was a problem hiding this comment.
You reset the loading state in both cases so move it to the finally block
src/App.tsx
Outdated
| useEffect(() => { | ||
| const timeoutId = setTimeout(() => { | ||
| setTodoError(null); | ||
| }, 3000); | ||
|
|
||
| return () => clearTimeout(timeoutId); | ||
| }, []); |
There was a problem hiding this comment.
| useEffect(() => { | |
| const timeoutId = setTimeout(() => { | |
| setTodoError(null); | |
| }, 3000); | |
| return () => clearTimeout(timeoutId); | |
| }, []); | |
| useEffect(() => { | |
| if (!todoError) { | |
| return; | |
| } | |
| const timeoutId = setTimeout(() => { | |
| setTodoError(null); | |
| }, 3000); | |
| return () => clearTimeout(timeoutId); | |
| }, [todoError]); |
Add dependency to create timeout every time you have an error.
Also, create an ErrorNotification component and move this logic to it too.
src/App.tsx
Outdated
| setTimeout(() => { | ||
| setTodoError(null); | ||
| }, 3000); |
There was a problem hiding this comment.
Set this time in useEffect as I mentioned before
src/App.tsx
Outdated
| showError(ErrorType.Title); | ||
| } else { |
There was a problem hiding this comment.
| showError(ErrorType.Title); | |
| } else { | |
| showError(ErrorType.Title); | |
| return; | |
| } | |
| // Other code |
Try to avoid else
src/App.tsx
Outdated
| <h1 className="todoapp__title">todos</h1> | ||
|
|
||
| <div className="todoapp__content"> | ||
| <header className="todoapp__header"> |
src/App.tsx
Outdated
| </div> | ||
|
|
||
| <div | ||
| data-cy="ErrorNotification" |
There was a problem hiding this comment.
Create an ErrorNotification component
src/components/TodoItem.tsx
Outdated
| if (newTitle === todo.title) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
| if (newTitle === todo.title) { | |
| return; | |
| } | |
| if (newTitle === todo.title) { | |
| setIsEditing(false); | |
| return; | |
| } |
Maybe you should add setIsEditing(false); here, I can't close editing if the title is the same
src/components/TodoItem.tsx
Outdated
| return; | ||
| } | ||
|
|
||
| if (newTitle.trim() === '') { |
There was a problem hiding this comment.
| if (newTitle.trim() === '') { | |
| if (!newTitle.trim()) { |
Don't compare it with an empty string, just check if exists
DEMO LINK