Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuidePopConfirmButton’s confirm-click handler is updated so that the button’s disabled state after confirmation respects the IsKeepDisabled parameter instead of always re-enabling the button; other referenced files appear to have no functional changes in this diff. Sequence diagram for PopConfirmButton confirm click behavior with IsKeepDisabledsequenceDiagram
actor User
participant PopConfirmButton
participant ConfirmCallback
User->>PopConfirmButton: ClickConfirm()
PopConfirmButton->>PopConfirmButton: OnClickConfirm()
PopConfirmButton->>ConfirmCallback: Invoke confirm callback
alt ConfirmCallback succeeds
ConfirmCallback-->>PopConfirmButton: Success
PopConfirmButton->>PopConfirmButton: IsDisabled = IsKeepDisabled
PopConfirmButton->>PopConfirmButton: IsAsyncLoading = false
PopConfirmButton->>PopConfirmButton: StateHasChanged()
else ConfirmCallback fails
ConfirmCallback-->>PopConfirmButton: Failure
PopConfirmButton->>PopConfirmButton: IsDisabled = IsKeepDisabled
PopConfirmButton->>PopConfirmButton: IsAsyncLoading = false
PopConfirmButton->>PopConfirmButton: StateHasChanged()
end
Updated class diagram for PopConfirmButton confirming disabled behaviorclassDiagram
class PopConfirmButton {
bool IsDisabled
bool IsKeepDisabled
bool IsAsyncLoading
OnClickConfirm() Task
}
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 there - I've reviewed your changes - here's some feedback:
- The assignment
IsDisabled = IsKeepDisabledappears to invert the intended behavior of theIsKeepDisabledparameter; if the goal is to keep the button disabled whenIsKeepDisabledis true, consider usingIsDisabled = IsKeepDisabled ? true : false(orIsDisabled = IsKeepDisabled || IsDisableddepending on prior state) and explicitly clarifying the logic against the component’s expected semantics.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The assignment `IsDisabled = IsKeepDisabled` appears to invert the intended behavior of the `IsKeepDisabled` parameter; if the goal is to keep the button disabled when `IsKeepDisabled` is true, consider using `IsDisabled = IsKeepDisabled ? true : false` (or `IsDisabled = IsKeepDisabled || IsDisabled` depending on prior state) and explicitly clarifying the logic against the component’s expected semantics.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 #7319 +/- ##
=======================================
Coverage 99.98% 99.98%
=======================================
Files 746 746
Lines 32561 32561
Branches 4500 4500
=======================================
Hits 32556 32556
Misses 1 1
Partials 4 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:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the IsKeepDisabled parameter was not functioning correctly in the PopConfirmButton component. Previously, after an async operation completed, the button would always become enabled again by hardcoding IsDisabled = false. Now it properly respects the IsKeepDisabled parameter, allowing buttons to remain disabled after async operations when desired.
Key Changes
- Fixed button disabled state handling to respect
IsKeepDisabledparameter after async operations - Updated documentation in both English and Chinese localization files to clarify the
IsKeepDisabledparameter usage - Version bump to 10.1.4-beta05
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Button/PopConfirmButton.razor.cs | Changed line 114 from hardcoded IsDisabled = false to IsDisabled = IsKeepDisabled, aligning behavior with other button implementations |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Version bump from 10.1.4-beta04 to 10.1.4-beta05 for the bug fix release |
| src/BootstrapBlazor.Server/Locales/zh-CN.json | Added documentation explaining the IsKeepDisabled parameter functionality in Chinese |
| src/BootstrapBlazor.Server/Locales/en-US.json | Added documentation explaining the IsKeepDisabled parameter functionality in English |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else | ||
| { | ||
| IsDisabled = false; | ||
| IsDisabled = IsKeepDisabled; |
There was a problem hiding this comment.
The IsKeepDisabled functionality lacks test coverage in PopConfirmButton. While Button has test coverage for this feature (ButtonTest.cs lines 223-231), PopConfirmButton does not. Consider adding a test case to verify that when IsAsync and IsKeepDisabled are both true, the button remains disabled after the async operation completes.
This was referenced Dec 22, 2025
This was referenced Jan 7, 2026
This was referenced Jan 21, 2026
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
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 #7318
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Bug Fixes: