Conversation
Kobeliukh
commented
Mar 14, 2026
Kobeliukh
commented
- DEMO LINK
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This is a solid implementation, but I'm requesting a few changes to fully meet the task requirements. The Dropdown component is missing the required delay prop for customizing the debounce time; it's currently hardcoded. Additionally, the filtering logic should handle whitespace-only input by using trim() on the query before checking if it's empty, as specified in [CHECKLIST ITEM #1]. Lastly, please remember to follow the naming conventions by renaming the onTextChange event handler to handleTextChange. After these adjustments, your solution will be ready!
✨ 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
| interface Props { | ||
| people: Person[]; | ||
| onSelected: (value: Person | null) => void; | ||
| } |
There was a problem hiding this comment.
The component should accept a delay prop to customize the debounce time, as stated in the requirements. The default value should be 300ms.
| const [isPersonSelected, setIsPersonSelected] = useState(false); | ||
| const [isOpen, setIsOpen] = useState(false); | ||
|
|
||
| const onTextChange = (event: React.ChangeEvent<HTMLInputElement>) => { |
There was a problem hiding this comment.
According to the naming conventions mentioned in checklist item #4, event handler functions defined within the component should be prefixed with handle. This function should be named handleTextChange.
| useEffect(() => { | ||
| const timerId = setTimeout(() => { | ||
| setFilterQuery(inputText); | ||
| }, 1000); |
There was a problem hiding this comment.
The debounce delay is hardcoded. It should use the delay prop as required by the task, with a default of 300ms.
| }, [inputText]); | ||
|
|
||
| const filteredPeople = useMemo(() => { | ||
| if (!filterQuery) { |
There was a problem hiding this comment.
The filter is triggered even for input containing only spaces. You should trim() the filterQuery before checking if it's empty to prevent this. This violates checklist item #1: 'make sure that filter won't be called if user enred spaces only'.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great progress on this! The core functionality is looking solid. However, a few changes are needed to meet all the task requirements before this can be approved.
Here are the specific items to address in src/components/Dropdown/Dropdown.tsx:
- Default Delay: The default value for the
delayprop is currently1000ms, but the requirement specifies it should be300ms. - Whitespace Filtering: The filter is still triggered for inputs containing only spaces. To fix this, please trim the query value before checking if it's empty to decide whether to run the filter. This addresses checklist item #1.
- Naming Convention: The event handler
onTextChangeshould be renamed tohandleTextChangeto align with the project's naming conventions from checklist item #4.
Once these adjustments are made, it should be all set. Keep up the excellent 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
| export const Dropdown = ({ | ||
| people, | ||
| onSelected, | ||
| delay = 1000, |
There was a problem hiding this comment.
Nice work making the delay customizable! However, the task description specifies that the default value for the delay should be 300ms.
| const [isPersonSelected, setIsPersonSelected] = useState(false); | ||
| const [isOpen, setIsOpen] = useState(false); | ||
|
|
||
| const onTextChange = (event: React.ChangeEvent<HTMLInputElement>) => { |
There was a problem hiding this comment.
To follow the naming conventions outlined in checklist item #4, this event handler should be named handleTextChange.
| }, [inputText]); | ||
|
|
||
| const filteredPeople = useMemo(() => { | ||
| if (!filterQuery) { |
There was a problem hiding this comment.
This check doesn't account for a string that contains only whitespace (e.g., ' '). In that case, filterQuery is truthy, and the filtering logic runs unnecessarily. To meet the requirement from checklist item #1, you should check the trimmed query, for example: if (!filterQuery.trim()).
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 submission! I'm happy to approve your solution. You've successfully addressed all the previous feedback, and the component is now working perfectly. It's great to see that you've implemented the correct default delay, handled whitespace inputs, and followed the naming conventions. The code is clean, well-structured, and fully meets all the requirements. Well done!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
