add task solution #1135
add task solution #1135
Conversation
There was a problem hiding this comment.
Great job on implementing the task requirements! 🎉 Your code meets the core functionality and constraints, and you've effectively used React and React Router for managing state and navigation. While there are a few areas for optimization and improvement, such as optimizing data handling and refining error messages, these do not critically impact the functionality of your solution. Keep up the good work and consider these suggestions for future enhancements! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| {['16', '17', '18', '19', '20'].map(century => ( | ||
| <SearchLink | ||
| key={century} | ||
| params={{ centuries: toggleCentury(century) }} |
There was a problem hiding this comment.
The toggleCentury function is called directly within the params prop of SearchLink. This means it will be executed on every render, which could lead to unexpected behavior. Consider refactoring this logic to ensure toggleCentury is only called when necessary, such as within an event handler.
| setPeople(data); | ||
| setLoading(false); | ||
| } catch (e) { | ||
| setError('Something went wrong'); |
There was a problem hiding this comment.
Consider providing more detailed error information to the user, if available. This can help users understand what went wrong and potentially how to fix it. You might want to log the actual error message or display it conditionally.
| @@ -1,5 +1,65 @@ | |||
| /* eslint-disable jsx-a11y/control-has-associated-label */ | |||
| export const PeopleTable = () => { | |||
| import React from 'react'; | |||
| import data from '../../public/api/people.json'; | |||
There was a problem hiding this comment.
Importing data directly from a JSON file within the component can lead to performance issues, especially if the data is large. Consider fetching the data asynchronously or passing it as a prop to the component.
| import { Person } from '../types/Person'; | ||
|
|
||
| const getSlugByName = (name: string) => { | ||
| const person = data.find(p => p.name === name); |
There was a problem hiding this comment.
The getSlugByName function searches through the entire dataset each time it's called. This could be inefficient if the dataset is large. Consider optimizing this by using a more efficient data structure or caching the results.
-DEMO LINK