Repository navigation
feat(ui): add search input clear buttons and unit test coverage - #119
Conversation
|
Hi maintainers, I’ve completed the changes for #119 and verified the PR with the full test suite, linting, formatting, and build checks. All checks are passing (311 tests across 33 test files). When you have a chance, could someone please review the PR? Thank you! |
|
hey @hrithik577, can you plz provide ss of before & after changes. Currently PR preview is not implemented yet, so its difficult to see the ui changes. i am working on it in issue 87. suggestion for future, plz better to create a issue first to have a context about the upcoming changes :) |
|
@PratikMane0112 ya i will look after it |
|
Hi @PratikMane0112, here are the Before & After screenshots for the search bar clear button changes: 1. Plugin Search Bar (
|
There was a problem hiding this comment.
🟡 Changes recommended
The new clear-button adornments should prevent default mouse-down behavior to avoid unintentionally blurring the search field (notably impacting mobile/keyboard UX).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a “clear search” affordance to the plugin and recipe search inputs, improving UX and accessibility, and expands automated coverage around the search-bar behaviors.
Changes:
- Add
ClearIconend-adornments toPluginSearchBarandRecipeSearchBarwhen the search text is non-empty (witharia-label="Clear search input"). - Add a new unit test suite for
PluginSearchBarand add a clear-button test case forRecipeSearchBar. - Increase timeout for a slow
PluginListintegration test that renders from realreport.jsondata.
File summaries
| File | Description |
|---|---|
| src/components/pluginList/PluginSearchBar.tsx | Adds a clear-button end adornment to reset the search input. |
| src/components/recipeList/RecipeSearchBar.tsx | Adds a clear-button end adornment to reset the search input. |
| tests/components/pluginList/PluginSearchBar.test.tsx | New unit tests covering placeholder, counts, filter chip behavior, and clear button behavior. |
| tests/components/recipeList/RecipeSearchBar.test.tsx | Adds a unit test for the new clear button behavior. |
| tests/pages/PluginList.integration.test.tsx | Adds a longer per-test timeout for real-data rendering stability. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
I have updated both Thank you for the review suggestion! |
There was a problem hiding this comment.
🔵 Needs a closer look
Keyboard activation can lose focus in both clear buttons, and empty-search regression assertions are needed.
Review details
Suppressed comments (6)
src/components/pluginList/PluginSearchBar.tsx:45
- When this control is activated with Enter or Space,
onClickclearssearchand this conditional unmounts the currently focused button. TheonMouseDownguard only covers pointer activation, so keyboard users can lose focus from the search field; move focus back to the input after clearing (and cover that keyboard path) so they can continue typing.
endAdornment: search ? (
<InputAdornment position="end">
<IconButton
size="small"
onClick={() => onSearchChange('')}
src/components/pluginList/PluginSearchBar.tsx:41
- The new
search ? ... : nullbranch is only tested for the truthy case. Add a regression assertion withsearch=""thatClear search inputis absent; otherwise a future change could leave a clear button visible for empty searches without failing this suite.
endAdornment: search ? (
src/components/recipeList/RecipeSearchBar.tsx:41
- When this control is activated with Enter or Space,
onClickclearssearchand this conditional unmounts the currently focused button. TheonMouseDownguard only covers pointer activation, so keyboard users can lose focus from the search field; move focus back to the input after clearing (and cover that keyboard path) so they can continue typing.
endAdornment: search ? (
<InputAdornment position="end">
<IconButton
size="small"
onClick={() => onSearchChange('')}
src/components/recipeList/RecipeSearchBar.tsx:37
- The new
search ? ... : nullbranch is only tested for the truthy case. Add a regression assertion withsearch=""thatClear search inputis absent; otherwise a future change could leave a clear button visible for empty searches without failing this suite.
endAdornment: search ? (
tests/components/pluginList/PluginSearchBar.test.tsx:96
- This test covers only the truthy
searchbranch. The existing empty-search cases do not assert that the clear button is absent, so an always-rendered clear adornment could regress without failing the suite; add aqueryByRole('button', { name: 'Clear search input' })null assertion for an empty search.
it('renders search clear button when search is non-empty and triggers onSearchChange("") when clicked', () => {
const onSearchChange = vi.fn();
render(
<PluginSearchBar
search="git"
tests/components/recipeList/RecipeSearchBar.test.tsx:86
- This test covers only the truthy
searchbranch. The existing empty-search cases do not assert that the clear button is absent, so an always-rendered clear adornment could regress without failing the suite; add aqueryByRole('button', { name: 'Clear search input' })null assertion for an empty search.
it('renders search clear button when search is non-empty and triggers onSearchChange("") when clicked', () => {
const onSearchChange = vi.fn();
render(
<RecipeSearchBar
search="Setup"
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
PratikMane0112
left a comment
There was a problem hiding this comment.
One comment otherwise LGTM
|
Hi @PratikMane0112 and @jonesbusy, Sorry for the delay, and thank you for your patience! I've updated the PR in commit
The full test suite (325 tests), lint, formatting, and TypeScript/Vite build checks passed locally. Could a maintainer please approve the pending workflow so CI can run? I'm happy to address any further feedback. Thank you! |
|
@PratikMane0112 would you like to give this another review? |
PratikMane0112
left a comment
There was a problem hiding this comment.
LGTM, @hrithik577 thanks :)



Description
Added interactive clear (
ClearIcon) end adornments to bothPluginSearchBarandRecipeSearchBartext inputs when search text is non-empty. Addedaria-label="Clear search input"for screen-reader accessibility.Changes
src/components/pluginList/PluginSearchBar.tsx: Added clear button endAdornment.src/components/recipeList/RecipeSearchBar.tsx: Added clear button endAdornment.tests/components/pluginList/PluginSearchBar.test.tsx: Added unit test suite for PluginSearchBar.tests/components/recipeList/RecipeSearchBar.test.tsx: Added search clear test case.Verification
npm test): Passed (33 test files, 311 tests passed).npm run lint): Passed.npm run format:check): Passed.npm run build): Passed.