feat(BootstrapBlazorOptions): add ShowErrorLoggerToast parameter #6117
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 GuideThis PR introduces a new ShowErrorLoggerToast option to BootstrapBlazorOptions and refactors the BootstrapBlazorRoot component to make its ShowToast parameter nullable and default to the new option, updating related private value resolution and tests accordingly. Sequence Diagram for ShowToast Logic in BootstrapBlazorRootsequenceDiagram
participant BBR as BootstrapBlazorRoot
participant Opts as BootstrapBlazorOptions
BBR->>BBR: Initialize / Resolve _showToast
activate BBR
BBR->>BBR: Read ShowToast parameter value
alt ShowToast parameter has a value
BBR->>BBR: _showToast = ShowToast parameter value
else ShowToast parameter is null
BBR->>Opts: Get Options.CurrentValue.ShowErrorLoggerToast
activate Opts
Opts-->>BBR: Return ShowErrorLoggerToast value
deactivate Opts
BBR->>BBR: _showToast = Options.ShowErrorLoggerToast value
end
BBR->>BBR: Use _showToast for ErrorLogger component's ShowToast input
deactivate BBR
Class Diagram for BootstrapBlazorOptions and BootstrapBlazorRoot ChangesclassDiagram
class BootstrapBlazorOptions {
+bool ShowErrorLoggerToast
}
class BootstrapBlazorRoot {
+bool? ShowToast
+bool? EnableErrorLogger
-bool _showToast
-bool _enableErrorLoggerValue
}
BootstrapBlazorRoot ..> BootstrapBlazorOptions : uses
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
Adds a new configuration flag for showing error-toasts and updates the root component to support an optional per-component override.
- Introduce
ShowErrorLoggerToastinBootstrapBlazorOptions(default true) - Change
BootstrapBlazorRoot.ShowToastto nullable and fall back toShowErrorLoggerToast - Update tests to cover the new options flag
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/UnitTest/Options/BootstrapBlazorOptionsTest.cs | Set ShowErrorLoggerToast in the options test |
| src/BootstrapBlazor/Options/BootstrapBlazorOptions.cs | Add ShowErrorLoggerToast property with XML documentation |
| src/BootstrapBlazor/Components/BaseComponents/BootstrapBlazorRoot.razor.cs | Make ShowToast nullable, rename internal logic for defaulting, and add _showToast / _enableErrorLoggerValue |
| src/BootstrapBlazor/Components/BaseComponents/BootstrapBlazorRoot.razor | Bind the ErrorLogger component to the new private properties |
Comments suppressed due to low confidence (3)
src/BootstrapBlazor/Components/BaseComponents/BootstrapBlazorRoot.razor.cs:68
- [nitpick] Private computed properties should follow PascalCase and avoid underscore prefixes. Rename
_enableErrorLoggerValuetoEnableErrorLoggerValueor similar.
private bool _enableErrorLoggerValue => EnableErrorLogger ?? Options.CurrentValue.EnableErrorLogger;
src/BootstrapBlazor/Components/BaseComponents/BootstrapBlazorRoot.razor.cs:70
- [nitpick] Rename
_showToasttoShowToastValue(PascalCase, no underscore) to match .NET naming conventions for properties.
private bool _showToast => ShowToast ?? Options.CurrentValue.ShowErrorLoggerToast;
src/BootstrapBlazor/Components/BaseComponents/BootstrapBlazorRoot.razor.cs:70
- New fallback logic for
ShowToastisn’t covered by existing tests. Add unit tests for whenShowToastis null (uses option) and when it’s explicitly set.
private bool _showToast => ShowToast ?? Options.CurrentValue.ShowErrorLoggerToast;
| public bool EnableErrorLogger { get; set; } = true; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets whether to enable show toast popup when global exception capture, default is true |
There was a problem hiding this comment.
The XML comment is awkwardly phrased. Consider: "Gets or sets whether a toast popup is shown when a global exception is captured; default is true."
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6117 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 703 703
Lines 31022 31024 +2
Branches 4386 4387 +1
=========================================
+ Hits 31022 31024 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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 and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 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.
| public bool? EnableErrorLogger { get; set; } | ||
|
|
||
| private bool EnableErrorLoggerValue => EnableErrorLogger ?? Options.CurrentValue.EnableErrorLogger; | ||
| private bool _enableErrorLoggerValue => EnableErrorLogger ?? Options.CurrentValue.EnableErrorLogger; |
Contributor
There was a problem hiding this comment.
suggestion: Private property naming deviates from conventions
Properties should use PascalCase without a leading underscore. Please rename to follow this convention.
Suggested implementation:
private bool EnableErrorLoggerValue => EnableErrorLogger ?? Options.CurrentValue.EnableErrorLogger;
private bool ShowToastValue => ShowToast ?? Options.CurrentValue.ShowErrorLoggerToast;If these properties are referenced elsewhere in the file, update all usages of _enableErrorLoggerValue to EnableErrorLoggerValue and _showToast to ShowToastValue.
| private bool EnableErrorLoggerValue => EnableErrorLogger ?? Options.CurrentValue.EnableErrorLogger; | ||
| private bool _enableErrorLoggerValue => EnableErrorLogger ?? Options.CurrentValue.EnableErrorLogger; | ||
|
|
||
| private bool _showToast => ShowToast ?? Options.CurrentValue.ShowErrorLoggerToast; |
Contributor
There was a problem hiding this comment.
suggestion: Rename private property for clarity and consistency
Consider renaming to EffectiveShowToast or ShowToastValue in PascalCase for better readability and consistency.
Suggested change
| private bool _showToast => ShowToast ?? Options.CurrentValue.ShowErrorLoggerToast; | |
| private bool ShowToastValue => ShowToast ?? Options.CurrentValue.ShowErrorLoggerToast; |
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 #6116
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Introduce a ShowErrorLoggerToast option to configure the display of error logger toasts, update the ShowToast parameter to be nullable and fallback to this new option, and adjust the root component and tests accordingly.
New Features:
Enhancements:
Tests: