-
-
Notifications
You must be signed in to change notification settings - Fork 363
feat(TabItem): support ErrorLogger catch exception #6139
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
This reverts commit d841341.
Reviewer's GuideThis PR enhances TabItem error handling by wrapping its content in ErrorLogger configured via the parent Layout, removes obsolete exception-handling code from the Tab component, refactors ErrorLogger for parameter-driven configuration, adds a new ShowExceptionDialog extension to DialogService, and updates unit tests to cover these exception scenarios. Sequence Diagram for TabItem Error Handling with ErrorLoggersequenceDiagram
actor User
participant TabItemContent
participant ErrorLogger
participant "BootstrapBlazorErrorBoundary" as BBEB
User->>TabItemContent: Interact with Tab Content (causes error)
TabItemContent->>ErrorLogger: Render Content (wrapped in ErrorLogger)
activate ErrorLogger
ErrorLogger->>BBEB: Render ChildContent (via BBEB)
activate BBEB
%% Exception occurs in ChildContent, BBEB catches it
BBEB->>ErrorLogger: OnErrorHandleAsync(Exception) // BBEB calls ErrorLogger's handler
activate ErrorLogger
%% ErrorLogger performs internal logic (e.g., logging)
ErrorLogger-->>BBEB: Processing complete
deactivate ErrorLogger
alt if BBEB.ShowToast is true (configured by Layout via ErrorLogger)
BBEB-->>User: Show Toast Notification
end
BBEB-->>User: Show Error UI (ErrorContent)
deactivate BBEB
ErrorLogger-->>TabItemContent: Return
deactivate ErrorLogger
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.
Hey @ArgoZhang - I've reviewed your changes and they look great!
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 #6139 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 703 703
Lines 31069 31084 +15
Branches 4397 4397
=========================================
+ Hits 31069 31084 +15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@sourcery-ai review |
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 and found some issues that need to be addressed.
Blocking issues:
- Avoid using MarkupString with untrusted content (link)
Here's what I looked at during the review
- 🟡 General issues: 1 issue 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.
Link issues
fixes #6138
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Delegate exception handling in Tab items to the ErrorLogger component and provide a new extension to display exceptions in dialogs.
New Features:
Enhancements:
Tests: