Apply BitDatePicker improvements (#12861) - #12863
Open
msynk wants to merge 9 commits into
Open
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBitDatePicker adds configurable date rules, calendar layouts, highlighting, callbacks, time-zone handling, keyboard navigation, accessibility semantics, sizing, colors, expanded demos, and comprehensive tests. ChangesDatePicker improvements
Estimated code review effort: 5 (Critical) | ~90+ minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs (1)
1917-1940: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider caching the focusable day per render.
GetFocusableDay()runs on each render from the markup, and it can scan the full 6x7 grid. Each scanned cell invokes the user suppliedIsDateDisableddelegate. The markup then callsIsDayDisabled(date)again for every cell. A consumer with an expensiveIsDateDisabledpays for it many times per render. Caching the result when the month data is generated would remove the repeated scan.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs` around lines 1917 - 1940, Cache the focusable day while generating the current month’s _daysOfCurrentMonth data, reusing the same disabled-date evaluations instead of rescanning the grid in GetFocusableDay() on every render. Update GetFocusableDay() to return the cached value, and invalidate or refresh that cache whenever the month data changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor`:
- Around line 317-330: Update the day button markup in BitDatePicker so
aria-readonly is emitted only when ReadOnly is true, instead of always setting
it to "true". Keep the existing disabled and aria-disabled behavior unchanged.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs`:
- Around line 1215-1225: Update the value comparison in the date input handler
around CurrentValueAsString and CheckCurrentCalendarMatchesCurrentValue to
compare the nullable CurrentValue directly before and after parsing, without
using GetNow() defaults. Preserve the early return when the picker is closed or
the nullable values are equal, so invalid typed text leaves CurrentValue null
and skips calendar synchronization.
- Around line 1306-1309: Update the selection flow in the date-selection method
containing IsDayDisabled so one-way IsOpen binding does not reject date
selection. Remove the early return that combines Standalone, IsOpenHasBeenSet,
and IsOpenChanged.HasDelegate; apply that condition only within the AutoClose
handling that calls AssignIsOpen(false), preserving standalone behavior and
allowing selections when AutoClose is false.
- Around line 883-895: Move the Responsive-gated BitSwipesSetup call into the
existing try block that guards BitCalendarsSetup in the first-render
initialization path, so both JavaScript calls are protected from
JSDisconnectedException while preserving the current Responsive condition and
arguments.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scss`:
- Around line 904-922: Update the responsive calendar-cell styling associated
with .bit-dtp-res so its font size uses the active var(--bit-dtp-cell-fs) value
instead of a fixed spacing(1.75). Preserve the size-specific values configured
by .bit-dtp-sm, .bit-dtp-md, and .bit-dtp-lg.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor`:
- Line 100: Change the GetDayClass attribute in BitDatePickerDemo.razor at lines
100-100 to use single-quote delimiters while preserving the lambda and
"sunday-cell" literal. Update the corresponding verbatim sample in
BitDatePickerDemo.razor.samples.cs at lines 54-55 to render the same
single-quoted attribute syntax.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.cs`:
- Around line 1104-1128: Update the class-style documentation entries in the
BitDatePicker demo to add the missing ClearButtonIcon property metadata,
matching the public string? ClearButtonIcon member in BitDatePickerClassStyles
and its corresponding description/default value.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.samples.cs`:
- Around line 54-55: Update the displayed BitDatePicker sample in the relevant
demo sample definition so its rendered Razor markup matches the corrected
GetDayClass example in BitDatePickerDemo.razor, using the proper verbatim-string
escaping and avoiding duplicated nested quotes.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs`:
- Around line 1282-1287: Update the comment above the expected week calculation
in the date-picker test to state that the first rendered row contains Monday,
February 23, 2026 through Sunday, March 1, rather than starting on March 2. Keep
the existing expected value and assertion unchanged.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs`:
- Around line 1917-1940: Cache the focusable day while generating the current
month’s _daysOfCurrentMonth data, reusing the same disabled-date evaluations
instead of rescanning the grid in GetFocusableDay() on every render. Update
GetFocusableDay() to return the cached value, and invalidate or refresh that
cache whenever the month data changes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 759ed505-67b5-4c04-a488-2b330f3b880b
📒 Files selected for processing (10)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePickerClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Calendars.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.scsssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.cs (1)
1104-1298: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRemove or document the undocumented
ClearButtonIconslot.
ClearButtonIconis documented in the Datepicker class/styles examples, but it has no matching public property onBitDatePickerClassStyles.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.cs` around lines 1104 - 1298, Update the BitDatePicker class/styles documentation metadata near the existing ClearButton-related entries to either remove the undocumented ClearButtonIcon slot or add the corresponding public BitDatePickerClassStyles property; keep the documentation and available API symbols consistent.
🧹 Nitpick comments (3)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs (2)
868-873: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRebuild the date lookups only when the source collections change.
BuildDatesLookupsruns on everyOnParametersSetcall. It allocates threeHashSetinstances each time, even whenDisabledDates,HighlightedDates, andDisabledDaysOfWeekdid not change. The other date parameters in this component use[CallOnSet(nameof(OnSetParameters))]for the same purpose. Consider the same attribute pattern here to keep the allocation off the parameter-set path.♻️ Proposed refactor
- protected override void OnParametersSet() - { - base.OnParametersSet(); - - BuildDatesLookups(); - } + protected override void OnParametersSet() + { + base.OnParametersSet(); + }Then annotate the three parameters:
[Parameter] [CallOnSet(nameof(BuildDatesLookups))] public IEnumerable<DateTimeOffset>? DisabledDates { get; set; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs` around lines 868 - 873, Update the DisabledDates, HighlightedDates, and DisabledDaysOfWeek parameters to use [CallOnSet(nameof(BuildDatesLookups))], and remove the unconditional BuildDatesLookups call from OnParametersSet while preserving base.OnParametersSet().
800-816: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRun the new public callout API on the renderer dispatcher.
OpenCalloutandCloseCalloutAndFocusare public. External code can call them from a context that is not the renderer synchronization context, for example a timer callback or a background service.HandleOnClickandCloseCalloutAndRestoreFocusmutate component state and perform JS interop.CloseCalloutAndFocusalready wraps only theStateHasChangedcall inInvokeAsync, which leaves the state mutation unprotected.Wrap the whole body of both methods in
InvokeAsync.♻️ Proposed refactor
public Task OpenCallout() { - return HandleOnClick(); + return InvokeAsync(HandleOnClick); } public async Task CloseCalloutAndFocus() { - await CloseCalloutAndRestoreFocus(); - await InvokeAsync(StateHasChanged); + await InvokeAsync(async () => + { + await CloseCalloutAndRestoreFocus(); + StateHasChanged(); + }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs` around lines 800 - 816, Update the public methods OpenCallout and CloseCalloutAndFocus to execute their entire bodies through InvokeAsync, including HandleOnClick, CloseCalloutAndRestoreFocus, and StateHasChanged. Preserve each method’s existing asynchronous behavior while ensuring state mutations and JS interop run on the renderer dispatcher.src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor (1)
137-143: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueCompute
focusableDayonly when the day picker renders.
GetFocusableDay()runs on every render, includingMonthPickermode and the states whereShowDayPicker()returns false. The method scans up to 42 grid cells and callsIsDayDisabledfor each, which can invoke the user-suppliedIsDateDisableddelegate. The value is used only inside theShowDayPicker()block at line 329.♻️ Proposed refactor
@{ var today = GetToday(); var todayYear = _culture.Calendar.GetYear(today); var todayMonth = _culture.Calendar.GetMonth(today); - var focusableDay = GetFocusableDay(); var showTimePicker = ShowTimePicker && ((_showTimePickerAsOverlayInternal && _isTimePickerOverlayOnTop) || _showTimePickerAsOverlayInternal is false); } `@if` (ShowDayPicker()) { + var focusableDay = GetFocusableDay(); <div style="`@Styles`?.DayPickerWrapper" class="bit-dtp-dwp `@Classes`?.DayPickerWrapper">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor` around lines 137 - 143, Update the render-state initialization around GetFocusableDay so focusableDay is computed only when ShowDayPicker() is true; preserve the existing value and usage within the day-picker rendering block while avoiding GetFocusableDay() for MonthPicker and other non-day-picker states.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor`:
- Line 327: Update the day-button style composition around GetDayButtonCss and
the rendered style attribute so generated styles are separated from
Styles?.DayButton by a semicolon. Prefer combining Styles?.DayButton through
AppendStyle within GetDayButtonCss and render only the resulting style value in
the Razor markup, preserving all existing declarations.
- Around line 126-131: Add focus trapping to the DatePicker callout represented
by the callout container, activating it only when Standalone is false so Tab and
Shift+Tab remain within the modal dialog; preserve the existing Escape and
selection focus restoration behavior.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs`:
- Around line 1381-1396: Update ClampToRange so the MinDate and MaxDate branches
return the same full-precision bound used by IsWeekDayOutOfMinAndMaxDate, rather
than calling .Date on the bound. Preserve the existing month checks and return
date unchanged when no clamp applies.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scss`:
- Line 3: Update the color-role stylesheet import in BitDatePicker.scss to omit
the partial-file extension, while preserving the existing relative path and
imported stylesheet.
- Line 396: Update the `.bit-dtp-pkt` and `.bit-dtp-ptb` header line-height
rules to use the active date-picker cell size, matching the `.bit-dtp-sm` header
height and preventing title overflow into the calendar grid.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs`:
- Around line 2019-2029: Update BitDatePickerDisposeShouldNotThrow to dispose
the rendered component through bUnit rather than calling
component.Instance.DisposeAsync() directly, allowing teardown to perform the
single framework-managed disposal while preserving the test’s no-throw
assertion.
---
Outside diff comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.cs`:
- Around line 1104-1298: Update the BitDatePicker class/styles documentation
metadata near the existing ClearButton-related entries to either remove the
undocumented ClearButtonIcon slot or add the corresponding public
BitDatePickerClassStyles property; keep the documentation and available API
symbols consistent.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor`:
- Around line 137-143: Update the render-state initialization around
GetFocusableDay so focusableDay is computed only when ShowDayPicker() is true;
preserve the existing value and usage within the day-picker rendering block
while avoiding GetFocusableDay() for MonthPicker and other non-day-picker
states.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs`:
- Around line 868-873: Update the DisabledDates, HighlightedDates, and
DisabledDaysOfWeek parameters to use [CallOnSet(nameof(BuildDatesLookups))], and
remove the unconditional BuildDatesLookups call from OnParametersSet while
preserving base.OnParametersSet().
- Around line 800-816: Update the public methods OpenCallout and
CloseCalloutAndFocus to execute their entire bodies through InvokeAsync,
including HandleOnClick, CloseCalloutAndRestoreFocus, and StateHasChanged.
Preserve each method’s existing asynchronous behavior while ensuring state
mutations and JS interop run on the renderer dispatcher.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dd068a77-b669-4eb4-b072-90b63b9b7778
📒 Files selected for processing (10)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePickerClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Calendars.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.scsssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs`:
- Around line 941-952: Update the parsed-date validation in the date parsing
method to call IsDayDisabled, ensuring typed dates honor DisabledDates,
DisabledDaysOfWeek, and related disabled-date rules just like SelectDate.
Preserve the existing out-of-range validation and return the appropriate
disabled-date validation message when IsDayDisabled rejects the value; ensure
BuildDatesLookups has populated the required lookups before validation.
- Around line 1929-1952: Update GetFocusableDay so its last-resort fallback
prefers an in-month date rather than _daysOfCurrentMonth[0, 0], avoiding an
outside-month placeholder when ShowOutsideDays is false. Also handle a fully
disabled month explicitly so the grid still has the intended focus behavior,
without returning a disabled day as though it were tabbable.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePickerClassStyles.cs`:
- Around line 140-144: Add focused tests in BitDatePickerTests for the public
DaysGrid and DayNameHeader styling properties, verifying each supplied CSS
class/style is rendered on its corresponding days grid or day-name header
element, alongside the existing HighlightedDayButton coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 065212ab-4c9f-4254-90cf-23fcf892da97
📒 Files selected for processing (10)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePickerClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Calendars.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.scsssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs (1)
1364-1372: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
CultureInfo.DateTimeFormat.Calendarto set the Hebrew calendar.The private
CultureInfo._calendarfield is an internal implementation detail, whileDateTimeFormatInfo.Calendaris the supported public API for assigning a different calendar to a culture. If theSystem.Reflectionimport is only used for that helper call, remove it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs` around lines 1364 - 1372, Update the culture setup in the date picker test to assign the HebrewCalendar through culture.DateTimeFormat.Calendar instead of reflecting into the private _calendar field. Remove the System.Reflection import if it is no longer used elsewhere in the test file, while preserving the existing calendar assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor`:
- Line 465: Update the month and year cell aria-readonly attributes in the
BitDatePicker markup to bind to the ReadOnly property instead of emitting the
literal true value, matching the existing day-cell behavior.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs`:
- Around line 1114-1116: In the open flow around AssignIsOpen(true), reset
_focusDayOnOpen before returning when the open request is refused. Ensure the
flag is cleared on the false path while preserving the existing behavior for
successful opens and the later pointer-open handling.
- Around line 1259-1269: Update the synchronization guard in the DatePicker
value-change handler around CurrentValue and
CheckCurrentCalendarMatchesCurrentValue to return when CurrentValue has no
value, before invoking the calendar sync. Preserve the existing IsOpen and
oldValue comparison checks, and ensure unparseable typed text does not move the
calendar to the current month.
In `@src/BlazorUI/Bit.BlazorUI/Scripts/Calendars.ts`:
- Around line 12-14: Update the _focusables selector used by wrapFocus to
exclude every element with tabindex="-1", including buttons that explicitly set
it, so roving calendar cells cannot define the focus-trap boundaries. Preserve
the existing inclusion of enabled buttons, enabled inputs, and other tabbable
elements.
---
Nitpick comments:
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs`:
- Around line 1364-1372: Update the culture setup in the date picker test to
assign the HebrewCalendar through culture.DateTimeFormat.Calendar instead of
reflecting into the private _calendar field. Remove the System.Reflection import
if it is no longer used elsewhere in the test file, while preserving the
existing calendar assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 447558c4-7eba-421f-847e-805c7dd43ac9
📒 Files selected for processing (12)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Calendar/BitCalendar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePickerClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/CalendarsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Calendars.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.scsssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs (2)
1124-1137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the culture in tests that parse rendered day numbers.
int.Parse(b.TextContent.Trim())reads digits rendered withCultureInfo.CurrentUICulture. A CI agent with a culture that renders non-ASCII digits, such asar-SAorfa-IR, makes this test fail. Several other new tests compareTextContentagainst ASCII strings, for example lines 1101, 1183, 1198, and 1836.Add
parameters.Add(p => p.Culture, CultureInfo.InvariantCulture);to those tests, as the keyboard tests at lines 1852-1857 already do.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs` around lines 1124 - 1137, Pin the component culture to CultureInfo.InvariantCulture in BitDatePickerShouldRespectIsDateDisabled and the other new tests comparing rendered TextContent or parsing day numbers, including the cases around lines 1101, 1183, 1198, and 1836. Add the Culture parameter alongside the existing test parameters, matching the established keyboard-test pattern.
1359-1383: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the reflection into
CultureInfofail with a clear reason.
GetField("_calendar", ...)targets a private BCL field with different initial null behavior, so this test can fail silently whileCultureInfo.Calendarstill works unchanged. Remove the null-forgiving operator and assert on the reflective field before callingSetValue, with a message that explains this is runtime-internal detail and needs updating when the field changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs` around lines 1359 - 1383, The BitDatePickerShouldRenderEveryMonthOfANonGregorianCalendar test should validate the reflective CultureInfo field before using it. Remove the null-forgiving operator from GetField("_calendar", ...) and assert the field is non-null with a clear message explaining that this depends on runtime-internal implementation details and must be updated if the field changes, then call SetValue only after that assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs`:
- Around line 1415-1447: Update ClampToRange and IsWeekDayOutOfMinAndMaxDate so
clamping returns date-only values, while range validation compares against
appropriately truncated bound dates. Ensure SelectDate receives midnight dates
in MonthPicker mode, preventing AddHours/AddMinutes from accumulating bound
times and preserving _focusedDate matching against calendar cells.
---
Nitpick comments:
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs`:
- Around line 1124-1137: Pin the component culture to
CultureInfo.InvariantCulture in BitDatePickerShouldRespectIsDateDisabled and the
other new tests comparing rendered TextContent or parsing day numbers, including
the cases around lines 1101, 1183, 1198, and 1836. Add the Culture parameter
alongside the existing test parameters, matching the established keyboard-test
pattern.
- Around line 1359-1383: The
BitDatePickerShouldRenderEveryMonthOfANonGregorianCalendar test should validate
the reflective CultureInfo field before using it. Remove the null-forgiving
operator from GetField("_calendar", ...) and assert the field is non-null with a
clear message explaining that this depends on runtime-internal implementation
details and must be updated if the field changes, then call SetValue only after
that assertion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a0aa4171-856a-4473-931f-4d6e01965ac1
📒 Files selected for processing (12)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Calendar/BitCalendar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePickerClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/CalendarsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Calendars.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.scsssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scss (1)
3-3: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the partial-file extension from the import.
Stylelint still reports
scss/load-partial-extensionon this line. Use the extensionless partial import.Proposed fix
-@import "../../../Styles/color-role-maps.scss"; +@import "../../../Styles/color-role-maps";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scss` at line 3, Update the color-role-maps import in BitDatePicker.scss to use the extensionless partial path, removing the .scss suffix while preserving the existing relative import location.Source: Linters/SAST tools
🧹 Nitpick comments (1)
src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs (1)
1364-1391: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid reflection into
CultureInfo._calendar.
BitDatePickerreads_culture.Calendar, and the test currently injects a non-GregorianHebrewCalendarthrough a private field..NETprovidesDateTimeFormat.Calendarfor the same calendar effect without runtime internals. Setculture.DateTimeFormat.Calendar = calendarbefore rendering the component, and keep this check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs` around lines 1364 - 1391, Update BitDatePickerShouldRenderEveryMonthOfANonGregorianCalendar to stop reflecting on CultureInfo._calendar and remove the related BindingFlags, FieldInfo, and null-check logic. Configure the HebrewCalendar through culture.DateTimeFormat.Calendar before rendering, while preserving the existing leap-year assertions and 13-month rendering check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs`:
- Around line 957-979: Update the typed-value validation in the parsed-value
flow to apply ClampToRange when Mode is BitDatePickerMode.MonthPicker before
calling IsWeekDayOutOfMinAndMaxDate, matching the behavior of SelectMonth.
Preserve the existing disabled-date validation and non-month modes unchanged.
---
Duplicate comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scss`:
- Line 3: Update the color-role-maps import in BitDatePicker.scss to use the
extensionless partial path, removing the .scss suffix while preserving the
existing relative import location.
---
Nitpick comments:
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs`:
- Around line 1364-1391: Update
BitDatePickerShouldRenderEveryMonthOfANonGregorianCalendar to stop reflecting on
CultureInfo._calendar and remove the related BindingFlags, FieldInfo, and
null-check logic. Configure the HebrewCalendar through
culture.DateTimeFormat.Calendar before rendering, while preserving the existing
leap-year assertions and 13-month rendering check.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e4e29551-ff3d-4afd-ad6b-15b2fb64f2a6
📒 Files selected for processing (12)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Calendar/BitCalendar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePickerClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/CalendarsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Calendars.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.scsssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs
…into 12861-blazorui-datepicker-improvements
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor (1)
267-274: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the TimeZone example text.
Line 267 contains
Remeber. Line 274 containsDefalt.Proposed fix
- Remeber using this feature in different runtimes needs more investigations, for example, + Remember that using this feature in different runtimes needs more investigation. For example, ... - <div>Defalt (local TimeZone):</div><br/> + <div>Default (local TimeZone):</div><br/>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor` around lines 267 - 274, Correct the spelling errors in the DatePicker demo explanatory text: update “Remeber” and “Defalt” to their correct spellings while preserving the surrounding content and formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor`:
- Around line 55-57: Update the MinDate/MaxDate description in BitDatePickerDemo
to qualify typed-input validation: state that out-of-range values entered in the
input are reported as validation errors only when AllowTextInput is enabled,
while preserving the existing range and navigation behavior.
---
Outside diff comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor`:
- Around line 267-274: Correct the spelling errors in the DatePicker demo
explanatory text: update “Remeber” and “Defalt” to their correct spellings while
preserving the surrounding content and formatting.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e4c689fe-7783-4887-aee8-333f678efdb1
📒 Files selected for processing (12)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Calendar/BitCalendar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePickerClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/CalendarsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Calendars.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DatePicker/BitDatePickerDemo.razor.scsssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DatePicker/BitDatePickerTests.cs
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.
closes #12861
Summary by CodeRabbit
New Features
Accessibility
Documentation
Tests