Conversation
Contributor
Reviewer's GuideAdds customizable DateTime/DateTimeOffset parsing for DateTimePicker, including component-level parameters and global options, and updates unit tests to cover nullable and offset scenarios and the new resolution priority behavior. Sequence diagram for DateTimePicker parsing resolution prioritysequenceDiagram
participant DateTimePicker
participant BootstrapBlazorOptions
participant DateTimeSettings
participant DateTimeHelper
DateTimePicker->>DateTimePicker: TryParseValueFromString(value, out result, out validationErrorMessage)
alt ValueType == typeof(DateTime)
DateTimePicker->>DateTimePicker: TryParseDateTime(value, out val)
DateTimePicker->>BootstrapBlazorOptions: Options.CurrentValue
BootstrapBlazorOptions-->>DateTimePicker: options
DateTimePicker->>DateTimeSettings: options.DateTimeSettings.ParseDateTimeResolve
alt ParseDateTimeResolve parameter or global option exists
DateTimePicker-->>DateTimePicker: resolve(value)
else no custom resolver
DateTimePicker->>DateTimeHelper: TryToDateTime(value, out d)
DateTimeHelper-->>DateTimePicker: ret, d
end
else ValueType != typeof(DateTime)
DateTimePicker->>DateTimePicker: TryParseDateTimeOffset(value, out val)
DateTimePicker->>BootstrapBlazorOptions: Options.CurrentValue
BootstrapBlazorOptions-->>DateTimePicker: options
DateTimePicker->>DateTimeSettings: options.DateTimeSettings.ParseDateTimeOffsetResolve
alt ParseDateTimeOffsetResolve parameter or global option exists
DateTimePicker-->>DateTimePicker: resolve(value)
else no custom resolver
DateTimePicker->>DateTimeHelper: ToDateTimeOffset(value)
DateTimeHelper-->>DateTimePicker: v
end
end
DateTimePicker-->>DateTimePicker: result = (TValue)val
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![bb-auto[bot]](https://avatars.githubusercontent.com/in/287253?s=60&v=4){kind=small}
ArgoZhang
enabled auto-merge (squash)
August 6, 2026 09:20
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
TryParseValueFromStringyou assume anything that's notDateTimeshould go through theDateTimeOffsetpath; consider explicitly checking forDateTimeOffsetand falling back to the original parsing logic (or throwing) for unsupportedTValuetypes to avoid silent misparsing if new value types are introduced. - Both
TryParseDateTimeandTryParseDateTimeOffsetassign sentinel values (DateTime.MinValue/DateTimeOffset.MinValue) on failure even though the caller ignoresvalwhenretis false; you can simplify by settingval = default!on failure to make the intent clearer and avoid misleading values.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `TryParseValueFromString` you assume anything that's not `DateTime` should go through the `DateTimeOffset` path; consider explicitly checking for `DateTimeOffset` and falling back to the original parsing logic (or throwing) for unsupported `TValue` types to avoid silent misparsing if new value types are introduced.
- Both `TryParseDateTime` and `TryParseDateTimeOffset` assign sentinel values (`DateTime.MinValue` / `DateTimeOffset.MinValue`) on failure even though the caller ignores `val` when `ret` is false; you can simplify by setting `val = default!` on failure to make the intent clearer and avoid misleading values.
## Individual Comments
### Comment 1
<location path="src/BootstrapBlazor/Options/BootstrapBlazorOptions.cs" line_range="193-198" />
<code_context>
+ /// <para lang="en">Gets or sets the <see cref="DateTimeSettings"/> configuration instance</para>
+ /// <para>v<version>10.9.1</version></para>
+ /// </summary>
+ public DateTimeSettings DateTimeSettings { get; set; } = new();
+
BootstrapBlazorOptions IOptions<BootstrapBlazorOptions>.Value => this;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider making `DateTimeSettings` non-nullable or read-only to reflect its required nature.
Because `DateTimeSettings` is consumed without null checks and initialized with `new()`, allowing it to be set to `null` breaks the intended invariant and can lead to null-reference issues. Making the property non-nullable and either removing the public setter or enforcing a non-null check in the setter would keep the type and usage consistent and prevent these latent bugs.
```suggestion
/// <summary>
/// <para lang="zh">获得 <see cref="DateTimeSettings"/> 配置实例</para>
/// <para lang="en">Gets the <see cref="DateTimeSettings"/> configuration instance</para>
/// <para>v<version>10.9.1</version></para>
/// </summary>
public DateTimeSettings DateTimeSettings { get; } = new();
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8302 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 770 771 +1
Lines 34509 34537 +28
=========================================
+ Hits 34509 34537 +28
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 #8301
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Introduce customizable date/time parsing for DateTimePicker, with support for both per-component and global configuration, and improve handling of DateTime and DateTimeOffset values.
New Features:
Enhancements:
Tests: