feat(ToggleButton): add ToggleButton component #6545
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
Contributor
Reviewer's GuideAdds a new ToggleButton component with active/inactive state management, asynchronous and event callbacks, complete with unit tests and a server sample demo, and includes a sortable Table unit test. Sequence diagram for ToggleButton click and state changesequenceDiagram
actor User
participant ToggleButton
User->>ToggleButton: Clicks button
ToggleButton->>ToggleButton: OnClickButton()
ToggleButton->>ToggleButton: HandlerClick()
ToggleButton->>ToggleButton: Toggle IsActive
alt IsActiveChanged set
ToggleButton->>IsActiveChanged: InvokeAsync(IsActive)
end
alt OnToggleAsync set
ToggleButton->>OnToggleAsync: Invoke(IsActive)
end
Class diagram for the new ToggleButton componentclassDiagram
class ToggleButton {
+Func<bool, Task>? OnToggleAsync
+bool IsActive
+EventCallback<bool> IsActiveChanged
-string? ToggleClassName
-Task OnClickButton()
-Task HandlerClick()
}
ToggleButton --|> ButtonBase
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}
Contributor
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
- The Table_Sortable test has no assertions; please validate sortable behavior to ensure the feature works as expected.
- The ToggleButton test method 'ToogleButton_Ok' has a typo in its name and does not assert the IsActiveChanged callback flag; please rename it and add the missing assertion.
- Consider adding tests to verify the ToggleButton's visual state (e.g., CSS 'active' class) and toggling back to inactive to cover full toggle behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Table_Sortable test has no assertions; please validate sortable behavior to ensure the feature works as expected.
- The ToggleButton test method 'ToogleButton_Ok' has a typo in its name and does not assert the IsActiveChanged callback flag; please rename it and add the missing assertion.
- Consider adding tests to verify the ToggleButton's visual state (e.g., CSS 'active' class) and toggling back to inactive to cover full toggle behavior.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Button/ToggleButton.razor.cs:35` </location>
<code_context>
+ .AddClass("active", IsActive)
+ .Build();
+
+ private async Task OnClickButton()
+ {
+ if (IsAsync)
</code_context>
<issue_to_address>
Consider refactoring by merging the two methods, extracting repeated loading/disabled logic, and consolidating callback invocations into helpers.
Here’s one way to collapse both methods into a single, linear flow, pull‐out the repeated “loading/disabled” toggle into its own helper, and share the callback invocation logic:
```csharp
private async Task OnClickButton()
{
var nextState = !IsActive;
if (IsAsync)
SetLoading(true);
IsActive = nextState;
await InvokeAllCallbacks(nextState);
if (IsAsync)
SetLoading(false);
}
private void SetLoading(bool isLoading)
{
IsAsyncLoading = isLoading;
IsDisabled = isLoading
? true
: IsKeepDisabled;
// optionally StateHasChanged() here if you need an immediate re-render
}
private async Task InvokeAllCallbacks(bool newState)
{
if (OnClickWithoutRender != null)
{
if (!IsAsync)
IsNotRender = true;
await OnClickWithoutRender();
}
if (OnClick.HasDelegate)
await OnClick.InvokeAsync();
if (IsActiveChanged.HasDelegate)
await IsActiveChanged.InvokeAsync(newState);
if (OnToggleAsync != null)
await OnToggleAsync(newState);
}
```
What this does:
1. Merges `OnClickButton` + `HandlerClick` into one clear sequence.
2. Extracts the “loading/disabled” block into `SetLoading(bool)`.
3. Extracts all callback invocations into `InvokeAllCallbacks(bool)`.
You retain the exact same state transitions and callback order, but with much less nesting or duplication.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This was referenced Aug 5, 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 #6544
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Implement a new ToggleButton component with associated demo and unit tests, and introduce a Table sortable test skeleton
New Features:
Enhancements:
Tests: