add task solution #762
add task solution #762
Conversation
There was a problem hiding this comment.
Great job on your work so far! 😊 However, there are a few improvements that need to be made. Please consider renaming your methods to make them more descriptive and check if the input value is not composed of spaces only before filtering. Keep up the good work! 💪
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/Autocomplete.tsx
Outdated
|
|
||
| useEffect(() => { | ||
| const timeoutId = setTimeout(() => { | ||
| if (inputValue.trim() === '') { |
There was a problem hiding this comment.
According to the checklist, the filter function shouldn't be called if the user entered spaces only. However, in this case, if the input value is only spaces, you will set all people as suggestions. To avoid unnecessary filtering, you should also check if the input value is not composed of spaces only.
| <div className="dropdown-content"> | ||
| {suggestions.map(person => ( | ||
| <div | ||
| key={person.slug} |
There was a problem hiding this comment.
The checklist mentions not to generate keys on render. In this case, you are using 'person.slug' as a key which is fine as long as 'slug' is unique for each person and doesn't change over time.
src/Autocomplete.tsx
Outdated
| onSelected(person); | ||
| }; | ||
|
|
||
| const handleInputChange = (event: React.ChangeEvent<HTMLInputElement>) => { |
There was a problem hiding this comment.
It's recommended to follow the naming conventions for methods. The method name 'handleInputChange' is fine, but it's better to specify what is changing. A more descriptive name could be 'handleInputValueChange'.
src/Autocomplete.tsx
Outdated
| return () => clearTimeout(timeoutId); | ||
| }, [inputValue, people, delay]); | ||
|
|
||
| const handleSelect = (person: Person) => { |
There was a problem hiding this comment.
The same applies to the 'handleSelect' method. It would be better to rename it to 'handlePersonSelect' to make it clear what is being selected.
There was a problem hiding this comment.
Great job! Your code is well-structured and readable. I've noticed a few areas for improvement, but they don't significantly impact the functionality of your code. Keep up the good work! 👍
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| const timeoutId = setTimeout(() => { | ||
| const trimmedInput = inputValue.trim(); | ||
|
|
||
| if (trimmedInput === '') { |
There was a problem hiding this comment.
According to the checklist, the filter function should not be called if the user entered only spaces. But in the current implementation, if the user enters spaces, the filter function will be called. To avoid this, you should check not only that the trimmedInput is not an empty string, but also that it is not composed entirely of spaces.
| {suggestions.map(person => ( | ||
| <div | ||
| key={person.slug} | ||
| className="dropdown-item" | ||
| onClick={() => handlePersonSelect(person)} | ||
| data-cy="suggestion-item" | ||
| > | ||
| <p className="has-text-link">{person.name}</p> | ||
| </div> |
There was a problem hiding this comment.
According to the checklist, keys should not be generated on render. In this case, you are using the slug property of the person object as a key, which is fine as long as slug is unique for each person and does not change during the component's lifecycle. If slug can change or is not unique, you will need to find another solution. Always ensure that keys are stable, predictable, and unique for each element in the list.
| const handlePersonSelect = (person: Person) => { | ||
| setInputValue(person.name); | ||
| setSelectedPerson(person); | ||
| setSuggestions([]); | ||
| onSelected(person); |
There was a problem hiding this comment.
The checklist mentions that you should follow specific naming conventions for methods. Your method handlePersonSelect is correctly named according to the conventions. However, handleInputValueChange could be renamed to handleInputChange to be more concise and still clear.
DEMO LINK