Add solution of atsk react_pople-table-basics #1797
Conversation
There was a problem hiding this comment.
Great job on implementing the task requirements! 🎉 The code is mostly well-structured and meets the core functionality needed. There are a few minor adjustments suggested, such as ensuring the HashRouter is used for GitHub Pages compatibility, including slug in the useEffect dependency array, and considering the use of a PersonLink component. These are small tweaks that can enhance your solution. Keep up the good work and happy coding! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -1,167 +1,52 @@ | |||
| import { Loader } from './components/Loader'; | |||
|
|
|||
| import { NavLink, Routes, Route, Navigate } from 'react-router-dom'; | |||
There was a problem hiding this comment.
Ensure that you are using HashRouter instead of the default router to be compatible with GitHub Pages, as specified in the task requirements.
| <Route path="/people" element={<People />}> | ||
| <Route path=":slug" element={<People/>} /> | ||
| </Route> |
There was a problem hiding this comment.
The nested route for People with :slug might not be necessary unless there's a specific requirement for it. Consider simplifying the routing if this is not needed.
| }; | ||
|
|
||
| fetchPeople(); | ||
| }, []); |
There was a problem hiding this comment.
Include slug in the dependency array of useEffect to ensure the effect re-runs when the URL parameter changes. This will correctly update the highlighted person when navigating directly to a person's page.
| return ( | ||
| <tr data-cy="person" key={person.name} className={selectedPerson?.name === person.name ? "has-background-warning" : ''}> | ||
| <td> | ||
| <Link to={`/people/${person.slug}`} onClick={() => selectPerson(person)} className={person.sex === 'f' ? 'has-text-danger' : ''}>{person.name}</Link> |
There was a problem hiding this comment.
Consider creating and using a PersonLink component for rendering links to people, as specified in the task requirements.
DEMO LINK