-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Solution #814
base: master
Are you sure you want to change the base?
Solution #814
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good progress!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/App.tsx
Outdated
@@ -1,24 +1,288 @@ | |||
/* eslint-disable max-len */ | |||
/* eslint-disable jsx-a11y/control-has-associated-label */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to rewrite the code so that it is not necessary to disable the linter
src/App.tsx
Outdated
useEffect(() => { | ||
if (errorMessage) { | ||
setTimeout(() => { | ||
setErrorMessage(Error.None); | ||
}, 3000); | ||
} | ||
}, [errorMessage]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please read the article, there is an example of how to correctly work with timers in useEffect
.
src/App.tsx
Outdated
const promiseArray = ( | ||
allTodosAreCompleted | ||
? completedTodosCount | ||
: activeTodosCount).map((todo: { id: number; completed: boolean; }) => client.patch(`/todos/${todo.id}`, { completed: !todo.completed })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better readability, it is better to split the code into small parts
src/App.tsx
Outdated
const [errorMessage, setErrorMessage] = useState(Error.None); | ||
const [value, setValue] = useState(''); | ||
const [isSubmitted, setIsSubmitted] = useState(false); | ||
const [areSubmiting, setAreSubmiting] = useState(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const [areSubmiting, setAreSubmiting] = useState(false); | |
const [isSubmiting, setIsSubmiting] = useState(false); |
src/App.tsx
Outdated
|
||
<div className="todoapp__content"> | ||
<header className="todoapp__header"> | ||
{todos.length !== 0 && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{todos.length !== 0 && ( | |
{!!todos.length && ( |
src/components/TodoItem/TodoItem.tsx
Outdated
disabled={processingIds.includes(todo.id) | ||
|| togglingId === todo.id | ||
|| areSubmiting} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to put it in a separate variable.
src/components/TodoItem/TodoItem.tsx
Outdated
'is-active': processingIds.includes(todo.id) | ||
|| togglingId === todo.id | ||
|| areSubmiting, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to put it in a separate variable.
{/* <button | ||
type="button" | ||
className="todo__remove" | ||
data-cy="TodoDelete" | ||
> | ||
× | ||
</button> */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is it?
|
||
<div | ||
data-cy="TodoLoader" | ||
className={className('modal overlay', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
className={className('modal overlay', { | |
className={className('modal', 'overlay', { |
src/utils/functions.ts
Outdated
export function getItemsLeftCountMessage(activeTodos: Todo[]) { | ||
switch (activeTodos.length) { | ||
case 1: | ||
return '1 items left'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return '1 items left'; | |
return '1 item left'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost done!
src/App.tsx
Outdated
useEffect(() => { | ||
if (errorMessage) { | ||
setTimeout(() => { | ||
setErrorMessage(Error.None); | ||
}, 3000); | ||
} | ||
}, [errorMessage]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please read the article, there is an example of how to correctly work with timers in useEffect
.
src/App.tsx
Outdated
style={{ | ||
visibility: | ||
completedTodosCount.length | ||
? 'visible' | ||
: 'hidden', | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using inline styles is a bad practice, use css instead.
src/utils/functions.ts
Outdated
case Status.All: { | ||
filteredTodos = todos; | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You do the same in the default
block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wel done 👍
DEMO LINK