-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
asdasdada #2906
base: master
Are you sure you want to change the base?
asdasdada #2906
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.
Great job on implementing the task! 🎉 While there are a few minor issues, such as the typo in the prop name heighestId
and the use of index as a key, these do not significantly impact the functionality of your code. Keep up the good work and consider addressing these minor points to improve your code quality even further. 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/App.tsx
Outdated
<FormPost | ||
onSubmit={addPosts} | ||
users={usersFromServer} | ||
heighestId={highestId} |
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 is a typo in the prop name heighestId
. It should be highestId
to match the variable name defined earlier in the code.
src/components/TodoList/TodoList.tsx
Outdated
return ( | ||
<section className="TodoList"> | ||
{todos.map((todo, indx) => { | ||
return <TodoInfo key={indx} todo={todo} />; |
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 the index indx
as a key can lead to issues with component re-rendering. Consider using a unique identifier from the todo
object, such as todo.id
, as the key instead.
src/components/formPost/formPost.tsx
Outdated
type Props = { | ||
onSubmit: (newAdd: ToDo) => void; | ||
users: User[]; | ||
heighestId: number; |
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 is a typo in the prop name heighestId
. It should be highestId
to match the variable name used in other parts of the code.
src/components/formPost/formPost.tsx
Outdated
const userId: User | null = | ||
users.find(user => user.name === select) || null; |
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.
The variable userId
is redundant as it repeats the logic of foundUser
. You can directly use foundUser?.id
in the onSubmit
call.
src/components/formPost/formPost.tsx
Outdated
</option> | ||
{users.map((user, indx) => { | ||
return ( | ||
<option value={user.name} key={indx}> |
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 the index indx
as a key can lead to issues with component re-rendering. Consider using a unique identifier from the user
object, such as user.id
, as the key instead.
No description provided.