feat(Search): remove OnClear callback reuse OnSearch #6761
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 584f654.
Contributor
Reviewer's GuideThis PR streamlines clear functionality by removing the separate OnClear callback in Search (merging it into OnSearch), moves clearable parameters into the common PopoverCompleteBase, refines markup and SCSS for search clear and template icons, and updates unit tests to match the revised behavior and class names. Sequence diagram for merged clear action in Search componentsequenceDiagram
participant User as actor User
participant Search as Search<TValue>
participant JS as JSInterop
User->>Search: Clicks clear button
Search->>JS: setValue(Id, "")
Search->>Search: _displayText = null
Search->>Search: OnSearchClick()
Search->>Search: (OnClear callback is obsolete, not called)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR removes the deprecated OnClear callback from the Search component and consolidates its functionality with the OnSearch method. The change simplifies the API by eliminating redundant callbacks and moves clear icon properties to the base class for better component inheritance.
- Deprecated and marked OnClear callback as obsolete in Search component
- Moved IsClearable and ClearIcon properties from individual components to PopoverCompleteBase for inheritance
- Updated styling to use logical properties and improved CSS class naming
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Search/Search.razor.cs | Deprecated OnClear callback and removed duplicate property definitions |
| src/BootstrapBlazor/Components/Search/Search.razor | Updated CSS class names and added wrapper for icon template |
| src/BootstrapBlazor/Components/Search/Search.razor.scss | Improved CSS with logical properties and updated class selectors |
| src/BootstrapBlazor/Components/AutoComplete/PopoverCompleteBase.cs | Added IsClearable and ClearIcon properties to base class |
| src/BootstrapBlazor/Components/AutoFill/AutoFill.razor.cs | Removed duplicate properties now inherited from base class |
| src/BootstrapBlazor/Components/AutoComplete/AutoComplete.razor.cs | Removed duplicate properties now inherited from base class |
| test/UnitTest/Components/SearchTest.cs | Updated test to use OnSearch instead of deprecated OnClear |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Search/Search.razor.scss:1` </location>
<code_context>
-.search.auto-complete {
+.search.auto-complete {
--bb-ac-padding-right: #{$bb-search-padding-right};
</code_context>
<issue_to_address>
**issue (bug_risk):** Addition of BOM (Byte Order Mark) may cause issues in some environments.
Some compilers or tools may not handle BOMs correctly. Remove it unless absolutely necessary.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Components/AutoComplete/PopoverCompleteBase.cs:94-103` </location>
<code_context>
[Parameter]
public bool ShowNoDataTip { get; set; } = true;
- /// <summary>
- /// Gets or sets whether the select component is clearable. Default is false.
- /// </summary>
- [Parameter]
- public bool IsClearable { get; set; }
-
- /// <summary>
- /// Gets or sets the right-side clear icon. Default is fa-solid fa-angle-up.
- /// </summary>
- [Parameter]
- [NotNull]
- public string? ClearIcon { get; set; }
-
/// <summary>
</code_context>
<issue_to_address>
**issue:** ClearIcon parameter is marked [NotNull] but is nullable.
Please update the ClearIcon property to either remove the [NotNull] attribute or make it non-nullable to ensure consistency and prevent possible runtime errors.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6761 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 739 739
Lines 31729 31718 -11
Branches 4466 4463 -3
=========================================
- Hits 31729 31718 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This was referenced Sep 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #6760
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Deprecate the OnClear callback on the Search component and consolidate clear actions into the OnSearch callback; unify clearable properties in the auto-complete base class; update styling and tests to match the new implementation.
New Features:
Bug Fixes:
Enhancements:
Tests: