-
-
Notifications
You must be signed in to change notification settings - Fork 363
fix(Table): should show the Toast when not set BootstrapBlazorOption ToastDelay #5810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's Guide by SourceryThis pull request improves the handling of delay options in various components and adds a necessary dependency in the unit tests. The delay condition checks in Sequence diagram for showing a toast messagesequenceDiagram
participant Table.razor.Toolbar
participant BootstrapBlazorOptions
participant ToastService
Table.razor.Toolbar->>BootstrapBlazorOptions: Get Options.CurrentValue.ToastDelay
alt Options.CurrentValue.ToastDelay > 0
Table.razor.Toolbar->>ToastService: Show Toast with Delay
else Options.CurrentValue.ToastDelay <= 0
Table.razor.Toolbar->>ToastService: Show Toast without Delay
end
Updated class diagram for ToastOptionclassDiagram
class ToastOption {
+string Title
+int Delay
}
note for ToastOption "Delay is only set if Options.CurrentValue.ToastDelay > 0"
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
test/UnitTest/Components/TableDialogTest.cs:19
- [nitpick] Directly modifying options.CurrentValue in the test could lead to side effects if the service container is shared. Consider configuring the options through dependency injection or ensuring test isolation to prevent interference with other tests.
var options = Context.Services.GetRequiredService<IOptionsMonitor<BootstrapBlazorOptions>>();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
Overall Comments:
- The changes to
MessageServiceandSwalServicelook like they might be needed in other similar components. - Consider adding a comment to explain why
ToastDelayis being set to 0 in the unit test.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5810 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 660 660
Lines 30117 30123 +6
Branches 4250 4251 +1
=========================================
+ Hits 30117 30123 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Link issues
fixes #5809
Summary By Copilot
This pull request includes several changes to improve the handling of delay options in various components and adds a necessary dependency in the unit tests. The most important changes include modifying the delay condition checks in
MessageServiceandSwalService, updating theGetToastOptionmethod inTable.razor.Toolbar.cs, and adding theIOptionsMonitordependency inTableDialogTest.Improvements to delay handling:
src/BootstrapBlazor/Components/Message/MessageService.cs: Changed the condition to check ifOptions.MessageDelayis greater than 0 instead of not equal to 0.src/BootstrapBlazor/Components/SweetAlert/SwalService.cs: Modified the condition to check ifoptions.CurrentValue.SwalDelayis greater than 0 instead of not equal to 0.src/BootstrapBlazor/Components/Table/Table.razor.Toolbar.cs: Updated theGetToastOptionmethod to include a check forOptions.CurrentValue.ToastDelaybeing greater than 0 before setting the delay.Unit test improvements:
test/UnitTest/Components/TableDialogTest.cs: AddedMicrosoft.Extensions.Optionsusing directive and injectedIOptionsMonitor<BootstrapBlazorOptions>to setToastDelayto 0 in theEditAsync_Oktest method. [1] [2]Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Improve delay handling in Toast, Message, and SweetAlert components by adding more precise delay condition checks
Bug Fixes:
Enhancements:
Tests: