fix(Cascade): display incorrect when set Items async #6954
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
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR fixes the erroneous display of the Cascader component when its Items are set asynchronously by introducing an early-exit guard for empty Items in OnParametersSet (which resets the internal value state) and updating the unit test to verify that behavior. Sequence diagram for async Items update in Cascader componentsequenceDiagram
participant "Parent Component"
participant "Cascader"
"Parent Component"->>"Cascader": Set Items asynchronously
activate "Cascader"
"Cascader"->>"Cascader": OnParametersSet()
alt Items is empty
"Cascader"->>"Cascader": Reset _lastValue and Value
"Cascader"-->>"Parent Component": Early exit (no display)
else Items is not empty
"Cascader"->>"Cascader": Continue normal processing
end
deactivate "Cascader"
Class diagram for updated Cascader component state handlingclassDiagram
class Cascader {
- string _lastValue
- object Value
- IEnumerable<Item> Items
OnParametersSet()
}
Cascader : OnParametersSet()
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where the Cascader component displays incorrectly when Items are set asynchronously. The fix ensures that when no Items are provided, the component properly resets its internal state and Value property.
Key changes:
- Modified OnParametersSet to check if Items is empty and reset state accordingly
- Updated test to verify behavior when Items are initially absent then provided
- Bumped version to 9.11.3-beta05
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Cascader/Cascader.razor.cs | Added early return logic to reset Value and internal state when Items collection is empty |
| test/UnitTest/Components/CascaderTest.cs | Enhanced test to verify Value behavior with and without Items data source |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Version bump to 9.11.3-beta05 |
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}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The new test doesn’t assert the behavior after setting Items and Value asynchronously—please add an assertion to verify that the component’s Value updates to "3" as expected.
- The inline test comment is in Chinese—consider translating it to English to keep the codebase language consistent.
- In OnParametersSet, the early return when Items is empty may skip other initializations; consider restructuring the guard clause so you only reset state but still run any shared setup logic needed for async loading.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new test doesn’t assert the behavior after setting Items and Value asynchronously—please add an assertion to verify that the component’s Value updates to "3" as expected.
- The inline test comment is in Chinese—consider translating it to English to keep the codebase language consistent.
- In OnParametersSet, the early return when Items is empty may skip other initializations; consider restructuring the guard clause so you only reset state but still run any shared setup logic needed for async loading.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Cascader/Cascader.razor.cs:142` </location>
<code_context>
+ if (!Items.Any())
+ {
+ _lastValue = string.Empty;
+ Value = default;
+ return;
+ }
</code_context>
<issue_to_address>
**question:** Setting Value to default may not be appropriate for all value types.
Ensure that assigning default to Value is safe for all possible Value types, especially reference types or complex structs.
</issue_to_address>
### Comment 2
<location> `test/UnitTest/Components/CascaderTest.cs:180-186` </location>
<code_context>
+ // 未提供数据源时 Value 赋值无效
+ Assert.Null(cut.Instance.Value);
+
+ cut.SetParametersAndRender(pb =>
+ {
+ pb.Add(a => a.Items, items);
+ pb.Add(a => a.Value, "3");
+ pb.Add(a => a.ShowFullLevels, false);
+ });
}
</code_context>
<issue_to_address>
**suggestion (testing):** Missing assertion for Value after setting Items and Value asynchronously.
Add an assertion to check that cut.Instance.Value is set to "3" after setting Items and Value asynchronously.
```suggestion
cut.SetParametersAndRender(pb =>
{
pb.Add(a => a.Items, items);
pb.Add(a => a.Value, "3");
pb.Add(a => a.ShowFullLevels, false);
});
// 验证异步设置 Items 和 Value 后 Value 被正确赋值为 "3"
Assert.Equal("3", cut.Instance.Value);
}
```
</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 #6954 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 740 740
Lines 31891 31896 +5
Branches 4472 4473 +1
=========================================
+ Hits 31891 31896 +5
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
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 #6953 #6915
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Fix Cascader to correctly handle async Items assignment by resetting its value when no items are provided and update tests to cover this behavior
Bug Fixes:
Enhancements:
Tests: