fix(SelectCity): reset Value after update IsMultiple parameter #6942
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR replaces two-way binding on the IsMultiple switch in the SelectCities sample with an explicit OnValueChanged handler that clears the existing selection and updates the IsMultiple flag, ensuring the component’s value resets when the mode toggles. Sequence diagram for IsMultiple switch value change in SelectCitiessequenceDiagram
participant User as actor User
participant Switch
participant SelectCities
User->>Switch: Toggle IsMultiple
Switch->>SelectCities: OnValueChagned(newValue)
SelectCities->>SelectCities: _value = ""
SelectCities->>SelectCities: _isMultiple = newValue
SelectCities->>SelectCities: StateHasChanged()
SelectCities-->>Switch: Task.CompletedTask
Class diagram for updated SelectCities componentclassDiagram
class SelectCities {
- bool _isDisabled
- bool _autoClose
- string _value
- bool _isMultiple
+ Task OnValueChagned(bool v)
}
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 and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor.Server/Components/Samples/SelectCities.razor.cs:23` </location>
<code_context>
private bool _autoClose = true;
+
+ private Task OnValueChagned(bool v)
+ {
+ _value = "";
</code_context>
<issue_to_address>
**suggestion (typo):** Typo in method name: 'OnValueChagned' should be 'OnValueChanged'.
Please rename the method to 'OnValueChanged' for clarity and consistency.
Suggested implementation:
```csharp
private Task OnValueChanged(bool v)
{
_value = "";
_isMultiple = v;
StateHasChanged();
return Task.CompletedTask;
}
}
```
If this method is referenced elsewhere in the file or project (e.g., in the .razor markup or other C# code), those references should also be updated to use `OnValueChanged` instead of `OnValueChagned`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| private bool _autoClose = true; | ||
|
|
||
| private Task OnValueChagned(bool v) |
There was a problem hiding this comment.
suggestion (typo): Typo in method name: 'OnValueChagned' should be 'OnValueChanged'.
Please rename the method to 'OnValueChanged' for clarity and consistency.
Suggested implementation:
private Task OnValueChanged(bool v)
{
_value = "";
_isMultiple = v;
StateHasChanged();
return Task.CompletedTask;
}
}If this method is referenced elsewhere in the file or project (e.g., in the .razor markup or other C# code), those references should also be updated to use OnValueChanged instead of OnValueChagned.
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that when the IsMultiple toggle changes in the SelectCities sample, the current selection value is reset to avoid inconsistent state between single and multi-select modes.
- Add an event handler to reset the selection when IsMultiple changes
- Replace two-way binding with explicit Value and OnValueChanged on the Switch
- Bump BootstrapBlazor.Region package from 9.0.5 to 9.0.6
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/BootstrapBlazor.Server/Components/Samples/SelectCities.razor.cs | Adds a handler to clear the current value and update _isMultiple when the switch changes |
| src/BootstrapBlazor.Server/Components/Samples/SelectCities.razor | Switch updated to use Value and OnValueChanged to trigger the handler |
| src/BootstrapBlazor.Server/BootstrapBlazor.Server.csproj | Updates BootstrapBlazor.Region to 9.0.6 to align with latest patch |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| private bool _autoClose = true; | ||
|
|
||
| private Task OnValueChagned(bool v) |
There was a problem hiding this comment.
Typo in method name: rename OnValueChagned to OnValueChanged and update call sites accordingly.
| private Task OnValueChagned(bool v) | |
| private Task OnValueChanged(bool v) |
| <BootstrapInputGroup> | ||
| <BootstrapInputGroupLabel DisplayText="IsMultiple"></BootstrapInputGroupLabel> | ||
| <Switch @bind-Value="@_isMultiple"></Switch> | ||
| <Switch Value="@_isMultiple" OnValueChanged="OnValueChagned"></Switch> |
There was a problem hiding this comment.
Typo in handler reference: update to OnValueChanged to match the corrected method name.
| <Switch Value="@_isMultiple" OnValueChanged="OnValueChagned"></Switch> | |
| <Switch Value="@_isMultiple" OnValueChanged="OnValueChanged"></Switch> |
| { | ||
| _value = ""; | ||
| _isMultiple = v; | ||
| StateHasChanged(); |
There was a problem hiding this comment.
Calling StateHasChanged inside an event callback is generally unnecessary because Blazor re-renders after handling the event; consider removing this call.
| StateHasChanged(); |
| private Task OnValueChagned(bool v) | ||
| { | ||
| _value = ""; | ||
| _isMultiple = v; | ||
| StateHasChanged(); | ||
| return Task.CompletedTask; |
There was a problem hiding this comment.
Since the handler does not perform asynchronous work, prefer a void-returning signature to avoid allocating Task.CompletedTask and simplify the method; e.g., change to 'private void OnValueChanged(bool isMultiple)'.
| private Task OnValueChagned(bool v) | |
| { | |
| _value = ""; | |
| _isMultiple = v; | |
| StateHasChanged(); | |
| return Task.CompletedTask; | |
| private void OnValueChagned(bool v) | |
| { | |
| _value = ""; | |
| _isMultiple = v; | |
| StateHasChanged(); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6942 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 740 740
Lines 31875 31875
Branches 4469 4469
=========================================
Hits 31875 31875
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:
|
Link issues
fixes #6941
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Reset the selected city value when toggling the IsMultiple option in the SelectCities sample and replace two-way binding with an explicit event callback.
Bug Fixes:
Enhancements: