Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job on implementing the Autocomplete component! The core features like debouncing and suggestion handling are working well.
However, this requires a change because there's one edge case that needs to be addressed. Currently, the filtering logic doesn't handle input that consists only of spaces, which violates a checklist requirement [CHECKLIST ITEM #1].
In src/Autocomplete/Autocomplete.tsx, please ensure that you trim the debouncedQuery before deciding whether to filter the list. If the trimmed query is empty, the filter should not be called.
Once you've fixed this, your solution will be complete. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
src/Autocomplete/Autocomplete.tsx
Outdated
| const normalized = debouncedQuery.toLowerCase(); | ||
|
|
||
| // eslint-disable-next-line prettier/prettier | ||
| const result = | ||
| debouncedQuery === '' | ||
| ? people // Показати всіх людей, якщо поле пусте | ||
| : people.filter(person => | ||
| person.name.toLowerCase().includes(normalized), | ||
| ); |
There was a problem hiding this comment.
This filtering logic does not handle input that consists only of spaces. Currently, if a user enters spaces, people.filter is still called, which violates checklist item #1: "make sure that filter won't be called if user enred spaces only".
You should trim the debouncedQuery before deciding whether to filter. If the trimmed query is empty, you should handle this case separately (e.g., by showing no suggestions) to avoid calling the filter.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this component! I am approving your solution as it meets all the core requirements of the task. You've successfully addressed the previous feedback, and the functionality for filtering, debouncing, and managing state is implemented correctly.
There is one minor user experience improvement you could consider. In src/Autocomplete/Autocomplete.tsx, the "No matching suggestions" message can briefly flicker while typing. This happens because its visibility is tied to the query state, which updates instantly, while the suggestions list updates based on the debouncedQuery. To fix this, you could also base the condition for showing this message on the debouncedQuery.
This is just a suggestion for refinement, and your implementation is solid. Great job!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| </p> | ||
| </div> | ||
| ))} | ||
| {visiblePeople.length === 0 && query !== '' && ( |
There was a problem hiding this comment.
The condition for displaying 'No matching suggestions' currently uses query. This can cause the message to flash briefly while waiting for the debounced search to complete, because visiblePeople is temporarily an empty array.
To prevent this, the condition should be based on a state that reflects that a search has actually been performed, such as debouncedQuery.
2 participants
DEMO LINK