solution #1735
solution #1735
Conversation
| return ( | ||
| <tr | ||
| data-cy="person" | ||
| className={person.slug === slug ? 'has-background-warning' : ''} |
There was a problem hiding this comment.
you can try use classNames here, because you don't have second option
| <td> | ||
| <Link | ||
| to={`${person.slug}`} | ||
| className={person.sex === 'f' ? 'has-text-danger' : ''} |
There was a problem hiding this comment.
you can try use classNames here, because you don't have second option
| </td> | ||
| <td> | ||
| {person.father ? ( | ||
| <Link to={person.father.slug}>{person.father.name}</Link> |
There was a problem hiding this comment.
you use here father.name but in previous line - motherName. I may be wrong, but I think it's better to use similar notation for these outputs
| if (mam) { | ||
| modifiedPerson.mother = mam; | ||
| } | ||
|
|
||
| if (dad) { | ||
| modifiedPerson.father = dad; | ||
| } |
There was a problem hiding this comment.
you can delete this because find will return undefined anyway
There was a problem hiding this comment.
Great job on implementing the task requirements! 🎉 While there are a few minor issues, such as the potential re-fetching logic in PeoplePage, the incorrect Link attribute for the father's name, and the nested route setup, these do not critically impact the core functionality. Keep up the good work, and consider addressing these points to enhance your code further. Happy coding! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| <Route path="people" element={<PeoplePage />}> | ||
| <Route path=":slug" element={<PeoplePage />} /> |
There was a problem hiding this comment.
The nested route <Route path=":slug" element={<PeoplePage />} /> might cause unexpected behavior if PeoplePage is not designed to handle both the base path /people and the dynamic path /people/:slug. Consider verifying that PeoplePage can handle both cases or use a different component for the dynamic route if necessary.
| </td> | ||
| <td> | ||
| {person.father ? ( | ||
| <Link to={person.father.slug}>{person.father.name}</Link> |
There was a problem hiding this comment.
The Link component for the father's name uses person.father.name, but the to attribute should be person.father.slug to ensure correct navigation. Verify that person.father has a slug property and use it here.
| const [isError, setIsError] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| if (!isError) { |
There was a problem hiding this comment.
The condition if (!isError) inside the useEffect hook might cause the fetch operation to be retried immediately after an error, due to the setIsError(false) call inside the catch block. Consider revising the logic to prevent unnecessary re-fetching, such as by using a separate state to control retries.
DEMO LINK