feat(components): add Datebox component#250
Conversation
📝 WalkthroughWalkthroughAdds a new generic LumexDatebox component with multi-type support and formatting/parsing, introduces InputDateType enum, restructures input base classes to support debounced behavior, updates Numbox/Textbox inheritance, adds comprehensive docs/examples/previews, and includes a BUnit test suite and navigation entries. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
desmondinho
changed the title
|
Should i rename the component name ? |
![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 (7)
docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Colors.razor (1)
6-6: Stray literal "6" will render on the page. Remove it.-6 +docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ClearButton.razor (1)
6-6: Stray literal "6" will render on the page. Remove it.-6 +docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Radius.razor (1)
6-6: Stray literal "6" will render on the page. Remove it.-6 +docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Disabled.razor (1)
6-6: Stray literal "6" will render on the page. Remove it.-6 +docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/StartEndContent.razor (1)
6-6: Stray literal "6" will render on the page. Remove it.-6 +docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/TwoWayDataBinding.razor (1)
6-6: Stray literal "6" will render on the page. Remove it.-6 +docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/LabelPlacements.razor (1)
6-6: Stray literal "6" will render on the page. Remove it.-6 +
🧹 Nitpick comments (21)
src/LumexUI/Common/Enums/InputDateType.cs (2)
17-17: Fix spacing in Description attribute.There's an extra space before the opening quote in the Description attribute.
- [Description( "date" )] + [Description("date")]
9-18: Consider future extensibility.The enum currently only contains a
Datemember. Since this is for a date input component, consider if you'll need additional date/time input types in the future (likeDateTime,DateTimeLocal,Time, etc.) to match HTML5 input types.docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/CustomStyles.razor (1)
1-5: Use date-oriented label/placeholder for clarity.This is a DateInput example, but it reads like a search box. Aligning the copy will reduce reader confusion.
-<LumexDateInput Label="Search" - Placeholder="Type to search..." +<LumexDateInput Label="Date" + Placeholder="dd/MM/yyyy" Radius="@Radius.Large" Clearable="@true" Classes="@_classes">Optional: swap SearchIcon for a calendar-related icon if available.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/DebounceDelay.razor (1)
2-12: Clarify semantics and format the displayed value.Use a date-focused label/placeholder and show a formatted value for readability.
- <LumexDateInput DebounceDelay="250" - Behavior="@InputBehavior.OnInput" - Label="Search" - Placeholder="Type to search..." + <LumexDateInput DebounceDelay="250" + Behavior="@InputBehavior.OnInput" + Label="Date" + Placeholder="dd/MM/yyyy" Clearable="@true" Class="max-w-xs" @bind-Value="@_value" /> @@ - <p class="text-sm text-default-500"> - Value: @_value - </p> + <p class="text-sm text-default-500"> + Value: @_value?.ToString("dd/MM/yyyy") ?? "—" + </p>docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/ClearButton.razor (3)
1-9: Prefer @bind-Value and conditionally render feedback for better UX/a11y.
- Bind the value to keep parent state in sync with user picks.
- Only render the message when present and announce it to AT.
Apply:
- <LumexDateInput Clearable="@true" + <LumexDateInput Clearable="@true" Label="Date" Placeholder="dd/MM/yyyy" - Value="@_value" + @bind-Value="_value" Variant="@InputVariant.Outlined" OnCleared="@Notify" Class="max-w-xs" />
11-13: Avoid empty paragraph; announce status to screen readers.-<p class="text-sm text-default-500"> - @_text -</p> +@if (!string.IsNullOrEmpty(_text)) +{ + <p class="text-sm text-default-500" role="status" aria-live="polite"> + @_text + </p> +}
19-27: Simplify Notify().- private void Notify() - { - _value = null; - - if (_value is null) - { - _text = "Input is cleared!"; - } - } + private void Notify() + { + _value = null; + _text = "Input is cleared!"; + }docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.cs (1)
35-35: Docs & API pages exist; gate LumexDateInput autofocus.
- Verified: docs/LumexUI.Docs.Client/Pages/Components/DateInput/DateInput.razor has @page "/docs/components/date-input" and docs/LumexUI.Docs.Client/Pages/Api/Api.razor has @page "/docs/api/{componentName}".
- Autofocus already exists on the base class (src/LumexUI/Components/Bases/LumexInputFieldBase.razor.cs — [Parameter] public bool Autofocus { get; set; } and handling).
- Action: update src/LumexUI/Components/DateInput/LumexDateInput.razor.cs (where it currently does
Focused = true;at ~line 53) to respect the base Autofocus instead of forcing focus. Example change:
if (Autofocus && !Disabled && !ReadOnly) { Focused = true; }docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/LabelPlacements.razor (1)
9-12: Placeholder on native date inputs is ignored in most browsers; remove to avoid confusion.- <LumexDateInput LabelPlacement="@placement" - Label="Date" - Placeholder="dd/MM/yyyy" /> + <LumexDateInput LabelPlacement="@placement" + Label="Date" />docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/StartEndContent.razor (2)
6-6: Use the control’s actual format and avoid placeholders that browsers ignore for type="date".Most browsers ignore Placeholder on date inputs; also your component parses “yyyy-MM-dd”. Suggest aligning.
- Placeholder="dd/MM/yyyy" + Placeholder="yyyy-MM-dd"Optional: drop Placeholder entirely for date inputs to avoid misleading UX.
Also applies to: 11-11, 16-16, 31-31, 36-36, 41-41, 56-56, 62-62, 68-68
5-19: Example content is text-field themed (protocols/domains), not date-themed.These adornments read like textbox examples and may confuse readers for a date input doc. Consider calendar/today icons, or textual cues like “UTC”/“Local”.
Also applies to: 30-44, 55-71
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/ErrorMessage.razor (1)
18-22: Start invalid to demonstrate the error message immediately.Setting a value on init hides the validation state. Consider starting with null.
- _user.Date = DateTime.UtcNow; - Validate(); + _user.Date = null; + Validate();src/LumexUI/Components/DateInput/LumexDateInput.razor.cs (4)
20-23: Fix doc comment grammar.- /// Gets or sets the date input type of the. + /// Gets or sets the input date type.
28-36: Don’t auto-focus by default; gate behind an explicit parameter.Auto-setting Focused on every parameter set can cause surprising focus jumps. Make it opt‑in.
@@ - [Parameter] - public string? ParsingErrorMessage { get; set; } + [Parameter] + public string? ParsingErrorMessage { get; set; } + + /// <summary> + /// When true, the input requests focus on first render (if not disabled/read-only). + /// </summary> + [Parameter] + public bool AutoFocus { get; set; } @@ - if( !Disabled && !ReadOnly ) - { - Focused = true; - } + if( AutoFocus && !Disabled && !ReadOnly && !Focused ) + { + Focused = true; + }Also applies to: 51-54
60-73: Trim whitespace before parsing for resilience.- if( string.IsNullOrWhiteSpace( value ) ) + if( string.IsNullOrWhiteSpace( value ) ) { result = null; return true; } - if( DateTime.TryParseExact( + value = value!.Trim(); + if( DateTime.TryParseExact( value, DateFormat, CultureInfo.InvariantCulture, DateTimeStyles.None, out DateTime dt ) )
16-18: Consider DateOnly for future-proofing.If/when you add a DateOnly path, you can avoid time-zone and Kind ambiguity. Not a request for this PR.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/DateInput.razor (1)
79-83: Optional doc note about HTML date format.Consider a short callout that HTML date inputs use “yyyy-MM-dd” regardless of locale; your component aligns to that.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Required.razor (1)
1-4: Standardize boolean attribute style across docsElsewhere you use Disabled="true" while here it's Required="@true". Pick one style for consistency; suggest Required="true".
-<LumexDateInput Required="@true" +<LumexDateInput Required="true" Label="Date" Placeholder="dd/MM/yyyy" Class="max-w-xs" />docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Variants.razor (2)
6-11: Add @key for stable diffing in foreachHelps Blazor reconcile elements if the list changes.
- <div> + <div @key="variant"> <div class="w-full flex flex-wrap gap-4 md:flex-nowrap"> <LumexDateInput Variant="@variant" Label="date" /> <LumexDateInput Variant="@variant" Label="Date" Placeholder="dd/MM/yyyy" />
6-11: Unify label casingMixing "date" and "Date" looks accidental in docs.
- <LumexDateInput Variant="@variant" - Label="date" /> + <LumexDateInput Variant="@variant" + Label="Date" />docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Sizes.razor (1)
6-11: Add @key for stable diffing in foreachSame rationale as Variants.
- <div> + <div @key="size"> <div class="w-full flex flex-wrap gap-4 md:flex-nowrap"> <LumexDateInput Size="@size" Label="Date" /> <LumexDateInput Size="@size" Label="Date" Placeholder="dd/MM/yyyy" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.cs(2 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/DateInput.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/ClearButton.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Colors.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/CustomStyles.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/DebounceDelay.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Description.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Disabled.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/ErrorMessage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/LabelPlacements.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/ReadOnly.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Required.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Sizes.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/StartEndContent.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/TwoWayDataBinding.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Usage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Variants.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/_Radius.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ClearButton.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Colors.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/CustomStyles.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/DebounceDelay.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Description.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Disabled.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ErrorMessage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/LabelPlacements.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Radius.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ReadOnly.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Required.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Sizes.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/StartEndContent.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/TwoWayDataBinding.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Usage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Variants.razor(1 hunks)src/LumexUI/Common/Enums/InputDateType.cs(1 hunks)src/LumexUI/Components/DateInput/LumexDateInput.razor(1 hunks)src/LumexUI/Components/DateInput/LumexDateInput.razor.cs(1 hunks)tests/LumexUI.Tests/Components/DateInput/DateInputTests.razor(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.cs (1)
src/LumexUI/Components/DateInput/LumexDateInput.razor.cs (1)
LumexDateInput(14-88)
src/LumexUI/Components/DateInput/LumexDateInput.razor.cs (1)
src/LumexUI/Components/Bases/LumexInputFieldBase.razor.cs (1)
SetInputType(265-268)
🔇 Additional comments (26)
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/_Radius.razor (2)
5-7: Effective radius demonstration.Using the radius value as the placeholder (
@radius.ToString()) provides a clear visual connection between the property and its effect.
17-22: Complete radius coverage.Good coverage of all standard radius options (None, Small, Medium, Large) for comprehensive styling documentation.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/TwoWayDataBinding.razor (3)
3-6: Clear demonstration of automatic two-way binding.The
@bind-Valuesyntax clearly demonstrates automatic two-way data binding with proper nullable DateTime support.
13-17: Effective manual two-way binding example.Shows the manual approach using separate
ValueandValueChangedparameters, providing developers with both binding patterns.
25-26: Good initialization patterns.Demonstrates both null initialization (
_valueOne) and preset value initialization (_valueTwo = DateTime.UtcNow) to show different use cases.docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/ReadOnly.razor (1)
1-4: LGTM.Good minimal example for ReadOnly.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Usage.razor (1)
1-6: LGTM.Clear, simple starter usage.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ReadOnly.razor (1)
1-5: LGTM.Preview wrapper follows the established pattern.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Usage.razor (1)
1-5: LGTM.Consistent with other PreviewCode wrappers.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Sizes.razor (1)
1-5: LGTM.Matches existing docs conventions.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ErrorMessage.razor (2)
1-5: LGTM: preview wiring and snippet key look consistent.
3-5: Verify DateInput snippet keys in docsSandbox search returned "No files were searched" so I couldn't confirm the snippet keys. Affected file: docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ErrorMessage.razor (lines 3–5). Run locally to verify:
git grep -nE 'DateInput.Code.(ErrorMessage|Variants|DebounceDelay|Required|Description|LabelPlacements|ClearButton)' || rg -nP 'DateInput.Code.(ErrorMessage|Variants|DebounceDelay|Required|Description|LabelPlacements|ClearButton)' docs
docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Variants.razor (1)
1-5: LGTM: matches the established PreviewCode pattern.docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/DebounceDelay.razor (1)
1-5: LGTM: consistent with other previews; nothing blocking.docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/LabelPlacements.razor (1)
22-26: Confirm C# 12 collection-expression compatibility (LangVersion/TFM)File: docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/LabelPlacements.razor (lines 22–26)
This file uses a C# 12 collection expression: LabelPlacement[] _labelPlacements = [ LabelPlacement.Inside, LabelPlacement.Outside ]. (learn.microsoft.com)
Ensure the project targets .NET 8+ or explicitly sets 12.0 (or preview/default) in the .csproj / Directory.Build.props / .editorconfig; otherwise this syntax may not compile. (learn.microsoft.com)
Could not verify repo project files due to script failures — confirm and update the project's TargetFramework/LangVersion settings.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Required.razor (1)
1-5: LGTM: preview registration matches examples.docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Description.razor (1)
1-5: LGTM: consistent wiring with the Description example.docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Radius.razor (1)
3-5: Underscore naming is consistent — no change required. Search shows DateInput and multiple other PreviewCodes/Radius.razor files (Popover, Numbox, Select, Tabs, Textbox, CheckboxGroup, Checkbox, DataGrid) use "Component.Code._Radius" and Examples._Radius; keep as-is.docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/CustomStyles.razor (1)
3-6: LGTM — preview wiring and classes look correct.Nothing blocking here. The Tailwind arbitrary property for mask-image is fine assuming it’s enabled in the docs setup.
If Tailwind’s arbitrary properties are restricted, confirm
[mask-image:none]is permitted by your config.src/LumexUI/Components/DateInput/LumexDateInput.razor.cs (1)
34-50: Confirm the parse error text is surfaced to users._parsingErrorText is set but not referenced here; ensure the base class reads it when TryParse fails, or expose it via an override/hook if needed.
tests/LumexUI.Tests/Components/DateInput/DateInputTests.razor (2)
201-223: Verify expectations for date-only semantics.These tests compare against second-truncated DateTime values; with type="date" the value typically normalizes to 00:00:00 without Kind. Make sure bUnit’s helpers and the component agree; otherwise assert against the date part only.
Also applies to: 225-249
296-306: Good guard: debounce with OnChange should throw.Nice coverage of the invalid combo.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Disabled.razor (1)
1-4: LGTMGood minimal disabled example with clear label and placeholder.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Variants.razor (1)
22-26: C# 12 collection expressions allowed — no change needed.
Projects target net8.0/net9.0 (e.g., src/LumexUI.csproj TargetFrameworks=net8.0;net9.0 and docs/LumexUI.Docs.Client.csproj TargetFramework=net9.0), so the [...] collection-expression initializer is supported; keep as-is.docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Sizes.razor (1)
20-24: C# collection-expression usage is fine — no change requireddocs/LumexUI.Docs.Client targets net9.0 (default C# 13) and src targets net8.0/net9.0 (C# 12+); the [...] array/collection initializer is supported — keep as-is.
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Colors.razor (1)
1-15: Search for Value attributes missing leading @ in Razor examples
Sandbox verification failed; run a repo-wide grep for Value="..." in .razor files and replace any occurrences missing a leading @ (e.g., Value="DateTime.UtcNow" → Value="@DateTime.UtcNow").
docs/LumexUI.Docs.Client/Pages/Components/DateInput/DateInput.razor
Outdated
Show resolved
Hide resolved
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Colors.razor
Outdated
Show resolved
Hide resolved
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Description.razor
Outdated
Show resolved
Hide resolved
docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/ErrorMessage.razor
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ClearButton.razor (4)
1-1: Confirm render mode choiceIf the docs app is SSR/Auto elsewhere, consider
InteractiveAutoto inherit the host’s mode; otherwiseInteractiveWebAssemblyhere is fine.
3-3: Target-typednewin attribute – verify the target type
Code="@new(...)"relies onPreviewCode.Codebeing a non-ambiguous type with a matching ctor. If repo analyzers complain or you want clearer intent, switch to an explicit type.
4-4: Avoid fully-qualified component name (readability)Import the namespace and use the short tag.
-@rendermode InteractiveWebAssembly +@rendermode InteractiveWebAssembly +@using LumexUI.Docs.Client.Pages.Components.DateInput.Examples @@ - <LumexUI.Docs.Client.Pages.Components.DateInput.Examples.ClearButton /> + <ClearButton />
3-5: Naming consistency: Datebox vs DateInputPR title says “Datebox” while docs/snippet keys use “DateInput”. Recommend standardizing on one (e.g., “DateInput” for an input wrapper; reserve “DatePicker” if there’s a calendar popover).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
docs/LumexUI.Docs.Client/Pages/Components/DateInput/DateInput.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Colors.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Description.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/ErrorMessage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ClearButton.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Colors.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/DebounceDelay.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Description.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Disabled.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ErrorMessage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/LabelPlacements.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Radius.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ReadOnly.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Required.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Sizes.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/StartEndContent.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/TwoWayDataBinding.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Usage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Variants.razor(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Description.razor
🚧 Files skipped from review as they are similar to previous changes (17)
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Disabled.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/TwoWayDataBinding.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Variants.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ErrorMessage.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/DateInput.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Description.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ReadOnly.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Radius.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Sizes.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Required.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/ErrorMessage.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/StartEndContent.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/Examples/Colors.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Colors.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/Usage.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/DebounceDelay.razor
- docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/LabelPlacements.razor
🔇 Additional comments (1)
docs/LumexUI.Docs.Client/Pages/Components/DateInput/PreviewCodes/ClearButton.razor (1)
3-5: LGTM: preview wiringThe preview correctly binds the snippet key and renders the example component.
Hey @sikelio, Yep, that would be great! Thank you :) |
|
Hey @desmondinho |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/LumexUI.Tests/Components/Bases/InputBaseTests.cs (1)
200-216: Make parsing culture-invariant and format-consistent.Format uses "yyyy/MM/dd" with InvariantCulture, but parsing uses DateTime.TryParse (culture-dependent). This can be flaky across environments.
Apply:
- protected override bool TryParseValueFromString( string? value, out DateTime result ) - { - if( DateTime.TryParse( value, out result ) ) - { - return true; - } - else - { - return false; - } - } + protected override bool TryParseValueFromString( string? value, out DateTime result ) + { + return DateTime.TryParseExact( + value, + "yyyy/MM/dd", + CultureInfo.InvariantCulture, + DateTimeStyles.None, + out result ); + }
🧹 Nitpick comments (24)
docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Required.razor (2)
1-1: Remove stray BOM/ZWNBSP before @rendermode and confirm render mode usageThere’s a visible zero‑width char before @rendermode. Also verify this component-level render mode aligns with the docs app’s global setting to avoid redundancy.
-@rendermode InteractiveWebAssembly +@rendermode InteractiveWebAssembly
3-5: Verify snippet key and namespace-qualified tag usage
- Ensure the snippet key "Datebox.Code.Required" exists in the snippets source.
- Using a fully qualified component tag is valid but noisy; consider adding an @using and a short tag for consistency with other docs.
Optional refactor:
-<PreviewCode Code="@new(name: null, snippet: "Datebox.Code.Required")"> - <LumexUI.Docs.Client.Pages.Components.Datebox.Examples.Required /> +@using LumexUI.Docs.Client.Pages.Components.Datebox.Examples +<PreviewCode Code="@new(name: null, snippet: "Datebox.Code.Required")"> + <Required /> </PreviewCode>tests/LumexUI.Tests/Components/Bases/InputBaseTests.cs (1)
80-107: Optional: lock test culture to avoid locale-dependent behavior.Even with TryParseExact, other tests may rely on current culture. Consider scoping CultureInfo.CurrentCulture/UICulture to InvariantCulture within these date-specific tests.
Example snippet (wrap per-test):
var prev = (CultureInfo)CultureInfo.CurrentCulture.Clone(); try { CultureInfo.CurrentCulture = CultureInfo.InvariantCulture; CultureInfo.CurrentUICulture = CultureInfo.InvariantCulture; // ... test body ... } finally { CultureInfo.CurrentCulture = prev; CultureInfo.CurrentUICulture = prev; }Also applies to: 113-124
src/LumexUI/Common/Enums/InputDateboxType.cs (1)
9-19: Docs polish and cref resolution.
- Prefer “subtype” (one word) and end sentence with a period.
- Ensure resolves; if not, fully-qualify the type or add the appropriate using.
Apply:
- /// A date box sub-type for selecting a date + /// A date box subtype for selecting a date.Optional (future-proof diffs):
- Date + Date,docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ClearButton.razor (2)
2-9: Use two-way binding and simplify attributes for a clearer example.Demonstrate recommended binding and streamline Clearable usage.
Apply:
- <LumexDatebox Clearable="@true" + <LumexDatebox Clearable Label="Date" Placeholder="dd/MM/yyyy" - Value="@_value" + @bind-Value="_value" Variant="@InputVariant.Outlined" OnCleared="@Notify" Class="max-w-xs" />Note: If the component’s default display format differs, consider aligning the placeholder accordingly.
19-27: Simplify OnCleared handler.The null check is redundant after setting _value to null (and with @Bind, you often don’t need to set it).
Apply:
- private void Notify() - { - _value = null; - - if (_value is null) - { - _text = "Input is cleared!"; - } - } + private void Notify() + { + _value = null; // optional if @bind already cleared + _text = "Input is cleared!"; + }docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Colors.razor (2)
5-8: Use local date to avoid timezone surprises.Using UtcNow can show “yesterday/tomorrow” depending on viewer locale. Prefer Today for date-only inputs.
Apply:
- Value="@DateTime.UtcNow" /> + Value="@DateTime.Today" />
7-8: Placeholder on input type="date" is typically ignored by browsers.If LumexDatebox renders a native date input, consider removing Placeholder or moving the hint to Description/helper text to avoid confusion.
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Variants.razor (2)
9-12: Prefer helper text over placeholder for date inputs.If native date inputs are used, placeholder won’t render. Consider a Description hint instead.
- <LumexDatebox Variant="@variant" - Label="Date" - Placeholder="dd/MM/yyyy" /> + <LumexDatebox Variant="@variant" + Label="Date" + Description="Format: dd/MM/yyyy" />
6-7: Consistent label casing (nit).Use consistent “Date” casing unless intentionally demonstrating label text differences.
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/_Radius.razor (2)
2-7: Rename radiuses → radii and show a concrete date.
- “radii” is the conventional plural.
- Supplying a value helps visualize radius when placeholders are ignored by date inputs.
- @foreach (Radius radius in _radiuses) + @foreach (Radius radius in _radii) { <div> <LumexDatebox Radius="@radius" Label="Date" - Placeholder="@radius.ToString()" /> + Placeholder="@radius.ToString()" + Value="@DateTime.Today" /> @@ - private Radius[] _radiuses = [ + private Radius[] _radii = [Also applies to: 16-23
6-7: Placeholder caveat.If LumexDatebox uses native , the placeholder won’t render; consider using Description or leaving it empty.
docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/CustomStyles.razor (1)
3-6: Preview wrapper reads clean; minor naming nitConsider renaming the backing field to clarify scope, then update the reference.
-<PreviewCode Code="@new(name: null, snippet: "Datebox.Code.CustomStyles")" - PreviewClasses="@_classes"> +<PreviewCode Code="@new(name: null, snippet: "Datebox.Code.CustomStyles")" + PreviewClasses="@_previewClasses">- private Preview.Slots _classes = new() + private Preview.Slots _previewClasses = new()docs/LumexUI.Docs.Client/Pages/Components/Datebox/Datebox.razor (1)
145-146: Unify terminology: “Date box” → “Datebox”Keep the component/page naming consistent with the type name LumexDatebox.
- <h4 class="font-semibold">Date box</h4> + <h4 class="font-semibold">Datebox</h4>- title: "Date box", + title: "Datebox", - description: "Date box allows users to input a specific date.", + description: "Datebox allows users to input a specific date.",Also applies to: 210-215
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/CustomStyles.razor (1)
1-9: Example semantics don’t match a date inputLabel/placeholder/icon read like a search box. Use date-oriented copy and icon to avoid confusing users.
-<LumexDatebox Label="Search" - Placeholder="Type to search..." +<LumexDatebox Label="Date" + Placeholder="yyyy-MM-dd" Radius="@Radius.Large" Clearable="@true" Classes="@_classes"> <StartContent> - <SearchIcon Size="18" class="text-secondary-400 shrink-0" /> + <CalendarIcon Size="18" class="text-secondary-400 shrink-0" /> </StartContent> </LumexDatebox>docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/TwoWayDataBinding.razor (1)
7-9: Show formatted values to avoid timezone/noise confusionDisplaying raw DateTime can be noisy. Consider formatting for clarity.
- <p class="text-sm text-default-500"> - Value: @_valueOne - </p> + <p class="text-sm text-default-500"> + Value: @_valueOne?.ToString("yyyy-MM-dd") + </p>- <p class="text-sm text-default-500"> - Value: @_valueTwo - </p> + <p class="text-sm text-default-500"> + Value: @_valueTwo?.ToString("yyyy-MM-dd") + </p>Also applies to: 18-20
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ErrorMessage.razor (1)
47-61: Limit example types’ visibilityThese are example-only types; avoid polluting the docs assembly public surface.
- public class User + private sealed class User { public DateTime? Date { get; set; } } - public class UserValidator : AbstractValidator<User> + private sealed class UserValidator : AbstractValidator<User> { public string? DateErrorMessage { get; set; }tests/LumexUI.Tests/Components/Datebox/DateboxTests.razor (1)
111-113: Prefer a more specific selector for the clear buttonUse the clear button slot if exposed to avoid false positives when other buttons exist.
-IElement clearButton = cut.Find("[role=button]"); +IElement clearButton = cut.Find("[data-slot=clear-button]");Apply similarly in the other tests.
Also applies to: 124-128, 143-151, 165-171
src/LumexUI/Components/Datebox/LumexDatebox.razor.cs (5)
20-24: Fix the XML doc grammar for DateboxType.“Gets or sets the date box type of the.” is incomplete.
Apply:
- /// Gets or sets the date box type of the. + /// Gets or sets the input mode for the datebox.
47-50: Ensure the parsing error message flows into validation UI.Right now
_parsingErrorTextis a private field. Verify that either:
- the .razor markup binds to
_parsingErrorTextwhen parse fails, orLumexInputFieldBase<T>consumes aParsingErrorMessage-like member for validation messages and you’re wiring this value into that pipeline.If the base exposes a message property, prefer surfacing it there so EditContext/ValidationMessageStore picks it up instead of holding it in a private field.
68-77: Parsing path looks good; consider DateOnly future-proofing.
TryParseExact("yyyy-MM-dd", InvariantCulture, None)is correct for<input type="date">. Optionally plan for aDateOnly-backed variant to avoid accidental time semantics when you add other modes.
14-14: Component naming follow-up.Per PR comments, if the project prefers “DateInput” over “Datebox”, consider renaming the component and files to
LumexDateInputfor consistency, and update docs/tests/routes accordingly.
17-18: Localize the default parsing error text.
"The {0} field must be a date."should come from resources to support i18n and consistent wording across components.I can extract this into a shared resources file and wire it up—want me to draft that?
docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Description.razor (1)
1-1: Remove BOM (invisible char) at file start.There’s a UTF‑8 BOM before the
@rendermodedirective; drop it to avoid diff noise and tooling hiccups.-@rendermode InteractiveWebAssembly +@rendermode InteractiveWebAssembly
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.cs(2 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Datebox.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ClearButton.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Colors.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/CustomStyles.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/DebounceDelay.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Description.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Disabled.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ErrorMessage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/LabelPlacements.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ReadOnly.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Required.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Sizes.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/StartEndContent.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/TwoWayDataBinding.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Usage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Variants.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/_Radius.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/ClearButton.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Colors.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/CustomStyles.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/DebounceDelay.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Description.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Disabled.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/ErrorMessage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/LabelPlacements.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Radius.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/ReadOnly.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Required.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Sizes.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/StartEndContent.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/TwoWayDataBinding.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Usage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Variants.razor(1 hunks)src/LumexUI/Common/Enums/InputDateboxType.cs(1 hunks)src/LumexUI/Components/Datebox/LumexDatebox.razor(1 hunks)src/LumexUI/Components/Datebox/LumexDatebox.razor.cs(1 hunks)tests/LumexUI.Tests/Components/Bases/InputBaseTests.cs(5 hunks)tests/LumexUI.Tests/Components/Datebox/DateboxTests.razor(1 hunks)
✅ Files skipped from review due to trivial changes (12)
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/LabelPlacements.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ReadOnly.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Sizes.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Disabled.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/TwoWayDataBinding.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Required.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Variants.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/ErrorMessage.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/DebounceDelay.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Usage.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Usage.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Description.razor
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.cs
🧰 Additional context used
🧬 Code graph analysis (1)
src/LumexUI/Components/Datebox/LumexDatebox.razor.cs (2)
src/LumexUI/Components/Bases/LumexInputFieldBase.razor.cs (1)
SetInputType(265-268)tests/LumexUI.Tests/Components/Bases/InputBaseTests.cs (3)
TryParseValueFromString(194-197)TryParseValueFromString(205-215)FormatValueAsString(202-203)
🔇 Additional comments (24)
docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Sizes.razor (1)
1-5: LGTM — preview wrapper matches existing pattern.Consistent usage of @rendermode and PreviewCode; no issues spotted.
docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/ReadOnly.razor (1)
1-5: LGTM — consistent PreviewCode setup.Mirrors other Datebox previews; nothing to change.
docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/DebounceDelay.razor (1)
1-5: LGTM — PreviewCode wiring looks correct.docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/LabelPlacements.razor (1)
1-5: LGTM — matches the established docs pattern.docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Disabled.razor (1)
1-5: LGTM — straightforward preview wrapper.tests/LumexUI.Tests/Components/Bases/InputBaseTests.cs (1)
1-1: Repo-wide rename sanity check — manual verification requiredSince we’re standardizing on "Datebox", verify no lingering "LumexDateInput" / "TestDateInputComponent" references remain. My automated search skipped files so I couldn't confirm — from the repo root run and paste the output:
rg -nP --hidden --no-ignore '\b(LumexDateInput|TestDateInputComponent)\b' -g '!bin/' -g '!obj/' or confirm there are no matches.docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Radius.razor (1)
3-5: Preview wiring looks correct.The snippet key and example component reference align with the pattern used elsewhere. No issues.
docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Colors.razor (1)
3-5: Preview wiring looks correct.Consistent with other PreviewCode wrappers.
docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/ClearButton.razor (1)
3-5: Preview wiring looks correct.Consistent with the established Datebox docs pattern.
src/LumexUI/Components/Datebox/LumexDatebox.razor (2)
4-6: Delegating rendering to base is appropriate here.This ensures the base input field markup is used while logic lives in the code-behind.
1-3: Naming consistent — use "LumexDatebox"; update PR description
Repository search found only "LumexDatebox"/"Datebox" in code, tests, and docs; no occurrences of "LumexDateInput" or "DateInput". Update the PR description to match.docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/StartEndContent.razor (1)
3-5: Preview wiring looks correct — snippet key existsDatebox.Code.StartEndContent is referenced in docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/StartEndContent.razor; Datebox.Code.Colors, Datebox.Code.ClearButton and Datebox.Code._Radius are present.
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Colors.razor (1)
18-26: OK — C# 12 collection expression is allowed (docs target net9.0)Docs projects target net9.0 (default language version ≥ C# 12 — net9 defaults to C# 13), so the collection-expression array syntax is supported; no change required. (learn.microsoft.com)
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Variants.razor (1)
22-26: C# 12 collection expression — OK (no action required). Project LumexUI.Docs.Client targets net9.0 (LumexUI.Docs.Client.csproj), whose default language version is C# 13, so C# 12 features like collection expressions are supported. (learn.microsoft.com)docs/LumexUI.Docs.Client/Pages/Components/Datebox/Datebox.razor (2)
80-84: Verify default LabelPlacement behaviorDocs state LabelPlacement defaults to Outside when Label is unset; my repo search for "LabelPlacement" in the Datebox component returned no matches. Confirm the component enforces LabelPlacement = Outside when Label is null/empty, or update the docs. Locations: docs/LumexUI.Docs.Client/Pages/Components/Datebox/Datebox.razor (lines 80–84); check implementation in src/LumexUI/Components/Datebox/LumexDatebox.razor(.cs).
1-1: Incorrect — repo uses "LumexDatebox" (no "LumexDateInput" found).
Search shows only LumexDatebox in src, docs, and tests; update PR text to use "Datebox / LumexDatebox" or rename the component and update code/tests/docs/navigation accordingly. Locations: src/LumexUI/Components/Datebox/LumexDatebox.razor(.cs), docs/LumexUI.Docs.Client/Pages/Components/Datebox/Datebox.razor, tests/LumexUI.Tests/Components/Datebox/DateboxTests.razor, docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.csLikely an incorrect or invalid review comment.
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/StartEndContent.razor (1)
5-19: Replace URL/email adornments with date-appropriate content and don’t rely on native placeholderExamples use protocol/domain/email fragments for date inputs — replace StartContent/EndContent with a calendar icon or date hints (e.g., “Today”, timezone label), and use a format hint like yyyy‑MM‑dd in helper text (not the native placeholder) because most browsers ignore placeholder for input[type="date"]. Verify whether LumexDatebox renders a native input[type="date"] or a custom text input and adjust the examples accordingly.
File: docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/StartEndContent.razor (also applies to lines 30–44, 55–71, 82–99).
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/CustomStyles.razor (1)
16-23: Confirmed — group-data-[focus=true] will trigger.
InputField._base includes the "group" class (src/LumexUI/Styles/InputField.cs) and LumexInputFieldBase renders data-focus on the base wrapper (src/LumexUI/Components/Bases/LumexInputFieldBase.razor); InputWrapper is nested inside that base so the group-data selector will apply.src/LumexUI/Components/Datebox/LumexDatebox.razor.cs (4)
39-44: Good guard against unsupported modes.Throwing for non-Date modes is clear and prevents silent misbehavior while the component evolves.
45-45: Correct input type for HTML date controls.Setting type="date" via SetInputType is appropriate.
60-67: Empty input → null is correct.Returning true with
result = nullfor empty/whitespace aligns with nullable Date semantics.
84-87: Formatting matches the HTML date value contract.Formatting with
"yyyy-MM-dd"and InvariantCulture is correct.docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Description.razor (2)
3-5: LGTM on structure and placement.Wrapper + example pattern matches other PreviewCodes; no functional concerns here.
1-1: Keep InteractiveWebAssembly — matches docs convention.
Search found 297 explicit @rendermode InteractiveWebAssembly instances; this file (docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Description.razor) uses the same — no change required.
| new("Error message"), | ||
| new("Debounce Delay"), | ||
| new("Two-way Data Binding"), | ||
| ]), |
There was a problem hiding this comment.
Fix nav anchor mismatch (“Error message” vs “Error Message”)
Headings array uses “Error message”, section title uses “Error Message”. This breaks in-page navigation anchors.
- new("Error message"),
+ new("Error Message"),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| new("Error message"), | |
| new("Debounce Delay"), | |
| new("Two-way Data Binding"), | |
| ]), | |
| new("Error Message"), | |
| new("Debounce Delay"), | |
| new("Two-way Data Binding"), | |
| ]), |
🤖 Prompt for AI Agents
In docs/LumexUI.Docs.Client/Pages/Components/Datebox/Datebox.razor around lines
180 to 183, the headings array contains "Error message" which doesn't match the
section title "Error Message" and breaks nav anchors; update the array entry to
"Error Message" (or make the section title and array identical) so the in-page
anchor text matches exactly and navigation works.
| @@ -0,0 +1,5 @@ | |||
| @rendermode InteractiveWebAssembly | |||
|
|
|||
| <PreviewCode Code="@new(name: null, snippet: "Datebox.Code.Description")"> | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Validate snippet key and Code parameter shape.
- Ensure
Datebox.Code.Descriptionexists and is correctly spelled/cased. - Confirm
PreviewCode’sCodeparameter supports target-typednew(name: null, snippet: ...)(null name allowed).
🏁 Script executed:
#!/bin/bash
# Snippet key presence
rg -nS 'Datebox\.Code\.Description'
# Example component exists
fd -a Description.razor docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples
# Check how Code is typically passed elsewhere (consistency with @new usage)
rg -n 'Code="@new\(' docs/LumexUI.Docs.Client/Pages -S
# Find PreviewCode component and inspect its Code parameter type/record
rg -nP -C2 '(?:component|class)\s+PreviewCode\b|Code\s*=' -SLength of output: 239316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== repo root ==="
pwd
echo
echo "=== files in docs/LumexUI.Docs.Client/Components ==="
ls -la docs/LumexUI.Docs.Client/Components || true
echo
echo "=== PreviewCode.razor ==="
if [ -f docs/LumexUI.Docs.Client/Components/PreviewCode.razor ]; then
sed -n '1,240p' docs/LumexUI.Docs.Client/Components/PreviewCode.razor
else
echo "NOT FOUND: docs/LumexUI.Docs.Client/Components/PreviewCode.razor"
fi
echo
echo "=== PreviewCode.razor.cs ==="
if [ -f docs/LumexUI.Docs.Client/Components/PreviewCode.razor.cs ]; then
sed -n '1,240p' docs/LumexUI.Docs.Client/Components/PreviewCode.razor.cs
else
echo "NOT FOUND: docs/LumexUI.Docs.Client/Components/PreviewCode.razor.cs"
fi
echo
echo "=== Try to locate CodeSnippet component files ==="
rg -n --hidden --follow --glob '!**/node_modules/**' "CodeSnippet\.razor|class\s+CodeSnippet|partial\s+class\s+CodeSnippet" -S || true
echo
echo "=== Print docs/LumexUI.Docs.Client/Components/CodeSnippet.* if present ==="
if [ -f docs/LumexUI.Docs.Client/Components/CodeSnippet.razor ]; then
sed -n '1,240p' docs/LumexUI.Docs.Client/Components/CodeSnippet.razor
else
echo "CodeSnippet.razor not found at docs/LumexUI.Docs.Client/Components/"
fi
if [ -f docs/LumexUI.Docs.Client/Components/CodeSnippet.razor.cs ]; then
sed -n '1,240p' docs/LumexUI.Docs.Client/Components/CodeSnippet.razor.cs
else
echo "CodeSnippet.razor.cs not found at docs/LumexUI.Docs.Client/Components/"
fi
echo
echo "=== Search for CodeBlock type/definition/usages ==="
rg -n --hidden --follow --glob '!**/node_modules/**' "record\s+CodeBlock|class\s+CodeBlock|struct\s+CodeBlock|new\s+CodeBlock" -S || true
echo
echo "=== Search for usages of Code=\"@new( ... )\" pattern ==="
rg -n --hidden --follow --glob '!**/node_modules/**' 'Code="@new\(|Code=\@\s*new\(' -S || true
echo
echo "=== Search for the snippet key string presence ==="
rg -n --hidden --follow --glob '!**/node_modules/**' 'Datebox\.Code\.Description' -S || true
echo
echo "=== List example file for Datebox Description example (if exists) ==="
if [ -f docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Description.razor ]; then
echo "Found example file:"
sed -n '1,240p' docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Description.razor
else
echo "No Example file at docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Description.razor"
fiLength of output: 8945
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== repo root ==="
pwd
echo
echo "=== Search for CodeBlock declarations (class/record/struct) ==="
rg -n --hidden --no-ignore --follow -P '^(?:public\s+)?(?:record|class|struct)\s+CodeBlock\b' -S || true
echo
echo "=== Search for any occurrence of 'CodeBlock' to locate files ==="
rg -n --hidden --no-ignore --follow '\bCodeBlock\b' -S || true
echo
echo "=== Print files that reference CodeBlock (first 240 lines) ==="
rg --hidden --no-ignore --follow '\bCodeBlock\b' -S --files-with-matches | sort -u | while read -r f; do
echo "----- $f -----"
sed -n '1,240p' "$f" || true
echo
doneLength of output: 18710
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== find .html files (limit 1000) ==="
find . -type f -iname '*.html' -print | sed -n '1,1000p' || true
echo
echo "=== search for filenames or contents that include 'Datebox.Code.Description' ==="
rg -n --hidden --no-ignore --follow 'Datebox\.Code\.Description' -S || true
echo
echo "=== search for files whose names include 'Datebox.Code' ==="
rg -n --hidden --no-ignore --follow 'Datebox\.Code' -S || true
echo
echo "=== list embedded-resources folder candidates ==="
rg -n --hidden --no-ignore --follow 'EmbeddedResource|BuildAction' -S || true
echo
echo "=== list files under docs/LumexUI.Docs.Client that might be snippets ==="
find docs/LumexUI.Docs.Client -type f \( -iname '*.html' -o -iname '*.txt' -o -iname '*.md' \) -print | sed -n '1,240p' || trueLength of output: 5635
Add the missing embedded snippet for "Datebox.Code.Description"; CodeBlock usage is fine.
- CodeBlock/new is valid — CodeBlock is declared (docs/LumexUI.Docs.Client/Common/CodeBlock.cs) and PreviewCode/CodeSnippet accept a CodeBlock, so Code="@new(name: null, snippet: ...)" is correct.
- Missing snippet resource — no embedded HTML matching "Datebox.Code.Description.html" was found under Pages/ or Samples/. Add the snippet HTML (named to include Datebox.Code.Description.html) into docs/LumexUI.Docs.Client/Samples/** or Pages/** so CodeSnippet can locate $"{Code.Snippet}.html" (the csproj already embeds Pages//*.html and Samples//*.html).
Location: docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Description.razor line 3.
🤖 Prompt for AI Agents
In
docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/Description.razor
around line 3, the PreviewCode references a snippet "Datebox.Code.Description"
but the corresponding embedded HTML snippet file is missing; add a file named to
match Datebox.Code.Description.html containing the example HTML and place it
under docs/LumexUI.Docs.Client/Samples/ or Pages/ (or a subfolder) so the
project’s existing globs (Pages/**/*.html or Samples/**/*.html) will embed it
and CodeSnippet can find $"{Code.Snippet}.html".
|
Thank you! I will review it asap :) |
|
Hey @sikelio, Sorry for the delay in reviewing this PR. My focus shifted to a side project I’ve been working on, and this week I also got a bit sick, so it’s been hard to sit down and focus on such a big PR :/ That said, I should be able to review it tomorrow. Before that, I just wanted to ask -- do you want to handle all the development changes yourself based on my comments, or would you prefer me to step in and help out? Edit: I just realized I also want to include custom segments, so it’ll probably be easier if I step in and help with this. |
|
Hey @desmondinho |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/LumexUI/Components/Textbox/LumexTextbox.razor.cs (1)
26-31: Fix missing base.OnParametersSet() call in LumexDebouncedInputBase.The parent class
LumexDebouncedInputBase.OnParametersSet()does not callbase.OnParametersSet(), breaking the Blazor component lifecycle chain. This prevents important initialization logic inLumexInputFieldBaseand further up the hierarchy from executing, potentially causing bugs with validation, change tracking, and other lifecycle-dependent features.Add
base.OnParametersSet();to the end ofLumexDebouncedInputBase.OnParametersSet()method.
♻️ Duplicate comments (2)
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Datebox.razor (1)
180-180: Fix nav anchor mismatch ("Error message" vs "Error Message").The heading entry uses "Error message" (lowercase 'm'), but the section title at Line 110 uses "Error Message" (uppercase 'M'). This mismatch breaks in-page anchor navigation.
🔎 Proposed fix
- new("Error message"), + new("Error Message"),src/LumexUI/Components/Datebox/LumexDatebox.razor.cs (1)
54-56: This concern was already addressed in a past review comment regarding auto-focus behavior. The suggestion to gate focus behind anAutoFocusparameter remains valid.
🧹 Nitpick comments (2)
src/LumexUI/Components/Datebox/LumexDatebox.razor.cs (1)
94-106: Simplify the if-else to a single return statement.The conditional structure can be simplified since both branches return the result of the conversion attempt.
🔎 Suggested simplification
protected override bool TryParseValueFromString( string? value, [MaybeNullWhen( false )] out TValue result ) { - if( BindConverter.TryConvertTo( value, CultureInfo.InvariantCulture, out result ) ) - { - Debug.Assert( result != null ); - return true; - } - else - { - return false; - } + var success = BindConverter.TryConvertTo( value, CultureInfo.InvariantCulture, out result ); + Debug.Assert( !success || result != null ); + return success; }src/LumexUI/Components/Bases/LumexInputFieldBase.razor.cs (1)
241-243: Remove extra blank line.There's a stray blank line before the closing brace.
🔎 Minor cleanup
return OnChangeCoreAsync( args ); - }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.csdocs/LumexUI.Docs.Client/Pages/Components/Datebox/Datebox.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/CustomStyles.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Description.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Disabled.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ErrorMessage.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/LabelPlacements.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ReadOnly.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Required.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Sizes.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/StartEndContent.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/TwoWayDataBinding.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Usage.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Variants.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/_Radius.razorsrc/LumexUI/Common/Enums/InputDateType.cssrc/LumexUI/Common/Enums/InputType.cssrc/LumexUI/Components/Bases/LumexDebouncedInputBase.cssrc/LumexUI/Components/Bases/LumexInputBase.cssrc/LumexUI/Components/Bases/LumexInputFieldBase.razorsrc/LumexUI/Components/Bases/LumexInputFieldBase.razor.cssrc/LumexUI/Components/Datebox/LumexDatebox.razorsrc/LumexUI/Components/Datebox/LumexDatebox.razor.cssrc/LumexUI/Components/Numbox/LumexNumbox.razorsrc/LumexUI/Components/Numbox/LumexNumbox.razor.cssrc/LumexUI/Components/Textbox/LumexTextbox.razorsrc/LumexUI/Components/Textbox/LumexTextbox.razor.cs
💤 Files with no reviewable changes (1)
- src/LumexUI/Components/Bases/LumexInputBase.cs
✅ Files skipped from review due to trivial changes (1)
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Required.razor
🚧 Files skipped from review as they are similar to previous changes (10)
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/CustomStyles.razor
- src/LumexUI/Components/Datebox/LumexDatebox.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Usage.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ErrorMessage.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/LabelPlacements.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Sizes.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ReadOnly.razor
- docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.cs
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/TwoWayDataBinding.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Variants.razor
🧰 Additional context used
🧬 Code graph analysis (4)
src/LumexUI/Components/Textbox/LumexTextbox.razor.cs (1)
src/LumexUI/Components/Bases/LumexDebouncedInputBase.cs (2)
LumexDebouncedInputBase(15-119)LumexDebouncedInputBase(27-30)
src/LumexUI/Components/Numbox/LumexNumbox.razor.cs (1)
src/LumexUI/Components/Bases/LumexDebouncedInputBase.cs (2)
LumexDebouncedInputBase(15-119)LumexDebouncedInputBase(27-30)
src/LumexUI/Components/Datebox/LumexDatebox.razor.cs (2)
src/LumexUI/Components/Bases/LumexInputFieldBase.razor.cs (3)
LumexInputFieldBase(21-332)LumexInputFieldBase(175-182)TryParseValueFromString(266-269)src/LumexUI/Extensions/EnumExtensions.cs (1)
ToDescription(11-19)
src/LumexUI/Components/Bases/LumexInputFieldBase.razor.cs (2)
src/LumexUI/Components/Bases/LumexInputBase.cs (9)
LumexInputBase(22-238)Task(132-143)Task(149-159)Task(165-189)Task(197-201)Task(209-214)TryParseValueFromString(237-237)ValueTask(120-129)ValueTask(228-228)src/LumexUI/Components/Bases/LumexDebouncedInputBase.cs (4)
Task(44-52)Task(55-58)Task(75-103)ValueTask(61-65)
🔇 Additional comments (15)
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Disabled.razor (1)
1-5: LGTM!Clean example demonstrating the disabled state of LumexDatebox with appropriate attributes.
src/LumexUI/Components/Numbox/LumexNumbox.razor (1)
2-2: LGTM!The change to inherit from
LumexDebouncedInputBase<TValue>correctly aligns Numbox with the refactored base class hierarchy, ensuring debounce functionality is preserved.src/LumexUI/Common/Enums/InputType.cs (1)
54-54: Removal ofInputType.Coloris safe and has no impact.No references to
InputType.Coloror related color input configuration were found in the codebase. This removal is not a breaking change within the application.src/LumexUI/Components/Bases/LumexInputFieldBase.razor (1)
2-2: Debounce functionality is correctly preserved across components.The architectural change has been properly implemented. Components requiring debounce functionality (Textbox, Numbox) explicitly inherit from
LumexDebouncedInputBase<TValue>, while components without debounce requirements (Datebox) inherit directly fromLumexInputFieldBase<TValue>. The inheritance chain is clean:LumexInputBase→LumexInputFieldBase→LumexDebouncedInputBase(for debounce components). All affected components have been correctly updated and no issues remain.src/LumexUI/Components/Textbox/LumexTextbox.razor.cs (2)
15-15: LGTM: Inheritance change adds debounce capability.The change from
LumexInputFieldBase<string?>toLumexDebouncedInputBase<string?>appropriately adds debouncing support to the Textbox component. SinceLumexDebouncedInputBaseinherits fromLumexInputFieldBaseand debouncing is opt-in (viaDebounceDelayparameter), this maintains backward compatibility while enabling a commonly needed feature for text inputs.
17-23: LGTM: Well-designed Type parameter.The addition of the
Typeparameter with a sensible default ofInputType.Textis a good API enhancement. This allows consumers to configure the HTML input type (e.g., email, password, url) while maintaining backward compatibility through the default value.docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Description.razor (1)
1-5: LGTM: Clear documentation example.The example effectively demonstrates the
Descriptionproperty onLumexDatebox, providing users with a straightforward reference for this feature.src/LumexUI/Components/Textbox/LumexTextbox.razor (1)
2-2: LGTM: Consistent with code-behind change.The
@inheritsdirective correctly matches the inheritance change in the corresponding.csfile.docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/_Radius.razor (1)
1-24: LGTM: Well-structured documentation example.The example effectively demonstrates the
Radiusproperty variations onLumexDateboxusing a clear, responsive grid layout with helpful captions for each variant.src/LumexUI/Components/Numbox/LumexNumbox.razor.cs (2)
18-18: LGTM: Inheritance change adds debounce capability.The change from
LumexInputFieldBase<TValue>toLumexDebouncedInputBase<TValue>appropriately adds debouncing support to the Numbox component. This is particularly useful for numeric inputs where you may want to delay processing until the user has finished typing. The change maintains backward compatibility as debouncing is opt-in.
23-29: Same base.OnParametersSet() verification needed.Like
LumexTextbox, this component relies onLumexDebouncedInputBase.OnParametersSet()callingbase.OnParametersSet()to maintain the lifecycle chain. The verification script in theLumexTextbox.razor.csreview will confirm whether this is implemented correctly.src/LumexUI/Common/Enums/InputDateType.cs (1)
9-37: Well-structured enum for HTML date input types.The enum correctly maps to HTML5 input types via
Descriptionattributes, and the XML documentation clearly describes each member's purpose. The integration withToDescription()extension method provides clean type-to-string conversion.src/LumexUI/Components/Bases/LumexDebouncedInputBase.cs (2)
70-118: Clean debouncer implementation with proper cancellation handling.The
Debouncerclass correctly handles:
- Cancellation of pending debounce operations
- Immediate execution when
milliseconds <= 0- Graceful handling of
OperationCanceledException- Idempotent disposal
32-41: Good validation: debounce requires OnInput behavior.The parameter validation correctly enforces that
DebounceDelaycan only be used withInputBehavior.OnInput. This prevents a confusing configuration where debouncing would have no effect withOnChangebehavior.src/LumexUI/Components/Bases/LumexInputFieldBase.razor.cs (1)
316-331: Robust disposal pattern for Blazor Server scenarios.The exception handling for
JSDisconnectedExceptionandOperationCanceledExceptioncorrectly handles the case where the client has already disconnected. This is the recommended pattern for Blazor components using JS interop.
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/StartEndContent.razor
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #250 +/- ##
==========================================
- Coverage 96.95% 92.96% -4.00%
==========================================
Files 70 153 +83
Lines 1542 2655 +1113
Branches 150 395 +245
==========================================
+ Hits 1495 2468 +973
- Misses 28 99 +71
- Partials 19 88 +69 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (2)
src/LumexUI/Components/Datebox/LumexDatebox.razor.cs (1)
53-64: Consider adding Type/TValue compatibility validation.The constructor validates that
TValueis a supported type, but there's no check that theTypeparameter is semantically compatible withTValue. For example, usingType = InputDateType.TimewithTValue = DateOnlycould produce unexpected behavior or formatting errors.src/LumexUI/Components/Bases/LumexInputFieldBase.razor.cs (1)
109-112: Typo in XML comment: "mesage" → "message".🔎 Fix typo
/// <summary> -/// Gets or sets the validation error mesage of the input. +/// Gets or sets the validation error message of the input. /// </summary> protected string? ValidationMessage { get; private set; }
🧹 Nitpick comments (2)
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ErrorMessage.razor (1)
10-10: Consider using a neutral color when there's no error.The
Colorparameter alternates betweenDangerandSuccess, meaning the component shows a success color even when there's no error. Typically, form inputs use a neutral/default color in a valid state and only highlight withDangeron error orSuccessafter explicit successful submission.🔎 Proposed adjustment
- Color="@(!string.IsNullOrEmpty(_userValidator.DateErrorMessage) ? ThemeColor.Danger : ThemeColor.Success)" + Color="@(!string.IsNullOrEmpty(_dateErrorMessage) ? ThemeColor.Danger : ThemeColor.Default)"Or omit the
Colorparameter entirely if the component has appropriate default styling.tests/LumexUI.Tests/Components/Datebox/DateboxTests.razor (1)
72-83: Clarify test name or assertion message.The test
ShouldRenderHelperWrapperWhenErrorMessageProvidedverifies that the error-message slot is not rendered whenInvalidis false—which is correct behavior. However, thebecausemessage says "Error message should be rendered when state is invalid", which is the opposite of what's being tested here.Consider updating the message for clarity:
🔎 Proposed fix
- action.Should().Throw<ElementNotFoundException>(because: "Error message should be rendered when state is invalid."); + action.Should().Throw<ElementNotFoundException>(because: "Error message should NOT be rendered when state is valid.");
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Datebox.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ClearButton.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Colors.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/CustomStyles.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Description.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Disabled.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ErrorMessage.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/LabelPlacements.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ReadOnly.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Required.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Sizes.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/StartEndContent.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/TwoWayDataBinding.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Usage.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Variants.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/_Radius.razordocs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/CustomStyles.razordocs/LumexUI.Docs.Client/Pages/Components/Numbox/Numbox.razorsrc/LumexUI/Components/Bases/LumexDebouncedInputBase.cssrc/LumexUI/Components/Bases/LumexInputFieldBase.razor.cssrc/LumexUI/Components/Datebox/LumexDatebox.razor.cstests/LumexUI.Tests/Components/Datebox/DateboxTests.razor
✅ Files skipped from review due to trivial changes (1)
- docs/LumexUI.Docs.Client/Pages/Components/Numbox/Numbox.razor
🚧 Files skipped from review as they are similar to previous changes (7)
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/PreviewCodes/CustomStyles.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/_Radius.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ReadOnly.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Datebox.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/StartEndContent.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Required.razor
- docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/LabelPlacements.razor
🧰 Additional context used
🧬 Code graph analysis (2)
src/LumexUI/Components/Bases/LumexDebouncedInputBase.cs (2)
src/LumexUI/Components/Bases/LumexInputFieldBase.razor.cs (10)
LumexInputFieldBase(21-296)LumexInputFieldBase(170-177)Task(180-193)Task(196-207)Task(217-217)Task(227-227)Task(252-258)Task(260-263)Task(265-271)Task(273-278)src/LumexUI/Components/Bases/LumexInputBase.cs (5)
Task(132-143)Task(149-159)Task(165-189)Task(197-201)Task(209-214)
src/LumexUI/Components/Datebox/LumexDatebox.razor.cs (2)
src/LumexUI/Components/Textbox/LumexTextbox.razor.cs (2)
OnParametersSet(26-31)TryParseValueFromString(34-38)src/LumexUI/Extensions/EnumExtensions.cs (1)
ToDescription(11-19)
🔇 Additional comments (11)
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Sizes.razor (1)
1-21: LGTM!Clean documentation example demonstrating the
Sizeparameter variations forLumexDatebox. The implementation correctly iterates over the size options and renders each variant with an appropriate label.docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Variants.razor (1)
1-23: LGTM!Well-structured documentation example showcasing the
InputVariantoptions forLumexDatebox. Consistent with the project's example patterns.docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/TwoWayDataBinding.razor (1)
1-31: LGTM!Excellent documentation example demonstrating both two-way data binding approaches:
- Simplified
@bind-Valuesyntax- Explicit
Value/ValueChangedpattern for more controlThis provides clear guidance for users choosing between binding styles.
src/LumexUI/Components/Datebox/LumexDatebox.razor.cs (1)
81-119: LGTM!The input/change handling and value formatting/parsing logic is well-implemented:
OnInputAsynccorrectly returns a no-op since this component updates on change events onlyOnChangeAsyncproperly delegates toSetCurrentValueAsStringAsyncFormatValueAsStringexhaustively handles all supported types with appropriate format stringsTryParseValueFromStringusesBindConverterwith invariant culture for consistent parsingsrc/LumexUI/Components/Bases/LumexDebouncedInputBase.cs (2)
86-137: LGTM!The
Debouncerimplementation is well-designed:
- Properly cancels pending debounce operations before starting new ones
- Handles edge case of
milliseconds <= 0as immediate execution- Catches
OperationCanceledExceptiongracefully when cancelled- Idempotent
Disposepattern with the_disposedflagThis works correctly within Blazor's single-threaded synchronization context.
52-77: LGTM!The
OnInputAsyncandOnChangeAsyncimplementations cleanly separate behavior:
OnInputAsyncrespects theBehaviorparameter and applies debouncing when configuredOnChangeAsynchandles immediate updates forOnChangebehavior- Both early-return when the behavior doesn't match, avoiding unnecessary work
tests/LumexUI.Tests/Components/Datebox/DateboxTests.razor (3)
6-13: LGTM!Good test setup with proper service registration and JS interop mocking for the input module.
15-49: LGTM!Good coverage of core functionality:
- Basic rendering validation
- Type constraint enforcement (throws for unsupported
TValue)- Parameterized test for all
InputDateTypevalues mapping to correct HTML input types
196-222: LGTM!Excellent tests verifying the default
OnChangebehavior:
ShouldChangeValueUsingChangeEventconfirms value updates on change eventsShouldNotChangeValueUsingInputEventcorrectly verifies that input events are ignored (no-op)This addresses the behavior documented in the component.
src/LumexUI/Components/Bases/LumexInputFieldBase.razor.cs (2)
195-207: LGTM!The autofocus logic is now correctly gated within the
firstRenderblock, preventing focus-stealing on subsequent re-renders. This addresses the concern from the previous review.
280-295: LGTM!Proper async disposal pattern with appropriate exception handling for
JSDisconnectedExceptionandOperationCanceledException—both expected when the client disconnects.
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ClearButton.razor
Outdated
Show resolved
Hide resolved
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ClearButton.razor
Outdated
Show resolved
Hide resolved
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ErrorMessage.razor
Show resolved
Hide resolved
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ErrorMessage.razor
Outdated
Show resolved
Hide resolved
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/ErrorMessage.razor
Show resolved
Hide resolved
docs/LumexUI.Docs.Client/Pages/Components/Datebox/Examples/Usage.razor
Outdated
Show resolved
Hide resolved
|
Hey @sikelio, Thank you for your contribution to this component -- great work. Apologies for the delay in reviewing and merging the PR. I needed to step away from the project for a short period and am now back with renewed focus. Much appreciated ❤️ |
|
Hey @desmondinho |
2 participants
Closes #192
Closes #248
Description
Implementation of Datebox component.
What's been done?
Checklist
Additional Notes
None
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.