done react autocomplete task#1125
done react autocomplete task#1125maximtsyrulnyk wants to merge 4 commits intomate-academy:masterfrom
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This is a strong submission that correctly implements most of the requirements. However, I'm requesting a change to address a significant usability issue.
In src/components/Autocomplete.tsx, when the list of suggestions is long enough to require a scrollbar, clicking on the scrollbar causes the input to lose focus and closes the dropdown. This prevents the user from being able to scroll and select items that aren't immediately visible.
To fix this, you can add an onMouseDown handler to the dropdown menu container and call event.preventDefault(). This will prevent the input from blurring when the user interacts with the scrollbar. After you've made this fix, the component will be fully functional!
✨ 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
| onSelected(null); | ||
| }} | ||
| onFocus={() => setIsActive(true)} | ||
| onBlur={() => setIsActive(false)} |
There was a problem hiding this comment.
This onBlur handler causes a usability issue. If the list of suggestions is long enough to have a scrollbar, clicking on the scrollbar will make the input lose focus, which triggers this onBlur and immediately closes the dropdown. This makes it impossible for the user to scroll the list.
A common way to fix this is to prevent the mousedown event on the dropdown menu from causing the input to blur. You can add an onMouseDown handler to the div.dropdown-menu element that calls event.preventDefault().
|
edited code according remarks |
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 core features of the autocomplete component! The code is clean and most of the functionality works as expected.
However, I am requesting changes because the critical usability issue with the scrollbar is still present. When the list of suggestions is long, clicking the scrollbar closes the dropdown, making it impossible to select items that aren't immediately visible. This happens because the input's onBlur event fires when you click the scrollbar.
To fix this, you can prevent the dropdown menu from taking focus away from the input. Add an onMouseDown handler to the div with the dropdown-menu class and call event.preventDefault():
onMouseDown={(event) => event.preventDefault()}Once you've applied this fix, the component will be ready to go!
✨ 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
| onSelected(null); | ||
| }} | ||
| onFocus={() => setIsActive(true)} | ||
| onBlur={() => setIsActive(false)} |
There was a problem hiding this comment.
This onBlur handler is the reason the dropdown closes when you click on the scrollbar. Clicking the scrollbar removes focus from the input, which triggers this handler and sets isActive to false. While this works for closing the dropdown when clicking elsewhere, it creates a poor user experience for scrollable lists.
| /> | ||
| </div> | ||
|
|
||
| <div className="dropdown-menu" role="menu" data-cy="suggestions-list"> |
There was a problem hiding this comment.
To fix the scrollbar issue, you can prevent the blur event from firing when the user interacts with the dropdown menu. As suggested in the previous review, adding an onMouseDown handler here and calling event.preventDefault() will solve the problem.
onMouseDown={(event) => event.preventDefault()}This prevents the mousedown event on the scrollbar from shifting focus away from the input, keeping the dropdown open.
|
edited code according requirements DEMO LINK |
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on this component! The debouncing and filtering logic are implemented very well. I am approving this solution as an educational override to help you move forward, but there is one critical issue left to address.
Currently, clicking the scrollbar in the suggestions list causes the dropdown to close, making it impossible to select items that aren't immediately visible. This is because the input's onBlur event fires.
To fix this, please add onMouseDown={(event) => event.preventDefault()} to the div with the dropdown-menu class in src/components/PeopleAutocomplete.tsx. This will prevent the input from losing focus when you interact with the scrollbar. Keep up the excellent effort!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| onBlur={() => setIsActive(false)} | ||
| /> | ||
| </div> | ||
|
|
||
| <div className="dropdown-menu" role="menu" data-cy="suggestions-list"> |
There was a problem hiding this comment.
The onBlur handler here is causing the issue mentioned in the last review. When a user tries to click the scrollbar in the suggestions list, this onBlur event fires, closing the dropdown before they can scroll.
As suggested previously, you can fix this by adding onMouseDown={(event) => event.preventDefault()} to the div with the dropdown-menu class on line 62. This prevents the input from losing focus when interacting with the dropdown.
2 participants
done react autocomplete task
DEMO LINK