Draft
Conversation
Move form code to codebehind Process review comments
- Comment CSS to check if it can be deleted - Do not fill MessageTemplate by default - Make FieldCondition use FluentText for each messsage and use the 'message' slot (instead of div's with FluentTexts) - Use correct style/color in FieldParameterCollection - Use Parameter/Attribute/Default for TextInput Message
Unit Tests
Details on your Workflow / Core Tests page. |
- Do not put each condition in its own message slot - Update tests and verified files
Summary - Unit Tests Code CoverageSummary
CoverageMicrosoft.FluentUI.AspNetCore.Components - 98.7%
|
- Add more unit tests
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new FluentValidationMessage component (Fluent-styled equivalent to Blazor’s ValidationMessage) and updates FluentField to surface EditContext validation messages in the field’s message slot, alongside extensive test/snapshot and demo documentation updates.
Changes:
- Introduce
FluentValidationMessage<TValue>(+ styling) and unit tests. - Extend
FluentField/IFluentFieldto track a bound expression/field and renderEditContextvalidation messages in the message slot. - Update demos/docs and refresh many bUnit Verify snapshots impacted by the
FluentFieldmessage rendering changes.
Reviewed changes
Copilot reviewed 74 out of 74 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Core/Components/Switch/FluentSwitchTests.razor | Tweaks label trimming to account for tabs in rendered markup. |
| tests/Core/Components/Switch/FluentSwitchTests.FluentSwitch_Default.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Switch/FluentSwitchTests.FluentCheckbox_Required.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Switch/FluentSwitchTests.FluentCheckbox_ReadOnly.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Switch/FluentSwitchTests.FluentCheckbox_LabelTemplate.verified.razor.html | Snapshot update reflecting new rendered structure/formatting. |
| tests/Core/Components/Slider/FluentSliderTests.FluentSlider_Slot_Thumb.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Slider/FluentSliderTests.FluentSlider_Default.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Radio/FluentRadioTests.FluentRadio_LabelWidth.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Radio/FluentRadioTests.FluentRadio_LabelTemplate.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Radio/FluentRadioTests.FluentRadio_Label.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Radio/FluentRadioTests.FluentRadio_HorizontalOrientation.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Radio/FluentRadioTests.FluentRadio_Default.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Radio/FluentRadioGroupTests.FluentRadioGroup_WithChildContent.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Radio/FluentRadioGroupTests.FluentRadioGroup_Items.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Radio/FluentRadioGroupTests.FluentRadioGroup_Default.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/List/FluentSelectTests.FluentSelect_OptionFunctions.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/List/FluentSelectTests.FluentSelect_Manual.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/List/FluentSelectTests.FluentSelect_Label.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/List/FluentSelectTests.FluentSelect_Enum.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/List/FluentSelectTests.FluentSelect_Default.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/List/FluentComboboxTests.FluentCombobox_OptionFunctions.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/List/FluentComboboxTests.FluentCombobox_Manual.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/List/FluentComboboxTests.FluentCombobox_Label.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/List/FluentComboboxTests.FluentCombobox_Enum.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/List/FluentComboboxTests.FluentCombobox_Default.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Forms/FluentValidationMessageTests.cs | Adds unit tests validating rendering, updates, precedence, and missing-EditContext behavior. |
| tests/Core/Components/Field/FluentFieldTests.razor | Adds tests for validation-message integration and Field/For precedence scenarios. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_Size-small.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_Size-medium.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_Size-large.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_Size-99.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_Size-[empty].verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_MessageState-warning.verified.razor.html | Snapshot update reflecting new message rendering behavior. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_MessageState-success.verified.razor.html | Snapshot update reflecting new message rendering behavior. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_MessageState-error.verified.razor.html | Snapshot update reflecting new message rendering behavior. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_MessageState-99.verified.razor.html | Snapshot update reflecting new message rendering behavior. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_MessageState-[empty].verified.razor.html | Snapshot update reflecting new message rendering behavior. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_MessageIcon.verified.razor.html | Snapshot update reflecting new message slot markup (wrapper + FluentText). |
| tests/Core/Components/Field/FluentFieldTests.FluentField_DefaultTemplate.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Field/FluentFieldTests.FluentField_Default.verified.razor.html | Snapshot update reflecting new rendered structure/message behavior. |
| tests/Core/Components/Field/FluentFieldParameterCollectorTests.razor | Renames/updates selector tests and adds ValueExpression tests. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Size-medium.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Size-large.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Size-999.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Size-[empty].verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_ShowIndeterminate.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Shape-square.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Shape-circular.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Shape-999.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Shape-[empty].verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_LabelTemplate.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_EnableThreeState.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Default.verified.razor.html | Snapshot update reflecting new rendered structure. |
| tests/Core/Components/Base/InputBaseTests.cs | Updates expected styling behavior for MessageState. |
| tests/Core/Components/Base/ComponentBaseTests.cs | Excludes FluentValidationMessage<> from generic component tests due to required EditContext setup. |
| src/Core/Components/TextInput/FluentTextInput.razor.cs | Enhances required-message derivation by reading [Required] attribute ErrorMessage via ValueExpression. |
| src/Core/Components/Forms/FluentValidationMessage.razor.css | Adds styling for FluentValidationMessage output. |
| src/Core/Components/Forms/FluentValidationMessage.razor.cs | Implements FluentValidationMessage behavior and EditContext subscription lifecycle. |
| src/Core/Components/Forms/FluentValidationMessage.razor | Renders field validation messages with icon and styling. |
| src/Core/Components/Field/Readme.md | Documentation wording/grammar improvements. |
| src/Core/Components/Field/IFluentField.cs | Extends the field contract with ValueExpression to support field identification. |
| src/Core/Components/Field/FluentFieldParameterCollector.cs | Replaces prior selector with collector; adds ValueExpression and changes message selection defaults. |
| src/Core/Components/Field/FluentField.razor.css | Ensures message slot content displays as block. |
| src/Core/Components/Field/FluentField.razor.cs | Adds EditContext tracking, FieldIdentifier inference, and validation message integration. |
| src/Core/Components/Field/FluentField.razor | Renders validation messages (and custom messages) in the field message slot. |
| src/Core/Components/Field/Condition/FluentFieldCondition.cs | Adjusts how multi-condition message templates are built/rendered. |
| src/Core/Components/Base/FluentInputBase.cs | Implements IFluentField.ValueExpression and improves disposal semantics. |
| examples/Tools/FluentUI.Demo.SampleData/Starship.cs | Extends sample model with Password validation attributes and adjusts other validation. |
| examples/Demo/FluentUI.Demo.Client/Documentation/Components/Forms/Forms.md | Adds FluentValidationMessage to docs and API section. |
| examples/Demo/FluentUI.Demo.Client/Documentation/Components/Forms/Examples/BasicForm.razor.css | Adds layout CSS for the updated basic form example. |
| examples/Demo/FluentUI.Demo.Client/Documentation/Components/Forms/Examples/BasicForm.razor | Updates the form example to mention/show validation-message support (currently commented). |
| examples/Demo/FluentUI.Demo.Client/Documentation/Components/Field/Examples/FieldStatesExample.razor | Updates the field states example to run inside an EditForm and bind values. |
| examples/Demo/FluentUI.Demo.Client/Documentation/Components/Field/Examples/FieldMessageMultipleConditions.razor | Marks the sample as Required to trigger message behavior. |
| .github/instructions/blazor.instructions.md | Adds repository guidance for Blazor component/app patterns. |
Comments suppressed due to low confidence (2)
src/Core/Components/Field/FluentFieldParameterCollector.cs:108
MessageConditionnow defaults toFluentFieldCondition.Neverwhen neither the FluentField nor the input component sets it. This makesMessage/MessageState/MessageIconeffectively never render for most components that only setMessage(e.g., helper text in demos) unless callers explicitly setMessageCondition. Consider restoring the previous default (Always), or defaulting toAlwayswhenMessage/MessageTemplate/MessageStateis provided to avoid a breaking regression.
tests/Core/Components/Field/FluentFieldParameterCollectorTests.razor:51- Typo in comment: "defaukt" → "default".
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else | ||
| { | ||
|
|
||
| <FluentText Color="@(Parameters.MessageState is null ? Color.Error : Color.Info)" Size="@TextSize.Size200" Style="display: flex; align-items: center;"> |
There was a problem hiding this comment.
The message rendering uses Color.Info for any non-null MessageState, so Success/Warning/Error all get the same color. This also diverges from FluentFieldParameterCollector.StateToIcon/StateToMessage behavior and makes MessageState largely meaningless. Consider mapping MessageState to the appropriate FluentText color (e.g., Success→Success, Warning→Info/Warning, Error→Error) instead of treating all states as Info.
Suggested change
| <FluentText Color="@(Parameters.MessageState is null ? Color.Error : Color.Info)" Size="@TextSize.Size200" Style="display: flex; align-items: center;"> | |
| @{ | |
| var messageColor = Color.Info; | |
| if (Parameters.MessageState is null) | |
| { | |
| messageColor = Color.Error; | |
| } | |
| else | |
| { | |
| var stateName = Parameters.MessageState.ToString(); | |
| if (string.Equals(stateName, "Success", System.StringComparison.OrdinalIgnoreCase)) | |
| { | |
| messageColor = Color.Success; | |
| } | |
| else if (string.Equals(stateName, "Warning", System.StringComparison.OrdinalIgnoreCase)) | |
| { | |
| messageColor = Color.Warning; | |
| } | |
| else if (string.Equals(stateName, "Error", System.StringComparison.OrdinalIgnoreCase)) | |
| { | |
| messageColor = Color.Error; | |
| } | |
| else | |
| { | |
| messageColor = Color.Info; | |
| } | |
| } | |
| } | |
| <FluentText Color="@messageColor" Size="@TextSize.Size200" Style="display: flex; align-items: center;"> |
Comment on lines
+81
to
+90
| builder.OpenComponent<FluentText>(0); | ||
| builder.AddComponentParameter(1, "As", TextTag.Span); | ||
| builder.AddComponentParameter(2, "Color", item.State == MessageState.Error ? Color.Error : Color.Info); | ||
| //builder.AddAttribute(4, "slot", "message"); | ||
| builder.AddAttribute(5, "style", "display: flex; align-items: center;"); | ||
| builder.AddAttribute(6, "ChildContent", (RenderFragment)(contentBuilder => | ||
| { | ||
| builder.AddContent(2, coloredMessage); | ||
| } | ||
| contentBuilder.AddContent(0, FluentField.CreateIcon(item.Icon ?? FluentFieldParameterCollector.StateToIcon(item.State))); | ||
| contentBuilder.AddContent(1, item.Message); | ||
| })); |
There was a problem hiding this comment.
FluentFieldCondition.Build assigns Color.Info for any state other than Error, which means Success messages render with the same color as Warning/Info. If MessageState.Success is intended to be visually distinct, map Success to Color.Success (and Warning to the appropriate color) to keep state semantics consistent.
Comment on lines
+212
to
+216
| private static FieldIdentifier CreateFieldIdentifier(LambdaExpression accessor) | ||
| { | ||
| var method = typeof(FieldIdentifier).GetMethod("Create", BindingFlags.Public | BindingFlags.Static)!; | ||
| return (FieldIdentifier)method.MakeGenericMethod(accessor.ReturnType).Invoke(null, [accessor])!; | ||
| } |
There was a problem hiding this comment.
CreateFieldIdentifier uses GetMethod("Create") without disambiguating overloads. If FieldIdentifier gains another public static Create overload, this can start returning the wrong method at runtime. Consider selecting the method by signature (name + IsGenericMethodDefinition + parameter count/type) or caching the MethodInfo to avoid repeated reflection.
| /// </summary> | ||
| [Required] | ||
| [MinLength(8, ErrorMessage = "Password is too short")] | ||
| [RegularExpression(@"[A-Z][a-z]{\d}+", ErrorMessage = "Password must contain at least one uppercase letter, one lowercase letter and one digit")] |
There was a problem hiding this comment.
The RegularExpression pattern "[A-Z][a-z]{\d}+" is not a valid regex quantifier ({\d} will throw at runtime when validation runs). If the intent is to require at least one uppercase, one lowercase, and one digit, update the pattern accordingly (e.g., using lookaheads or a corrected quantifier).
Suggested change
| [RegularExpression(@"[A-Z][a-z]{\d}+", ErrorMessage = "Password must contain at least one uppercase letter, one lowercase letter and one digit")] | |
| [RegularExpression(@"^(?=.*[A-Z])(?=.*[a-z])(?=.*\d).+$", ErrorMessage = "Password must contain at least one uppercase letter, one lowercase letter and one digit")] |
| public string? Identifier { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// The unique identifier for the starship. |
There was a problem hiding this comment.
The XML doc summary for Password is a copy/paste of Identifier ("The unique identifier for the starship"). Please update it to describe the password field to avoid misleading generated docs.
Suggested change
| /// The unique identifier for the starship. | |
| /// The password used to secure access to the starship. |
| @@ -0,0 +1,7 @@ | |||
| .fluent-validation-message { | |||
| display: flex; | |||
| color: var(--colorPaletteRedForeground1); | |||
There was a problem hiding this comment.
CSS uses --colorPaletteRedForeground1 for the error text color, while other components typically use the shared alert tokens (var(--error), var(--warning), etc.). Using var(--error) here would better align with the library's existing theming tokens and keep behavior consistent across components.
Suggested change
| color: var(--colorPaletteRedForeground1); | |
| color: var(--error); |
Comment on lines
10
to
13
| <p> | ||
| This form uses the Fluent UI input components. It uses a `DataAnnotationsValidator` and `FluentValidationSummary`. | ||
| This form uses the Fluent UI input components. It uses a `DataAnnotationsValidator`, a `FluentValidationSummary` | ||
| and `FluentValidationMessage`s. | ||
|
|
There was a problem hiding this comment.
This example text says the form uses FluentValidationMessages, but all FluentValidationMessage usages in the markup are currently commented out. Either uncomment at least one instance to demonstrate it, or adjust the description so it matches what the page actually renders.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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
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.
With this PR you can use
FluentValidationMessageto display the validation message like the standard BlazorValidationMesageAlso, the
FluentFieldhas been adapted so the validation messages will be displayed in the message part of the field.Unit Test need to be validated still