fix(AutoComplete): support bind value clear value function #6936
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
Reviewer's GuideThis PR adds support for a clear-value function in the AutoComplete component by introducing a JS-invokable TriggerClear method, updating the JS event handling and sample usage, and adding a unit test, while also refactoring TriggerChange to a void method. Sequence diagram for AutoComplete clear value interactionsequenceDiagram
participant User as actor User
participant UI as "AutoComplete UI"
participant JS as "AutoComplete.razor.js"
participant DotNet as "AutoComplete.razor.cs"
User->>UI: Click clear icon
UI->>JS: .clear-icon click event
JS->>DotNet: invokeMethodAsync('TriggerClear')
DotNet->>DotNet: TriggerClear()
DotNet->>DotNet: TriggerFilter("")
DotNet->>DotNet: _clientValue = null
DotNet->>DotNet: CurrentValueAsString = ""
Class diagram for updated AutoComplete componentclassDiagram
class AutoComplete {
+List<string> Rows
+Task TriggerFilter(string val)
+void TriggerChange(string v)
+async Task TriggerClear()
-List<string> GetFilterItemsByValue(string val)
-string _clientValue
-string CurrentValueAsString
}
AutoComplete : [JSInvokable] TriggerClear()
AutoComplete : [JSInvokable] TriggerChange(string v)
AutoComplete : [JSInvokable] TriggerFilter(string val)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- TriggerClear doesn’t invoke the ValueChanged callback, so bound Value parameters won’t be notified of the cleared state—consider calling ValueChanged.InvokeAsync after clearing.
- For consistency with other JSInvokable methods, either have TriggerClear return a Task or rename it to TriggerClearAsync to match the async naming convention.
- Consider adding a test to verify that the clear icon only appears when IsClearable is true and that clicking it actually invokes TriggerClear.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- TriggerClear doesn’t invoke the ValueChanged callback, so bound Value parameters won’t be notified of the cleared state—consider calling ValueChanged.InvokeAsync after clearing.
- For consistency with other JSInvokable methods, either have TriggerClear return a Task or rename it to TriggerClearAsync to match the async naming convention.
- Consider adding a test to verify that the clear icon only appears when IsClearable is true and that clicking it actually invokes TriggerClear.
## Individual Comments
### Comment 1
<location> `test/UnitTest/Components/AutoCompleteTest.cs:199-208` </location>
<code_context>
Assert.Equal(2, menus.Count);
}
+ [Fact]
+ public async Task TriggerClear_Ok()
+ {
+ var val = "task1";
+ var items = new List<string>() { "task1", "Task2" };
+ var cut = Context.RenderComponent<AutoComplete>(builder =>
+ {
+ builder.Add(a => a.Items, items);
+ builder.Add(a => a.IgnoreCase, false);
+ builder.Add(a => a.Value, val);
+ builder.Add(a => a.IsClearable, true);
+ builder.Add(a => a.ValueChanged, EventCallback.Factory.Create<string?>(this, v =>
+ {
+ val = v;
+ }));
+ });
+
+ await cut.InvokeAsync(cut.Instance.TriggerClear);
+ Assert.Empty(val);
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for edge cases when clearing the value.
Please add tests for clearing when the value is already empty, when Items is empty, and when IsClearable is false to ensure all edge cases are covered.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull Request Overview
Fixes the AutoComplete component's clear value functionality to properly support bind value clearing when the clear button is clicked.
- Added a new
TriggerClearmethod that properly clears both client and server state - Updated JavaScript to call the new clear method instead of the generic filter method
- Simplified the
TriggerChangemethod by removing unnecessary async/await
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| AutoComplete.razor.cs | Added TriggerClear method and simplified TriggerChange method |
| AutoComplete.razor.js | Updated clear button handler to call TriggerClear instead of TriggerFilter |
| AutoCompleteTest.cs | Added unit test for the clear functionality |
| AutoCompletes.razor | Enabled clear button in demo for testing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6936 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 740 740
Lines 31815 31819 +4
Branches 4469 4469
=========================================
+ Hits 31815 31819 +4
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:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 #6931
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Implement clear functionality in the AutoComplete component so that clicking the clear icon resets the bound value and filter, along with test and sample updates.
New Features:
Enhancements:
Documentation:
Tests: