feat: add OnLoading and OnError RenderFragments
#9
Conversation
WalkthroughThis update introduces explicit handling for loading and error states when retrieving browser time zone information in Blazor components. New parameters and conditional rendering logic are added to display custom UI fragments during these states. Logging and exception handling are enhanced, and comprehensive tests are added to verify the new behaviors and service state transitions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LocalTime/LocalTimeForm/LocalTimeZone Component
participant BlazorLocalTimeProvider
participant JSInterop
participant LocalTimeService
User->>LocalTime/LocalTimeForm/LocalTimeZone Component: Render
LocalTime/LocalTimeForm/LocalTimeZone Component->>LocalTimeService: Check IsSuccessLoadBrowserTimeZone
alt IsSuccessLoadBrowserTimeZone == null
LocalTime/LocalTimeForm/LocalTimeZone Component->>User: Render OnLoading fragment
else IsSuccessLoadBrowserTimeZone == false
LocalTime/LocalTimeForm/LocalTimeZone Component->>User: Render OnError fragment
else IsSuccessLoadBrowserTimeZone == true
LocalTime/LocalTimeForm/LocalTimeZone Component->>User: Render normal content
end
Note over BlazorLocalTimeProvider: On first render
BlazorLocalTimeProvider->>JSInterop: Get browser time zone
alt Success
JSInterop-->>BlazorLocalTimeProvider: Time zone string
BlazorLocalTimeProvider->>LocalTimeService: SetBrowserTimeZoneInfo(timeZone)
LocalTimeService->>LocalTimeService: IsSuccessLoadBrowserTimeZone = true
else JSDisconnectedException
BlazorLocalTimeProvider->>Logger: Log debug (disconnected)
BlazorLocalTimeProvider->>LocalTimeService: SetBrowserTimeZoneInfo(null)
LocalTimeService->>LocalTimeService: IsSuccessLoadBrowserTimeZone = false
else JSException
BlazorLocalTimeProvider->>Logger: Log warning (JS error)
BlazorLocalTimeProvider->>LocalTimeService: SetBrowserTimeZoneInfo(null)
LocalTimeService->>LocalTimeService: IsSuccessLoadBrowserTimeZone = false
end
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/BlazorLocalTime/Components/LocalTimeZone.razor (1)
7-15: Fix minor syntax issue and approve the logic.The loading and error state handling follows the same pattern as
LocalTime.razor, which is good for consistency. However, there's a missing space in the condition.-else if (OnError != null && LocalTimeService.IsSuccessLoadBrowserTimeZone ==false) +else if (OnError != null && LocalTimeService.IsSuccessLoadBrowserTimeZone == false)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/BlazorLocalTime/Components/BlazorLocalTimeProvider.razor.cs(2 hunks)src/BlazorLocalTime/Components/LocalTime.razor(1 hunks)src/BlazorLocalTime/Components/LocalTime.razor.cs(1 hunks)src/BlazorLocalTime/Components/LocalTimeForm.razor(1 hunks)src/BlazorLocalTime/Components/LocalTimeForm.razor.cs(1 hunks)src/BlazorLocalTime/Components/LocalTimeText.razor(1 hunks)src/BlazorLocalTime/Components/LocalTimeText.razor.cs(1 hunks)src/BlazorLocalTime/Components/LocalTimeZone.razor(1 hunks)src/BlazorLocalTime/Components/LocalTimeZone.razor.cs(1 hunks)src/BlazorLocalTime/LocalTimeService.cs(2 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt(3 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt(3 hunks)tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.cs(1 hunks)tests/BlazorLocalTimeTest/Component/LocalTimeTotal2.razor(1 hunks)tests/BlazorLocalTimeTest/LocalTimeMockService.cs(1 hunks)tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs(1 hunks)tests/BlazorLocalTimeTest/LocalTimeTest.razor(1 hunks)tests/BlazorLocalTimeTest/LocalTimeTotalTest.razor(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/BlazorLocalTime/Components/LocalTimeText.razor.cs (1)
src/BlazorLocalTime/LocalTimeService.cs (1)
LocalTimeService(118-175)
tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs (2)
tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.cs (4)
Fact(18-29)Fact(31-42)Fact(44-55)Fact(57-65)src/BlazorLocalTime/LocalTimeService.cs (2)
LocalTimeService(118-175)TimeZoneInfo(95-112)
🔇 Additional comments (34)
src/BlazorLocalTime/Components/LocalTimeText.razor.cs (1)
54-55: LGTM! Good separation of concerns.Moving the timezone availability check from the formatting logic to the rendering logic improves code organization. The
FormattedValueproperty now focuses solely on formatting when a value exists.tests/BlazorLocalTimeTest/LocalTimeMockService.cs (1)
17-24: LGTM! Clean test helper implementation.The
LocalTimeEmptyMockServiceproperly simulates the state where browser timezone information is not available, which is essential for testing loading states.src/BlazorLocalTime/Components/LocalTimeText.razor (1)
3-13: LGTM! Proper rendering layer guard condition.The outer condition correctly ensures the component only renders when both a value exists and timezone information is available. This complements the simplified
FormattedValueproperty logic.src/BlazorLocalTime/Components/LocalTime.razor (1)
7-14: LGTM! Proper loading and error state handling.The conditional rendering logic correctly uses
IsSuccessLoadBrowserTimeZoneto determine when to show loading (null) and error (false) states. The cascadingelse ifstructure ensures proper exclusive rendering.src/BlazorLocalTime/Components/LocalTimeZone.razor.cs (2)
20-23: Breaking change: Parameter renamed fromUnknownContenttoOnLoading.This is a breaking change that will require consumers to update their code. The new name
OnLoadingis more descriptive and aligns better with the newOnErrorparameter, but ensure this breaking change is properly documented in release notes.The parameter documentation is clear and follows consistent patterns.
25-29: Well-implemented error state parameter.The
OnErrorparameter follows the same pattern asOnLoadingwith consistent documentation and nullable RenderFragment type. This provides good symmetry for handling both loading and error states.src/BlazorLocalTime/Components/LocalTime.razor.cs (1)
19-29: Consistent implementation of loading and error state parameters.The
OnLoadingandOnErrorparameters are implemented consistently with the same pattern used inLocalTimeZone.razor.cs. The documentation is clear and follows the established conventions.src/BlazorLocalTime/Components/LocalTimeForm.razor (1)
8-15: Well-structured conditional rendering for loading and error states.The conditional rendering logic follows a clear hierarchy:
- Show content when timezone is available
- Show loading when state is unknown (
null)- Show error when loading explicitly failed (
false)The null checks for optional RenderFragments are properly implemented. This provides good user feedback during different phases of timezone loading.
tests/BlazorLocalTimeTest/Component/LocalTimeTotal2.razor (1)
1-16: Effective test component demonstrating new functionality.This test component effectively demonstrates the usage of the new
OnLoadingandOnErrorparameters:
- Proper service injection for required dependencies
- Uses current UTC time from
TimeProvider.GetUtcNow()- Simple, clear render fragments for loading and error states
- Includes
BlazorLocalTimeProviderfor complete functionalityThe component serves as a good example of how to use the new features.
src/BlazorLocalTime/Components/LocalTimeForm.razor.cs (1)
20-30: Consistent parameter implementation across component family.The
OnLoadingandOnErrorparameters are implemented with the same pattern used in the other components (LocalTimeandLocalTimeZone). The documentation is consistent and the nullable RenderFragment typing is appropriate.This maintains consistency across the entire component family, which is excellent for developer experience.
tests/BlazorLocalTimeTest/LocalTimeTest.razor (2)
72-92: LGTM! Well-structured loading state test.The test correctly sets up an empty mock service to simulate the loading state and verifies that the OnLoading fragment is rendered as expected.
94-113: LGTM! Good test coverage for successful loading scenario.The test properly verifies that when the service has time zone information available, the component bypasses the loading state and renders the formatted time directly.
tests/BlazorLocalTimeTest/LocalTimeTotalTest.razor (2)
30-40: LGTM! Comprehensive success scenario test.The test properly sets up the mock time provider and JavaScript interop, then verifies the expected time conversion output.
42-52: LGTM! Excellent error handling test.The test effectively simulates a JavaScript exception scenario and verifies that the error state is properly handled and displayed.
src/BlazorLocalTime/Components/BlazorLocalTimeProvider.razor.cs (2)
21-22: LGTM! Proper logging dependency injection.The logger is correctly injected and will provide valuable diagnostic information for time zone loading issues.
29-59: Excellent error handling and logging implementation.The implementation demonstrates several best practices:
- Proper exception handling: Specific handling for
JSDisconnectedExceptionandJSExceptionwith appropriate log levels- Robust state management: The
finallyblock ensures the service state is always updated regardless of success or failure- Clear diagnostic messages: Log messages provide actionable information about potential causes
The variable scoping and error handling logic are well-designed.
tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs (3)
8-15: LGTM! Good initial state verification.The test correctly verifies that a new
LocalTimeServiceinstance starts in the expected initial state withIsSuccessLoadBrowserTimeZoneas null and no time zone info available.
17-27: LGTM! Comprehensive time zone setting test.The test properly verifies that setting the browser time zone info makes the service available and accessible through the interface methods.
29-41: LGTM! Well-designed time zone conversion test.The test effectively validates the time zone conversion logic using a concrete example (UTC to Asia/Tokyo) and verifies both the converted hour and the offset values.
tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt (3)
32-34: LGTM! Consistent API design for LocalTime component.The new
OnErrorandOnLoadingparameters follow proper Blazor component conventions with nullableRenderFragmenttypes and correct parameter attributes.
59-61: LGTM! Consistent API design for LocalTimeForm component.The parameters maintain consistency with the
LocalTimecomponent, providing a uniform API across the component family.
89-91: LGTM! Consistent API design for LocalTimeZone component.The parameters complete the consistent API design across all three time-related components, enabling uniform loading and error state handling.
src/BlazorLocalTime/LocalTimeService.cs (4)
38-43: Well-designed tri-state tracking property.The nullable boolean design elegantly handles three distinct states: uninitialized (null), success (true), and failure (false). The comprehensive documentation clearly explains the property's lifecycle and usage.
49-49: Good defensive programming with nullable parameter.The method signature change to accept
TimeZoneInfo?enables proper handling of null values, which aligns with the error handling scenarios where time zone retrieval might fail.
159-159: Proper initialization of the success tracking property.The property is correctly initialized to null, indicating the uninitialized state before any time zone loading attempt.
162-174: Flag Setting Verified in BlazorLocalTimeProviderThe
IsSuccessLoadBrowserTimeZoneproperty is indeed set based on the outcome of the JS interop call in BlazorLocalTimeProvider.razor.cs (within thefinallyblock):• File: src/BlazorLocalTime/Components/BlazorLocalTimeProvider.razor.cs
finally { LocalTimeService.IsSuccessLoadBrowserTimeZone = (timeZone != null); LocalTimeService.SetBrowserTimeZoneInfo(timeZone); }No further changes are needed here.
tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt (3)
32-34: Excellent API consistency across components.The
OnErrorandOnLoadingparameters are consistently defined across all relevant components (LocalTime,LocalTimeForm<T>,LocalTimeZone) with proper nullableRenderFragmenttypes and[Parameter]attributes.
59-61: Consistent parameter design in LocalTimeForm.The parameter definitions match the pattern established in other components, ensuring a uniform developer experience.
89-91: Proper parameter design in LocalTimeZone component.The
OnErrorandOnLoadingparameters are correctly defined, maintaining consistency with the other components in the library.tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.cs (5)
10-16: Well-structured test class with proper setup.The test class properly inherits from
TestContextand sets up the necessary services includingBlazorLocalTimeServiceand logging. The constructor approach ensures consistent test setup.
18-29: Comprehensive success scenario test.The test properly validates the happy path by checking that:
- Time zone information is available
- Success flag is set to true
- Correct time zone ("Asia/Tokyo") is loaded
The use of nullable boolean assertions (
?.ShouldBeTrue()) is appropriate for the tri-state property.
31-42: Proper error handling test for JSDisconnectedException.This test correctly simulates a JavaScript disconnection scenario and validates that:
- Time zone information is not available
- Success flag is set to false
The test setup using
JSInterop.SetupModuleandSetExceptionis appropriate for testing JS interop failures.
44-55: Good coverage for generic JSException scenarios.This test covers the broader category of JavaScript exceptions (like unsupported browser APIs) and validates the same error state conditions as the disconnection test.
57-65: Important initial state verification.This test validates the initial state before any time zone loading attempt, ensuring the property starts as null. This is crucial for the tri-state logic to work correctly.
…ine loading state management
…isplaying local time and input forms
…nd add missing return types
arika0093
force-pushed
the
feat/add-loading-and-failed-fragment
branch
from
d662f43 to
a03cef3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/API.md (3)
15-15: Spell-out the tri-state semantics ofIsSuccessLoadBrowserTimeZone.Relying on the parenthetical “
null = loading, true = success, false = error” can be easy to skim past. Consider turning this into a bulleted list or a short sentence (“null– loading,true– loaded,false– failed”) to make the meaning unambiguous at a glance.
42-43: Improve discoverability of the new fragment parameters.The lead sentence introduces loading/error-state support but the parameter table follows later. Adding “(see parameters below)” or moving the sentence directly above the table would help readers connect the dots more quickly.
48-49: Maintain table-column consistency across component docs.
LocalTimeTextlists a “Default” column, but the newOnLoading/OnErrorrows omit it. Adding a “Default” column (valuenull) keeps the documentation uniform and clearly communicates the default behaviour.Also applies to: 57-58, 68-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
README.md(3 hunks)docs/API.md(2 hunks)example/BlazorLocalTimeSample/Pages/Home.razor(2 hunks)src/BlazorLocalTime/Components/BlazorLocalTimeProvider.razor.cs(2 hunks)src/BlazorLocalTime/Components/LocalTime.razor(1 hunks)src/BlazorLocalTime/Components/LocalTime.razor.cs(1 hunks)src/BlazorLocalTime/Components/LocalTimeForm.razor(1 hunks)src/BlazorLocalTime/Components/LocalTimeForm.razor.cs(1 hunks)src/BlazorLocalTime/Components/LocalTimeText.razor(1 hunks)src/BlazorLocalTime/Components/LocalTimeText.razor.cs(1 hunks)src/BlazorLocalTime/Components/LocalTimeZone.razor(1 hunks)src/BlazorLocalTime/Components/LocalTimeZone.razor.cs(1 hunks)src/BlazorLocalTime/LocalTimeService.cs(2 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt(3 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt(3 hunks)tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.cs(1 hunks)tests/BlazorLocalTimeTest/Component/LocalTimeTotal2.razor(1 hunks)tests/BlazorLocalTimeTest/LocalTimeMockService.cs(1 hunks)tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs(1 hunks)tests/BlazorLocalTimeTest/LocalTimeTest.razor(1 hunks)tests/BlazorLocalTimeTest/LocalTimeTotalTest.razor(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- example/BlazorLocalTimeSample/Pages/Home.razor
- README.md
🚧 Files skipped from review as they are similar to previous changes (18)
- src/BlazorLocalTime/Components/LocalTimeText.razor.cs
- src/BlazorLocalTime/Components/LocalTime.razor
- src/BlazorLocalTime/Components/LocalTime.razor.cs
- src/BlazorLocalTime/Components/LocalTimeZone.razor.cs
- tests/BlazorLocalTimeTest/LocalTimeMockService.cs
- tests/BlazorLocalTimeTest/LocalTimeTotalTest.razor
- src/BlazorLocalTime/Components/LocalTimeZone.razor
- src/BlazorLocalTime/Components/LocalTimeForm.razor
- tests/BlazorLocalTimeTest/LocalTimeTest.razor
- src/BlazorLocalTime/Components/LocalTimeForm.razor.cs
- src/BlazorLocalTime/Components/LocalTimeText.razor
- tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs
- tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.cs
- tests/BlazorLocalTimeTest/Component/LocalTimeTotal2.razor
- tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt
- src/BlazorLocalTime/Components/BlazorLocalTimeProvider.razor.cs
- tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt
- src/BlazorLocalTime/LocalTimeService.cs
🧰 Additional context used
🪛 LanguageTool
docs/API.md
[duplication] ~18-~18: Possible typo: you repeated a word.
Context: ...erts UTC to local DateTime | | Method | ToLocalTime(DateTimeOffset dateTimeOffset) | DateTime | Converts DateTimeOffse...
(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~20-~20: Possible typo: you repeated a word.
Context: ...TC to local DateTimeOffset | | Method | ToLocalTimeOffset(DateTimeOffset dateTimeOffset) | DateTimeOffset | Converts DateTim...
(ENGLISH_WORD_REPEAT_RULE)
This pull request introduces enhancements to the
BlazorLocalTimelibrary, focusing on error handling, user feedback during time zone loading, and improved test coverage. Key changes include adding new render fragments for loading and error states, improving logging for exceptions, and updating theLocalTimeServicewith a success flag for time zone loading. Additionally, comprehensive tests have been added to validate these updates.Enhancements to error handling and user feedback:
BlazorLocalTimeProvider: Added logging forJSDisconnectedExceptionandJSExceptionduring time zone loading, and updated thefinallyblock to set the success flag and time zone information accordingly. (src/BlazorLocalTime/Components/BlazorLocalTimeProvider.razor.cs, src/BlazorLocalTime/Components/BlazorLocalTimeProvider.razor.csR21-R58)OnLoadingandOnErrorparameters toLocalTime,LocalTimeForm, andLocalTimeZonecomponents, providing user feedback during time zone loading and error scenarios. (src/BlazorLocalTime/Components/LocalTime.razor, [1];src/BlazorLocalTime/Components/LocalTimeForm.razor, [2];src/BlazorLocalTime/Components/LocalTimeZone.razor, [3]Updates to
LocalTimeService:IsSuccessLoadBrowserTimeZoneproperty: Introduced a nullable boolean flag to track the success of browser time zone loading. Updated theSetBrowserTimeZoneInfomethod to handle null values and set the success flag. (src/BlazorLocalTime/LocalTimeService.cs, [1] [2]Test coverage improvements:
BlazorLocalTimeProvider: Added tests to verify successful time zone loading, handling ofJSDisconnectedExceptionandJSException, and initial state behavior. (tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.cs, tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.csR1-R66)LocalTimeService: Validated initial state, time zone setting, and time zone conversion functionality. (tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs, tests/BlazorLocalTimeTest/LocalTimeServiceTest.csR1-R42)Public API updates:
PublicApiCheckTest: Reflected newOnLoadingandOnErrorparameters in the approved API files for .NET 8 and .NET 9 compatibility. (tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt, [1];tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt, [2]Summary by CodeRabbit
New Features
Bug Fixes
Tests