Conversation
WalkthroughLocalTimeForm now constructs LocalTimeFormValue with an injected ILocalTimeService. LocalTimeFormValue gained an internal ctor, stores the service, changed Date/Time/TimeSpan getters, and added ValueOrNow/DateOrToday/TimeOrNow/TimeSpanOrNow. Several csproj and test files were reformatted and new tests added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant LocalTimeForm
participant LocalTimeFormValue
participant ILocalTimeService
User->>LocalTimeForm: Render / bind
LocalTimeForm->>LocalTimeFormValue: new LocalTimeFormValue(ILocalTimeService)
alt Value is null
LocalTimeFormValue->>ILocalTimeService: read Now
ILocalTimeService-->>LocalTimeFormValue: current local DateTime
LocalTimeFormValue-->>LocalTimeForm: expose ValueOrNow / DateOrToday / TimeOrNow / TimeSpanOrNow
else Value present
LocalTimeFormValue-->>LocalTimeForm: expose Date/Time/TimeSpan derived from Value
end
LocalTimeForm-->>User: populate bound fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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 (8)
example/BlazorLocalTimeSample/BlazorLocalTimeSample.csproj (1)
10-14: Align patch versions: In example/BlazorLocalTimeSample/BlazorLocalTimeSample.csproj, bump Microsoft.AspNetCore.Components.WebAssembly from 9.0.4 to 9.0.8 to match the DevServer and avoid subtle tooling/runtime mismatches.tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs (1)
22-24: Optional: reduce platform variance in time zone assertions.Elsewhere you assert IDs like "America/New_York". On some Windows runners without IANA mapping, this can be fragile. Prefer asserting offsets or using TimeZoneInfo equality when possible.
src/BlazorLocalTime/Components/LocalTimeForm.razor.cs (1)
100-100: Rename TimespanChangedHandler to TimeSpanChangedHandler.Minor naming consistency with .NET type
TimeSpan.Apply:
- TimeSpanChanged = EventCallback.Factory.Create<TimeSpan?>(this, TimespanChangedHandler), + TimeSpanChanged = EventCallback.Factory.Create<TimeSpan?>(this, TimeSpanChangedHandler), @@ - private async Task TimespanChangedHandler(TimeSpan? newTimeSpan) + private async Task TimeSpanChangedHandler(TimeSpan? newTimeSpan)Also applies to: 240-260
src/BlazorLocalTime/Components/LocalTimeFormValue.cs (5)
10-20: Defensive null check for injected service.Guard against accidental nulls to fail fast.
Apply:
internal LocalTimeFormValue(ILocalTimeService localTimeService) { - LocalTimeService = localTimeService; + ArgumentNullException.ThrowIfNull(localTimeService); + LocalTimeService = localTimeService; }
73-82: ValueOrNow may throw before time zone availability.
LocalTimeService.Nowrequires a resolved browser time zone. Verify callers only read this afterIsTimeZoneInfoAvailable == true, or add a safe fallback.Possible safe getter:
- get => _innerValue ?? LocalTimeService.Now.DateTime; + get => _innerValue + ?? (LocalTimeService.IsTimeZoneInfoAvailable + ? LocalTimeService.Now.DateTime + : DateTime.SpecifyKind(DateTime.Now, DateTimeKind.Unspecified));
83-100: Simplify DateOrToday getter.Use DateOnly.FromDateTime and null-coalescing for brevity.
Apply:
- get - { - if (Date.HasValue) - { - return Date.Value; - } - var now = LocalTimeService.Now; - return new DateOnly(now.Year, now.Month, now.Day); - } + get => Date ?? DateOnly.FromDateTime(LocalTimeService.Now.DateTime);
101-110: Prefer TimeOnly.MinValue over constructing 00:00:00.Idiomatic and clearer.
Apply:
- get => Time ?? new TimeOnly(0, 0, 0); + get => Time ?? TimeOnly.MinValue;
111-120: Prefer TimeSpan.Zero overnew TimeSpan(0,0,0).Idiomatic and avoids unnecessary construction.
Apply:
- get => TimeSpan ?? new TimeSpan(0, 0, 0); + get => TimeSpan ?? TimeSpan.Zero;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
example/BlazorLocalTimeSample/BlazorLocalTimeSample.csproj(1 hunks)src/BlazorLocalTime/BlazorLocalTime.csproj(1 hunks)src/BlazorLocalTime/Components/LocalTimeForm.razor.cs(1 hunks)src/BlazorLocalTime/Components/LocalTimeFormValue.cs(2 hunks)src/Directory.Build.props(1 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt(1 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt(1 hunks)tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.cs(4 hunks)tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs (2)
src/BlazorLocalTime/LocalTimeService.cs (2)
SetBrowserTimeZoneInfo(52-52)SetBrowserTimeZoneInfo(165-177)tests/BlazorLocalTimeTest/LocalTimeMockService.cs (1)
LocalTimeEmptyMockService(17-24)
tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.cs (1)
src/BlazorLocalTime/Components/BlazorLocalTimeProvider.razor.cs (1)
BlazorLocalTimeProvider(10-62)
src/BlazorLocalTime/Components/LocalTimeFormValue.cs (3)
src/BlazorLocalTime/LocalTimeService.cs (2)
LocalTimeService(121-178)ILocalTimeService(8-116)src/BlazorLocalTime/Components/LocalTimeText.razor.cs (1)
LocalTimeText(8-59)src/BlazorLocalTime/Components/LocalTime.razor.cs (1)
LocalTime(8-55)
src/BlazorLocalTime/Components/LocalTimeForm.razor.cs (2)
src/BlazorLocalTime/LocalTimeService.cs (1)
LocalTimeService(121-178)src/BlazorLocalTime/Components/LocalTime.razor.cs (1)
LocalTime(8-55)
🔇 Additional comments (7)
tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs (1)
12-12: Whitespace-only changes — OK.No functional impact.
Also applies to: 22-24, 36-38, 42-42
src/BlazorLocalTime/BlazorLocalTime.csproj (1)
11-14: Formatting-only change — LGTM.Multiline PackageReference reads fine. Using major wildcard (6., 7., 8., 9.) is acceptable given the repo uses a lock file.
Also applies to: 19-22, 26-29, 33-36
src/Directory.Build.props (1)
14-18: Formatting-only change — LGTM.No behavioral impact; attributes preserved.
Also applies to: 26-30
tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.cs (1)
22-22: Fluent chain reformat improves readability — LGTM.No logic changes; assertions remain intact.
Also applies to: 25-25, 35-38, 41-41, 50-53, 56-56, 66-66, 70-70
src/BlazorLocalTime/Components/LocalTimeForm.razor.cs (1)
93-101: Passing ILocalTimeService into LocalTimeFormValue looks correct.Instantiating with
new(LocalTimeService)aligns with the new internal ctor and enables the “...Or...” defaults.tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt (1)
47-56: API additions acknowledged (DateOrToday, TimeOrDefault, TimeSpanOrDefault, ValueOrNow).Baselines reflect intended non-nullable bind support. Ensure changelog/docs mention the removed parameterless ctor and the new defaults’ behavior.
tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt (1)
47-56: API additions mirrored for net9.Looks consistent with net8. Confirm package version bump as this is a public API change.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/BlazorLocalTime/Components/LocalTimeFormValue.cs (1)
97-113: Minor simplification for TimeSpanOrNowYou can return now.TimeOfDay directly for readability and consistency with “time of day” semantics.
- var now = LocalTimeService.Now; - return new TimeSpan(now.Hour, now.Minute, now.Second); + return LocalTimeService.Now.TimeOfDay;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/BlazorLocalTime/Components/LocalTimeFormValue.cs(4 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt(1 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt
🧰 Additional context used
🧬 Code graph analysis (2)
tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt (1)
src/BlazorLocalTime/Components/LocalTimeForm.razor.cs (5)
LocalTimeForm(9-261)Task(197-216)Task(240-260)Task(218-238)Task(168-195)
src/BlazorLocalTime/Components/LocalTimeFormValue.cs (2)
src/BlazorLocalTime/LocalTimeService.cs (1)
LocalTimeService(121-178)src/BlazorLocalTime/Components/LocalTimeForm.razor.cs (1)
LocalTimeForm(9-261)
🔇 Additional comments (7)
tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt (1)
47-56: API surface changes look correct and aligned with the PR intentThe additions of DateOrToday, TimeOrNow, TimeSpanOrNow, and ValueOrNow, alongside removal of the parameterless ctor, match the “default non-null value” goal for bindings.
Please confirm the net9 approval baseline also reflects these members and the removed ctor to keep the approvals in sync across target frameworks.
src/BlazorLocalTime/Components/LocalTimeFormValue.cs (6)
42-42: Date getter simplification is correctUsing DateOnly.FromDateTime(Value.Value) is concise and avoids manual component extraction.
51-51: Time getter simplification is correctTimeOnly.FromDateTime(Value.Value) is the right API here.
67-76: ValueOrNow semantics align with non-null binding use-casesReturns LocalTimeService.Now when Value is null and writes through to Value on set. Good.
77-86: DateOrToday is consistent and useful for non-nullable DateOnly bindingsBehavior mirrors Date with a sensible default to today.
87-96: TimeOrNow is consistent and useful for non-nullable TimeOnly bindingsMatches Time behavior with current local time fallback.
10-20: LocalTimeFormValue instantiation verified
FormValue is usingnew(LocalTimeService)and no parameterlessnew()calls remain.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/BlazorLocalTime/Components/LocalTimeFormValue.cs (4)
64-72: Docs polish for ValueOrNowMinor punctuation fix.
-/// It is essentially the same as <see cref="Value"/>. but returns the current date and time when the value is null. <br/> +/// Essentially the same as <see cref="Value"/>, but returns the current date and time when the value is null. <br/>
84-92: Docs polish for TimeOrNowGrammar tweak.
-/// It is essentially the same as <see cref="Time"/>, but returns current time when the value is null. <br/> +/// Essentially the same as <see cref="Time"/>, but returns the current time when the value is null. <br/>
94-102: Docs polish for TimeSpanOrNowClarify “time-of-day”.
-/// It is essentially the same as <see cref="TimeSpan"/>, but returns current time when the value is null. <br/> +/// Essentially the same as <see cref="TimeSpan"/>, but returns the current time-of-day when the value is null. <br/>
16-20: Convert LocalTimeFormValue to a class
It doesn’t leverage any record-only features (no with-expressions, Deconstruct(), or value-equality calls) and will prevent the injected service from participating in equality.-public record LocalTimeFormValue +public sealed class LocalTimeFormValue
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/BlazorLocalTime/Components/LocalTimeFormValue.cs(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/BlazorLocalTime/Components/LocalTimeFormValue.cs (5)
src/BlazorLocalTime/Components/LocalTimeForm.razor.cs (1)
LocalTimeForm(9-261)src/BlazorLocalTime/Components/LocalTimeText.razor.cs (1)
LocalTimeText(8-59)src/BlazorLocalTime/Components/LocalTime.razor.cs (1)
LocalTime(8-55)src/BlazorLocalTime/LocalTimeService.cs (1)
LocalTimeService(121-178)src/BlazorLocalTime/BlazorLocalTimeExtension.cs (1)
BlazorLocalTimeExtension(11-38)
🔇 Additional comments (3)
src/BlazorLocalTime/Components/LocalTimeFormValue.cs (3)
42-61: Getters returning null when Value is null — LGTMDate/Time/TimeSpan fallbacks are correct and side-effect free.
74-82: DateOrToday API — LGTMBehavior aligns with goal for non-nullable binds.
10-15: Add null guard to internal LocalTimeFormValue constructor
InjectedArgumentNullException.ThrowIfNull(localTimeService);and verified no remaining parameterless or target-typed instantiations.
for non-nullable datetime component, such as MasaBlazor.DateTimePicker
…plify TimeSpan logic
…e-time input handling
arika0093
force-pushed
the
feat/add_default_formvalue
branch
from
f2b2235 to
5066910
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
tests/BlazorLocalTimeTest/Component/LocalTimeFormWithDefault.razor (1)
2-4: Optional: add ValueExpression for validation scenarios.If this component is ever wrapped in an EditForm with validation, consider supplying ValueExpression on each InputDate for full validation wiring.
-<InputDate Type="InputDateType.DateTimeLocal" @bind-Value="dtf.ValueOrNow" /> +<InputDate Type="InputDateType.DateTimeLocal" + @bind-Value="dtf.ValueOrNow" + ValueExpression="() => dtf.ValueOrNow" />(Same idea for DateOrToday and TimeOrNow.)
tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt (1)
43-57: Public API changes acknowledged; document the breaking ctor removal.The removal of LocalTimeFormValue’s public parameterless constructor is a breaking change. Please document migration (instances come from LocalTimeForm context) and consider a version bump per SemVer.
If you want, I can draft a short MIGRATION note for the README/CHANGELOG.
tests/BlazorLocalTimeTest/LocalTimeFormDefaultTest.razor (2)
7-29: Nice end-to-end coverage with deterministic time; add a quick null assert before update.As a small guard, assert Dt is null before user interaction.
var input_time = cut.Find("input[type='time']"); // value is set to the local time in the input input_dt.GetAttribute("value").ShouldBe("2020-01-01T21:34:56"); input_date.GetAttribute("value").ShouldBe("2020-01-01"); input_time.GetAttribute("value").ShouldBe("21:34:56"); + dt.ShouldBeNull();
7-9: Rename test for clarity.Name reflects intent better, e.g., “InitialDefaultsAndUpdate_UsesBrowserTZ_AndConvertsBackToUtc”.
src/BlazorLocalTime/Components/LocalTimeFormValue.cs (3)
8-9: Consider class over record to avoid surprising equality/with semantics.This mutable record carries DI state and EventCallbacks; value-based equality and with-cloning can be surprising. If feasible, switch to a sealed class (future, not blocking).
- public record LocalTimeFormValue + public sealed class LocalTimeFormValue
10-14: Add null guard for the injected service.Defensive check prevents late NREs if DI is misconfigured.
- internal LocalTimeFormValue(ILocalTimeService localTimeService) - { - _localTimeService = localTimeService; - } + internal LocalTimeFormValue(ILocalTimeService localTimeService) + { + _localTimeService = localTimeService ?? throw new ArgumentNullException(nameof(localTimeService)); + }
64-102: Non-null accessors look good; minor consistency note.Using _localTimeService.Now per accessor is fine. If you ever need atomic “today/now” across multiple bindings in one render, consider capturing a single snapshot in the parent and passing it in.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
example/BlazorLocalTimeSample/BlazorLocalTimeSample.csproj(1 hunks)src/BlazorLocalTime/BlazorLocalTime.csproj(1 hunks)src/BlazorLocalTime/Components/LocalTimeForm.razor.cs(1 hunks)src/BlazorLocalTime/Components/LocalTimeFormValue.cs(4 hunks)src/Directory.Build.props(1 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt(1 hunks)tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt(1 hunks)tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.cs(4 hunks)tests/BlazorLocalTimeTest/Component/LocalTimeFormWithDefault.razor(1 hunks)tests/BlazorLocalTimeTest/LocalTimeFormDefaultTest.razor(1 hunks)tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/BlazorLocalTime/BlazorLocalTime.csproj
- tests/BlazorLocalTimeTest/BlazorLocalTimeProviderCodeTest.cs
🚧 Files skipped from review as they are similar to previous changes (5)
- example/BlazorLocalTimeSample/BlazorLocalTimeSample.csproj
- src/Directory.Build.props
- tests/BlazorLocalTimeTest/LocalTimeServiceTest.cs
- tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net9.approved.txt
- src/BlazorLocalTime/Components/LocalTimeForm.razor.cs
🧰 Additional context used
🧬 Code graph analysis (3)
tests/BlazorLocalTimeTest/LocalTimeFormDefaultTest.razor (2)
tests/BlazorLocalTimeTest/TestInitializer.cs (1)
TestInitializer(8-25)tests/BlazorLocalTimeTest/LocalTimeMockService.cs (2)
LocalTimeMockService(7-15)LocalTimeMockServiceExtension(26-34)
tests/BlazorLocalTimeTest/Approvals/PublicApiCheckTest.Run.net8.approved.txt (1)
tests/BlazorLocalTimeTest/PublicApiCheckTest.cs (1)
PublicApiCheckTest(8-30)
src/BlazorLocalTime/Components/LocalTimeFormValue.cs (4)
src/BlazorLocalTime/Components/LocalTimeText.razor.cs (1)
LocalTimeText(8-59)src/BlazorLocalTime/Components/LocalTime.razor.cs (1)
LocalTime(8-55)src/BlazorLocalTime/LocalTimeService.cs (1)
LocalTimeService(121-178)src/BlazorLocalTime/BlazorLocalTimeExtension.cs (1)
BlazorLocalTimeExtension(11-38)
🔇 Additional comments (3)
tests/BlazorLocalTimeTest/Component/LocalTimeFormWithDefault.razor (1)
1-5: LGTM: bindings correctly use the new non-null accessors.The component exercises ValueOrNow/DateOrToday/TimeOrNow exactly as intended for non-nullable inputs. No issues found.
src/BlazorLocalTime/Components/LocalTimeFormValue.cs (2)
27-35: EventCallback invocation pattern is acceptable.Set then InvokeAsync is the right order; not awaiting in a setter is expected in Blazor.
37-62: Callbacks correctly wired for Value recomposition
DateChangedHandler, TimeChangedHandler, and TimeSpanChangedHandler all recombine their inputs and invoke ValueChangedHandler, ensuring Value is updated upstream.
1 participant
Add an API to LocalTimeFormValue that returns today's time if no value is present.
These APIs are useful for specifying values with @bind-Value on non-nullable DateTime components.
Summary by CodeRabbit
New Features
Refactor
Style
Tests