feat: add keyboard event support to SearchBar#1425
Conversation
Signed-off-by: AasthathecoderX <bhat.aasthaa@gmail.com>
|
The below are the videos showing the working:: enter_working.mp4Tab_working.mp4up_downworking.mp4spacebar_working.mp4 |
There was a problem hiding this comment.
Code Review
This pull request introduces an onKeyDown prop to the SearchBar and StyledSearchBar components, enabling custom keyboard event handling and search execution via the 'Enter' key. Feedback focuses on enhancing the handleKeyDown logic to respect event.defaultPrevented, cleaning up redundant comments, and fixing indentation inconsistencies to maintain code quality.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Rajesh Nagarajan <139469505+Rajesh-Nagarajan-11@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Rajesh Nagarajan <139469505+Rajesh-Nagarajan-11@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: AasthathecoderX <bhat.aasthaa@gmail.com>
miacycle
left a comment
There was a problem hiding this comment.
One thing worth tightening here: pressing Enter can trigger onSearch twice. handleSearchChange already schedules the debounced search, and if Enter is pressed before that debounce expires, handleKeyDown calls onSearch(searchText) immediately while the pending debounced call still fires shortly after. Please cancel the pending debounced callback before the immediate Enter-path search (or funnel Enter through the same search path) so consumers do not get duplicate requests or state updates.
ok @miacycle ,I will look into it |
|
@AasthathecoderX Thank you for your contribution! Let's discuss this during the website call tomorrow at 5:30 PM IST | 7 AM CST Add it as an agenda item to the meeting minutes, if you would 🙂 |
|
@AasthathecoderX any updates ? |
|
@Rajesh-Nagarajan-11 ,I have informed @Bhumikagarggg due to my back to back exams I won't be able to do much this week but I will surely do it next week. |
Rajesh-Nagarajan-11
left a comment
There was a problem hiding this comment.
Make sure to resolve merge conflicts
|
@AasthathecoderX Thank you for your contribution! Let's discuss this during the website call tomorrow at 5:30 PM IST | 7 AM CST Add it as an agenda item to the meeting minutes, if you would 🙂 |
Clear existing search timeout before executing search on Enter key press. Signed-off-by: AasthathecoderX <bhat.aasthaa@gmail.com>
Signed-off-by: AasthathecoderX <bhat.aasthaa@gmail.com>
Added onKeyDown prop to handle keyboard events in StyledSearchBar. Signed-off-by: AasthathecoderX <bhat.aasthaa@gmail.com>
Added a ref to manage search timeout for debouncing the search function. Signed-off-by: AasthathecoderX <bhat.aasthaa@gmail.com>
|
@Rajesh-Nagarajan-11 ,I have done the changes. |
There was a problem hiding this comment.
Pull request overview
Adds onKeyDown support to the custom SearchBar and StyledSearchBar components so consumers can react to keyboard events (e.g. Enter to submit). SearchBar also wires Enter to trigger an immediate onSearch, attempting to bypass the debounce.
Changes:
- Add optional
onKeyDownprop toSearchBarand forward it to the underlyingTextField, with Enter triggering an immediateonSearch. - Add optional
onKeyDownprop toStyledSearchBarand forward it to the inner input. - Minor formatting cleanups (one-line
if, removed comment).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/custom/SearchBar.tsx | Adds onKeyDown prop and Enter-key handler; introduces an unused searchTimeoutRef intended to cancel a pending debounce. |
| src/custom/StyledSearchBar/StyledSearchBar.tsx | Adds and forwards onKeyDown prop with JSDoc entry. |
| if (event.key === 'Enter') { | ||
| if (searchTimeoutRef.current) { | ||
| clearTimeout(searchTimeoutRef.current); | ||
| searchTimeoutRef.current = null; | ||
| } | ||
| onSearch(searchText); | ||
| } |
Notes for Reviewers
This PR fixes #718
Signed commits