Apply BitDateRangePicker improvements (#12867) - #12868
Open
msynk wants to merge 6 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:
WalkthroughThe ChangesDateRangePicker enhancements
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Input
participant BitDateRangePicker
participant CalendarJavaScript
participant BoundValue
Input->>BitDateRangePicker: Enter text or select a date
BitDateRangePicker->>CalendarJavaScript: Handle keyboard, focus, and swipe events
BitDateRangePicker->>BoundValue: Apply validated copied range
BitDateRangePicker-->>Input: Render months, presets, focus, and ARIA state
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 |
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.scss (1)
403-412: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winConfigure the selected-range body color.
Line 406 keeps the range body at
$clr-pri-light. The configured color applies to range endpoints, but dates between them remain primary-colored. Add a range-body CSS custom property to each palette and use it here.Proposed fix
.bit-dtrp-dsb { - background-color: $clr-pri-light; + background-color: var(--bit-dtrp-clr-range); }🤖 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/DateRangePicker/BitDateRangePicker.scss` around lines 403 - 412, Update the .bit-dtrp-dsb selected-range body styling to use a dedicated range-body CSS custom property instead of the hardcoded $clr-pri-light value. Define that custom property consistently in each date-range-picker palette alongside the existing endpoint color variables, while preserving the current hover behavior.
🧹 Nitpick comments (5)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.cs (3)
136-141: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestore the alphabetical order of the parameter entries.
The
componentParameterslist is sorted byName. Three new or moved entries break that order, so the rendered parameter table is harder to scan.
ExcludeDisabledDates(Line 136) precedes theEndTime*entries. It belongs afterEndTimeIncreaseMinuteIconName.OnMonthChange(Line 548) andOnPresetSelect(Line 560) are interleaved withOnFocusInandOnFocusOut. The order should beOnClick,OnFocus,OnFocusIn,OnFocusOut,OnMonthChange,OnPresetSelect.ShowOutsideDays(Line 694) followsShowTimePicker. It belongs beforeShowTimePicker.Also applies to: 547-566, 693-698
🤖 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/DateRangePicker/BitDateRangePickerDemo.razor.cs` around lines 136 - 141, Restore alphabetical ordering in the componentParameters entries: move ExcludeDisabledDates after EndTimeIncreaseMinuteIconName, order the On* entries as OnClick, OnFocus, OnFocusIn, OnFocusOut, OnMonthChange, OnPresetSelect, and move ShowOutsideDays before ShowTimePicker. Preserve each entry’s existing metadata and descriptions while changing only their list positions.
1839-1912: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider removing the redundant
StateHasChangedcall and aligning the field name.
HandleValidSubmitruns as anEditFormevent handler, so Blazor re-renders the component after it returns. The explicitStateHasChanged()call at Line 1905 adds no effect.SuccessMessageat Line 1899 is a private field but uses PascalCase, while the other private fields in this file use camelCase.♻️ Proposed cleanup
- private string SuccessMessage = string.Empty; + private string successMessage = string.Empty; private FormValidationDateRangePickerModel validationModel = new(); private void HandleValidSubmit() { - SuccessMessage = "Form Submitted Successfully!"; - StateHasChanged(); + successMessage = "Form Submitted Successfully!"; } private void HandleInvalidSubmit() { - SuccessMessage = string.Empty; + successMessage = string.Empty; }If you rename the field, update the two references in
BitDateRangePickerDemo.razor(Lines 457, 462-464) and the sample string inBitDateRangePickerDemo.razor.samples.cs.🤖 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/DateRangePicker/BitDateRangePickerDemo.razor.cs` around lines 1839 - 1912, Remove the redundant StateHasChanged call from HandleValidSubmit and rename the private SuccessMessage field to camelCase, updating both references in BitDateRangePickerDemo.razor and the corresponding sample string in BitDateRangePickerDemo.razor.samples.cs.
1064-1084: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDocument the remaining
BitDateRangePickerClassStylesproperties.
Id = "daterangepicker-class-styles"documents the class-style part names, but it omits the existing propertiesClearButtonandClearButtonIconfromBitDateRangePickerClassStyles. Add documentation for those two entries so the documented API stays aligned with the type.🤖 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/DateRangePicker/BitDateRangePickerDemo.razor.cs` around lines 1064 - 1084, Add documentation entries for the existing BitDateRangePickerClassStyles properties ClearButton and ClearButtonIcon in the daterangepicker-class-styles definition, matching the established Name, Type, DefaultValue, and Description structure used by PresetsContainer and the other class-style entries.src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DateRangePicker/BitDateRangePickerTests.cs (1)
1974-1993: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe test name does not match what the test verifies.
BitDateRangePickerDisabledPresetShouldNotBeSelectablesetsReadOnlytotrueand supplies an enabled preset. It therefore verifies the read-only guard inSelectPreset, notpreset.IsEnabled. Rename the test to describe the read-only behavior, and add a separate test that setsIsEnabled = falseon the preset.🤖 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/DateRangePicker/BitDateRangePickerTests.cs` around lines 1974 - 1993, Rename BitDateRangePickerDisabledPresetShouldNotBeSelectable to describe that a read-only date range picker cannot select a preset. Add a separate test covering a preset with IsEnabled set to false, verifying that selecting it does not update the picker value.src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs (1)
1652-1662: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
IsPresetSelectedruns everyValueProvideron every render.The razor markup calls
IsPresetSelectedfor each preset on each render. For presets that are not the currently selected one,ValueProvider()is invoked again, so a provider with non-trivial work runs repeatedly. Consider comparing onlypreset.Valuehere and relying on_selectedPresetfor provider-based presets.♻️ Proposed refactor
private bool IsPresetSelected(BitDateRangePickerPreset preset) { if (CurrentValue is null) return false; if (ReferenceEquals(preset, _selectedPreset)) return true; - var presetValue = preset.ValueProvider is not null ? preset.ValueProvider() : preset.Value; - if (presetValue is null) return false; + // A ValueProvider preset is only marked through _selectedPreset, so the provider + // is not re-evaluated on every render. + if (preset.ValueProvider is not null) return false; + + var presetValue = preset.Value; + if (presetValue is null) return false; return presetValue.StartDate == CurrentValue.StartDate && presetValue.EndDate == CurrentValue.EndDate; }🤖 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/DateRangePicker/BitDateRangePicker.razor.cs` around lines 1652 - 1662, Update IsPresetSelected to avoid invoking preset.ValueProvider during render-time comparisons; retain the ReferenceEquals(preset, _selectedPreset) fast path for provider-based presets, and compare dates only against preset.Value when available. Preserve the existing null and non-selected behavior without evaluating ValueProvider for presets that are not _selectedPreset.
🤖 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/DateRangePicker/BitDateRangePicker.razor`:
- Around line 302-305: Update the week-numbers header div in the ShowWeekNumbers
block of BitDateRangePicker to include the appropriate table header
accessibility role, matching the rowheader semantics already used by the
week-number cells.
- Around line 307-318: Update the weekday header rendering in BitDateRangePicker
so it does not index shortestDayOfWeekName with [0], which can split surrogate
pairs; render the complete shortest day name or extract its first Unicode text
element while preserving the existing culture-specific weekday labels.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs`:
- Around line 2229-2232: Update GetToday so its fallback derives the current
date and time using the configured TimeZone rather than DateTime.Now, while
preserving the existing GetDateTime(Today.Value) behavior when Today is
provided. Ensure the value used by the “today” cell and HandleGoToToday reflects
the configured zone.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.scss`:
- Around line 618-643: Update the .bit-dtrp-prb styles to add a visible
:focus-visible state for keyboard-focused preset buttons, while preserving the
existing transparent outline for non-keyboard focus and disabled-state behavior.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor`:
- Around line 313-314: In the BitDateRangePickerDemo markup, correct the label
text from “Defalt (local TimeZone):” to “Default (local TimeZone):” without
changing the surrounding markup.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DateRangePicker/BitDateRangePickerTests.cs`:
- Around line 1327-1339: Pin the culture in each affected test to make date,
day-name, separator, and digit assertions deterministic. In
BitDateRangePickerFirstDayOfWeekTest (lines 1327-1339), add
CultureInfo.InvariantCulture or derive the expected name from the active
culture; apply the same culture to the tests at BitDateRangePickerTests.cs lines
1485-1500, 2319-2332, and 2334-2353, ensuring the round-trip uses the culture
that builds the input; at lines 1264-1278, pin the culture or parse day text
with the matching culture.
---
Outside diff comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.scss`:
- Around line 403-412: Update the .bit-dtrp-dsb selected-range body styling to
use a dedicated range-body CSS custom property instead of the hardcoded
$clr-pri-light value. Define that custom property consistently in each
date-range-picker palette alongside the existing endpoint color variables, while
preserving the current hover behavior.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs`:
- Around line 1652-1662: Update IsPresetSelected to avoid invoking
preset.ValueProvider during render-time comparisons; retain the
ReferenceEquals(preset, _selectedPreset) fast path for provider-based presets,
and compare dates only against preset.Value when available. Preserve the
existing null and non-selected behavior without evaluating ValueProvider for
presets that are not _selectedPreset.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.cs`:
- Around line 136-141: Restore alphabetical ordering in the componentParameters
entries: move ExcludeDisabledDates after EndTimeIncreaseMinuteIconName, order
the On* entries as OnClick, OnFocus, OnFocusIn, OnFocusOut, OnMonthChange,
OnPresetSelect, and move ShowOutsideDays before ShowTimePicker. Preserve each
entry’s existing metadata and descriptions while changing only their list
positions.
- Around line 1839-1912: Remove the redundant StateHasChanged call from
HandleValidSubmit and rename the private SuccessMessage field to camelCase,
updating both references in BitDateRangePickerDemo.razor and the corresponding
sample string in BitDateRangePickerDemo.razor.samples.cs.
- Around line 1064-1084: Add documentation entries for the existing
BitDateRangePickerClassStyles properties ClearButton and ClearButtonIcon in the
daterangepicker-class-styles definition, matching the established Name, Type,
DefaultValue, and Description structure used by PresetsContainer and the other
class-style entries.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DateRangePicker/BitDateRangePickerTests.cs`:
- Around line 1974-1993: Rename
BitDateRangePickerDisabledPresetShouldNotBeSelectable to describe that a
read-only date range picker cannot select a preset. Add a separate test covering
a preset with IsEnabled set to false, verifying that selecting it does not
update the picker value.
🪄 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: ec309b25-13dd-4859-9153-9b964c6417a4
📒 Files selected for processing (12)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePickerClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePickerPreset.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Calendars.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/FormValidationDateRangePickerModel.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DateRangePicker/BitDateRangePickerTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.scss (1)
369-413: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the configured active color for all selected range days.
bit-dtrp-dss,bit-dtrp-dse, andbit-dtrp-dsbdo not define:activestyles. The generic.bit-dtrp-dbt:activerule then applies$clr-bg-pri-active. This bypasses--bit-dtrp-clr-activefor non-primary variants.Proposed fix
.bit-dtrp-dss { // existing declarations + &:active { + background-color: var(--bit-dtrp-clr-active); + } } .bit-dtrp-dse { // existing declarations + &:active { + background-color: var(--bit-dtrp-clr-active); + } } .bit-dtrp-dsb { // existing declarations + &:active { + background-color: var(--bit-dtrp-clr-active); + } }🤖 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/DateRangePicker/BitDateRangePicker.scss` around lines 369 - 413, Update the selected-range styles in .bit-dtrp-dss, .bit-dtrp-dse, and .bit-dtrp-dsb to define their :active background using the configured --bit-dtrp-clr-active value, preventing the generic .bit-dtrp-dbt:active rule from overriding it.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs (1)
2625-2657: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winInline styles are concatenated without a
;separator.
GetDayButtonCssappends severalStyles?.*values into onestylestring. The newHoveredDayButtonsblock appends with no separator, and the newHighlightedDayButtonblock appends a single space. A CSS declaration list needs;between declarations. When two of these style values apply to the same cell, the browser drops the merged declaration.The same pattern already exists in the surrounding blocks, so a small helper fixes every site at once.
♻️ Proposed refactor
+ private static void AppendStyle(StringBuilder style, string? value) + { + if (value.HasNoValue()) return; + + if (style.Length > 0 && style[^1] != ';') + { + style.Append(';'); + } + + style.Append(value); + }Then replace each
style.Append(Styles?.X)call inGetDayButtonCsswithAppendStyle(style, Styles?.X), for example:- if (Styles?.HoveredDayButtons is not null) - { - style.Append(Styles?.HoveredDayButtons); - } + AppendStyle(style, Styles?.HoveredDayButtons);🤖 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/DateRangePicker/BitDateRangePicker.razor.cs` around lines 2625 - 2657, Update GetDayButtonCss to append every Styles?.* value through a shared AppendStyle helper, ensuring each non-empty style fragment is separated by a semicolon before concatenation. Apply this consistently to the existing and newly added style blocks, including HoveredDayButtons and HighlightedDayButton, while preserving their current conditional behavior.
🤖 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/DateRangePicker/BitDateRangePicker.razor`:
- Around line 128-133: Update the callout container markup in BitDateRangePicker
so the dialog role is applied only when Standalone is false; omit the role for
inline standalone rendering while preserving the existing role for callout mode.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs`:
- Around line 995-1004: Extend the existing try block in OnAfterRenderAsync to
include the BitSwipesSetup call within the Responsive branch, ensuring both
BitCalendarsSetup and BitSwipesSetup are covered by the existing
JSDisconnectedException catch.
- Around line 1095-1103: Update TryParseRange to check
IsEmptyDateToken(startText) before calling TryParseDate, allowing a missing
start date to produce a null start value while preserving normal parsing for
non-empty input. Keep the existing end-date handling and range validation
behavior unchanged.
---
Outside diff comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.scss`:
- Around line 369-413: Update the selected-range styles in .bit-dtrp-dss,
.bit-dtrp-dse, and .bit-dtrp-dsb to define their :active background using the
configured --bit-dtrp-clr-active value, preventing the generic
.bit-dtrp-dbt:active rule from overriding it.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs`:
- Around line 2625-2657: Update GetDayButtonCss to append every Styles?.* value
through a shared AppendStyle helper, ensuring each non-empty style fragment is
separated by a semicolon before concatenation. Apply this consistently to the
existing and newly added style blocks, including HoveredDayButtons and
HighlightedDayButton, while preserving their current conditional behavior.
🪄 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: 25997153-48e1-4d29-9836-8cb72e7b2114
📒 Files selected for processing (12)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePickerClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePickerPreset.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Calendars.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/FormValidationDateRangePickerModel.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DateRangePicker/BitDateRangePickerTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/DateRangePicker/BitDateRangePicker.razor.cs`:
- Around line 1095-1118: Update TryParseRange to reject ranges where both
startDate and endDate remain null before constructing the
BitDateRangePickerValue; preserve parsing and date-order normalization when at
least one side contains a date.
- Around line 2655-2679: Update GetDayButtonCss in
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs
at lines 2655-2679 to terminate the style builder with a semicolon before
returning, preserving valid concatenation with Styles?.DayButton. In
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor
at lines 150-173, separate Styles?.PresetButton and preset.Style with a
semicolon instead of a space.
- Around line 1624-1656: Update SelectPreset to validate presetValue against the
same MinDate and MaxDate bounds enforced by SelectDate, and clamp its end date
with GetMaxEndDate when MaxRange applies before assigning CurrentValue. Reject
presets that cannot produce a valid range, while preserving the existing preset
selection, callback, and auto-close flow for valid values.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor`:
- Around line 61-66: Update the presets documentation in BitDateRangePickerDemo
to state that selecting a preset closes the callout only when AutoClose is true
and Standalone is false; remove the claim that preset selection always closes
it.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.scss`:
- Around line 37-40: Update the .custom-friday style in
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.scss#L37-L40
to use an accessible text/background color combination, then update the
displayed sample in
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.samples.cs#L101-L105
to match the same accessible style.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DateRangePicker/BitDateRangePickerTests.cs`:
- Around line 1247-1263: Pin the calendar date in both
BitDateRangePickerDisabledDatesTest (lines 1247-1263) and the highlighted-date
test (lines 1298-1311) by setting Today to the same fixed date, such as
FixedDate(2024, 6, 12), and derive disabledDate and highlightedDate from that
fixed value instead of DateTimeOffset.Now.Date.
🪄 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: d7e09b82-aee2-49ed-94ef-32b18b47757d
📒 Files selected for processing (12)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePickerClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePickerPreset.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Calendars.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/FormValidationDateRangePickerModel.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DateRangePicker/BitDateRangePickerTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor (1)
177-177: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider caching the focusable day and the per-day disabled state.
GetFocusableDay()scans every rendered cell and callsIsDayDisabledfor each one. Line 351 then callsIsDayDisabled(date)again for every rendered day. WithMonthCount3 and a customIsDateDisabledpredicate, the predicate runs a few hundred times per render. A single pass that records the disabled state per cell would remove the duplicated work.Also applies to: 340-369
🤖 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/DateRangePicker/BitDateRangePicker.razor` at line 177, Optimize the DateRangePicker render path around GetFocusableDay and the per-day rendering at lines 340-369 by caching each cell’s disabled state during the focusable-day scan and reusing it when rendering. Ensure custom IsDateDisabled predicates are evaluated at most once per rendered day while preserving the existing focusable-day and disabled-day behavior.src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePickerClassStyles.cs (1)
180-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the naming of the two new day-state slots.
HighlightedDayButtonis singular andHoveredDayButtonsis plural. Both slots apply to many day buttons. The existing slots use the plural form for multi-element states (SelectedDayButtons). Renaming one of them now avoids a breaking change later, because both properties are new in this PR.🤖 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/DateRangePicker/BitDateRangePickerClassStyles.cs` around lines 180 - 189, Rename the new HighlightedDayButton property in BitDateRangePickerClassStyles to the plural HighlightedDayButtons, matching HoveredDayButtons and the existing SelectedDayButtons naming convention. Update any references to the property while preserving its styling behavior.src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs (1)
1029-1038: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReset
_focusAfterRendereven when_focusedDateis null.The flag is cleared inside the
_focusedDate.HasValuebranch only. If a later code path sets_focusAfterRenderwithout a focused date, the condition stays true and is re-evaluated on every render. Clearing the flag first keeps the state self-consistent.♻️ Proposed refactor
- if (_focusAfterRender && _focusedDate.HasValue) + if (_focusAfterRender) { _focusAfterRender = false; + if (_focusedDate.HasValue is false) return; + try { await _js.BitCalendarsFocusDay(GetDayButtonId(_focusedDate.Value)); } catch (JSDisconnectedException) { } // we can ignore this exception here }🤖 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/DateRangePicker/BitDateRangePicker.razor.cs` around lines 1029 - 1038, Update the focus-after-render logic in the BitDateRangePicker render flow so _focusAfterRender is reset whenever it is set, before checking _focusedDate.HasValue. Only invoke BitCalendarsFocusDay when a focused date exists, while preserving the existing JSDisconnectedException handling.
🤖 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/DateRangePicker/BitDateRangePicker.scss`:
- Around line 1019-1024: Update the narrow-screen styles around `.bit-dtrp-grp`
and `.bit-dtrp-prc` so the parent layout wraps while the presets use a full-row
flex basis, allowing them to stack beneath the calendar as a mobile row. Adjust
the divider styling to match the stacked layout, while preserving the preset
button wrapping behavior.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor`:
- Around line 391-394: Update the parameter description near YearCellTemplate to
state that DayCellTemplate and MonthCellTemplate receive the cell date, while
YearCellTemplate receives an int year value; preserve the existing customization
examples.
In
`@src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DateRangePicker/BitDateRangePickerTests.cs`:
- Around line 1240-1244: Pin the date-picker culture or FirstDayOfWeek in both
affected tests in BitDateRangePickerTests.cs: lines 1240-1244 and 2306-2310.
Update each test’s parameter setup to use a deterministic setting, such as
CultureInfo.InvariantCulture, so the fixed grid-index assertions remain valid
across ambient cultures.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor`:
- Line 177: Optimize the DateRangePicker render path around GetFocusableDay and
the per-day rendering at lines 340-369 by caching each cell’s disabled state
during the focusable-day scan and reusing it when rendering. Ensure custom
IsDateDisabled predicates are evaluated at most once per rendered day while
preserving the existing focusable-day and disabled-day behavior.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs`:
- Around line 1029-1038: Update the focus-after-render logic in the
BitDateRangePicker render flow so _focusAfterRender is reset whenever it is set,
before checking _focusedDate.HasValue. Only invoke BitCalendarsFocusDay when a
focused date exists, while preserving the existing JSDisconnectedException
handling.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePickerClassStyles.cs`:
- Around line 180-189: Rename the new HighlightedDayButton property in
BitDateRangePickerClassStyles to the plural HighlightedDayButtons, matching
HoveredDayButtons and the existing SelectedDayButtons naming convention. Update
any references to the property while preserving its styling behavior.
🪄 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: 6e0048c7-5b50-4c76-8fcf-61674ad718fa
📒 Files selected for processing (12)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.scsssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePickerClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePickerPreset.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Calendars.tssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/BitDateRangePickerDemo.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Inputs/DateRangePicker/FormValidationDateRangePickerModel.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/DateRangePicker/BitDateRangePickerTests.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 #12867
Summary by CodeRabbit
New Features
Bug Fixes