fix(Button): add Task yield prevent block thread#7303
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures the async button click handler yields control back to the UI thread before executing the click handler to avoid blocking the UI when using async loading/disabled state. Sequence diagram for async button click with Task_YieldsequenceDiagram
actor User
participant UIThread
participant Button
participant ClickHandler
User->>UIThread: Clicks button
UIThread->>Button: OnClickButton()
Button->>Button: IsAsyncLoading = true
Button->>Button: IsDisabled = true
Button->>UIThread: await Task.Yield()
UIThread-->>User: UI re-renders with loading/disabled
UIThread->>ClickHandler: HandlerClick()
ClickHandler-->>UIThread: Complete
UIThread->>Button: Update IsAsyncLoading/IsDisabled if needed
UIThread-->>User: UI reflects final state
Class diagram for Button component OnClickButton changeclassDiagram
class Button {
bool IsAsyncLoading
bool IsDisabled
Task OnClickButton()
Task HandlerClick()
}
Button : OnClickButton() sets IsAsyncLoading = true
Button : OnClickButton() sets IsDisabled = true
Button : OnClickButton() awaits Task.Yield()
Button : OnClickButton() awaits HandlerClick()
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:
- Consider replacing
await Task.Yield();with a more explicit UI update mechanism (e.g.,await InvokeAsync(StateHasChanged);) or at least adding a brief comment explaining why yielding is required here, so the intent is clear and future maintainers don’t inadvertently remove or alter this line.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider replacing `await Task.Yield();` with a more explicit UI update mechanism (e.g., `await InvokeAsync(StateHasChanged);`) or at least adding a brief comment explaining why yielding is required here, so the intent is clear and future maintainers don’t inadvertently remove or alter this line.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 #7303 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 745 745
Lines 32629 32630 +1
Branches 4520 4521 +1
=========================================
+ Hits 32629 32630 +1
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:
|
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #7302 by preventing the button component from blocking the UI thread during async operations. The fix adds await Task.Yield() after setting the button's loading and disabled states to ensure the UI thread is not blocked before executing the click handler.
Key Changes
- Added
await Task.Yield()in theOnClickButtonmethod to yield control back to the UI thread after setting async loading states - Removed BOM (Byte Order Mark) character from the beginning of the file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -46,6 +46,7 @@ protected virtual async Task OnClickButton() | |||
| { | |||
| IsAsyncLoading = true; | |||
| IsDisabled = true; | |||
There was a problem hiding this comment.
After setting IsAsyncLoading and IsDisabled to true, StateHasChanged() should be called to immediately update the UI before yielding control. This ensures the loading indicator and disabled state are rendered before the async operation continues. The PopConfirmButton component follows this pattern by calling StateHasChanged() immediately after setting these properties (see PopConfirmButton.razor.cs lines 100-101).
| IsDisabled = true; | |
| IsDisabled = true; | |
| StateHasChanged(); |
| { | ||
| IsAsyncLoading = true; | ||
| IsDisabled = true; | ||
| await Task.Yield(); |
There was a problem hiding this comment.
Consider applying this same pattern to other button components (LinkButton, ToggleButton) which also set IsAsyncLoading and IsDisabled when IsAsync is true but don't call Task.Yield(). This inconsistency means only the base Button component will prevent blocking the UI thread, while other async button types may still experience the original issue.
| { | ||
| IsAsyncLoading = true; | ||
| IsDisabled = true; | ||
| await Task.Yield(); |
There was a problem hiding this comment.
The addition of Task.Yield() should have test coverage to verify that it properly prevents UI thread blocking. Consider adding a test case in ButtonTest.cs that verifies the button's loading state is rendered before the async operation completes, ensuring the Task.Yield() behavior is working as intended.
Link issues
fixes #7302
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Bug Fixes: