Repository navigation
[Input] Required message refactoring - Part 1 - #5399
Open
Vincent Baaij (vnbaaij) wants to merge 5 commits into
Open
Vincent Baaij (vnbaaij) wants to merge 5 commits into
Vincent Baaij (vnbaaij) wants to merge 5 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Vincent Baaij (vnbaaij)
requested a review
from Denis Voituron (dvoituron)
as a code owner
October 6, 2026 13:42
|
✅ All tests passed successfully Details on your Workflow / Core Tests page. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Field-message suppression and constructor-time localization capture introduce user-visible regressions.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Centralizes required-field validation messaging across Fluent input components, including annotation-derived messages and updated examples/tests.
Changes:
- Adds reusable required-message logic to
FluentInputBase. - Adopts the logic across seven input components and adjusts field rendering.
- Updates tests, snapshots, and form sample data.
| File | Description |
|---|---|
tests/Core/Components/TextInput/FluentTextInputTests.razor |
Tests default and annotation-based messages. |
tests/Core/Components/TextArea/FluentTextAreaTests.razor |
Tests required textarea behavior. |
tests/Core/Components/Switch/FluentSwitchTests.razor |
Tests required switch states. |
tests/Core/Components/Switch/FluentSwitchTests.FluentSwitch_Required.verified.razor.html |
Updates required snapshot. |
tests/Core/Components/Switch/FluentSwitchTests.FluentSwitch_ReadOnly.verified.razor.html |
Updates read-only snapshot. |
tests/Core/Components/Switch/FluentSwitchTests.FluentSwitch_LabelTemplate.verified.razor.html |
Updates label-template snapshot. |
tests/Core/Components/Switch/FluentSwitchTests.FluentSwitch_Default.verified.razor.html |
Updates default snapshot. |
tests/Core/Components/Switch/FluentSwitchTests.FluentSwitch_Default_TrueValue.verified.razor.html |
Updates true-value snapshot. |
tests/Core/Components/Radio/FluentRadioTests.FluentRadio_LabelWidth.verified.razor.html |
Removes empty group message. |
tests/Core/Components/Radio/FluentRadioTests.FluentRadio_LabelTemplate.verified.razor.html |
Removes empty group message. |
tests/Core/Components/Radio/FluentRadioTests.FluentRadio_Label.verified.razor.html |
Removes empty group message. |
tests/Core/Components/Radio/FluentRadioTests.FluentRadio_HorizontalOrientation.verified.razor.html |
Removes empty group message. |
tests/Core/Components/Radio/FluentRadioTests.FluentRadio_Default.verified.razor.html |
Removes empty group message. |
tests/Core/Components/Radio/FluentRadioGroupTests.razor |
Tests required radio groups. |
tests/Core/Components/Radio/FluentRadioGroupTests.FluentRadioGroup_WithChildContent.verified.razor.html |
Updates child-content snapshot. |
tests/Core/Components/Radio/FluentRadioGroupTests.FluentRadioGroup_Items.verified.razor.html |
Updates items snapshot. |
tests/Core/Components/Radio/FluentRadioGroupTests.FluentRadioGroup_Default.verified.razor.html |
Updates default snapshot. |
tests/Core/Components/Number/FluentNumberTests.razor |
Tests required number input. |
tests/Core/Components/Field/FluentFieldTests.FluentField_MessageState-error.verified.razor.html |
Captures error text styling. |
tests/Core/Components/DateTimes/FluentCalendarTests.razor |
Tests required calendar behavior. |
tests/Core/Components/Checkbox/FluentCheckboxTests.razor |
Tests required checkbox states. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Size-medium.verified.razor.html |
Updates medium-size snapshot. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Size-large.verified.razor.html |
Updates large-size snapshot. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Size-999.verified.razor.html |
Updates invalid-size snapshot. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Size-[empty].verified.razor.html |
Updates empty-size snapshot. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_ShowIndeterminate.verified.razor.html |
Updates indeterminate snapshot. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Shape-square.verified.razor.html |
Updates square-shape snapshot. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Shape-circular.verified.razor.html |
Updates circular-shape snapshot. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Shape-999.verified.razor.html |
Updates invalid-shape snapshot. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Shape-[empty].verified.razor.html |
Updates empty-shape snapshot. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_LabelTemplate.verified.razor.html |
Updates label-template snapshot. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_EnableThreeState.verified.razor.html |
Updates three-state snapshot. |
tests/Core/Components/Checkbox/FluentCheckboxTests.FluentCheckbox_Default.verified.razor.html |
Updates default snapshot. |
src/Core/Components/TextInput/FluentTextInput.razor.cs |
Uses centralized required logic. |
src/Core/Components/TextArea/FluentTextArea.razor.cs |
Uses centralized required logic. |
src/Core/Components/Switch/FluentSwitch.razor.cs |
Adds required switch messaging. |
src/Core/Components/Radio/FluentRadioGroup.razor.cs |
Adds required selection messaging. |
src/Core/Components/NumberInput/FluentNumberInput.razor.cs |
Uses centralized required logic. |
src/Core/Components/Field/FluentField.razor.cs |
Exposes field-identifier creation internally. |
src/Core/Components/Field/FluentField.razor |
Prioritizes validation messages and styles errors. |
src/Core/Components/DateTime/FluentCalendar.razor.cs |
Uses centralized calendar validation. |
src/Core/Components/Checkbox/FluentCheckbox.razor.cs |
Adds required checkbox messaging. |
src/Core/Components/Base/FluentInputBase.Validation.cs |
Implements shared required-message handling. |
examples/Tools/FluentUI.Demo.SampleData/Starship.cs |
Adds required annotation messages. |
examples/Demo/FluentUI.Demo.Client/Documentation/Components/Forms/Examples/BasicForm.razor |
Marks the teleporter switch required. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary - Unit Tests Code CoverageSummary
CoverageMicrosoft.FluentUI.AspNetCore.Components - 98.3%
Microsoft.FluentUI.AspNetCore.Components.Charts - 100%
|
Denis Voituron (dvoituron)
requested changes
Oct 7, 2026
| { | ||
| LabelPosition = Components.LabelPosition.After; | ||
|
|
||
| MessageCondition = CreateRequiredMessageCondition(() => !Value); |
Collaborator
There was a problem hiding this comment.
High — Default required conditions hide caller-supplied messages on Checkbox, Switch, and RadioGroup
The new constructor defaults in FluentCheckbox.razor.cs, FluentSwitch, and FluentRadioGroup replace the previous implicit Always condition. The shared condition also calls SetRequiredErrorMessage before checking whether the component is required, focused out, or empty in FluentInputBase.Validation.cs.
Consequently, a consumer-provided Message without a custom MessageCondition is no longer displayed and can be overwritten even when Required is false.
Suggested fix: preserve explicit message behavior. Compute the fallback required message only after the required predicate succeeds, and do not mutate a caller-supplied Message/MessageIcon.
| size="@TextSize.Size200"> | ||
|
|
||
| @CreateIcon(Parameters.MessageIcon) | ||
| @CreateIcon(Parameters.MessageIcon) |
Collaborator
There was a problem hiding this comment.
Medium — FluentField now suppresses all custom messages when validation errors exist
The new else in FluentField.razor prevents every MessageCondition message from rendering whenever the EditContext has a validation message. This also suppresses unrelated hints, warnings, and status messages. The generated required predicate already checks for validation messages, so the broad rendering-level suppression is unnecessary for avoiding duplicate required messages.
Suggested fix: keep custom message rendering independent. Suppress only the generated required fallback when an EditContext validation message exists.
| { | ||
| return field => | ||
| { | ||
| var fieldIdentifier = fieldIdentifierProvider?.Invoke() ?? FieldIdentifier; |
Collaborator
There was a problem hiding this comment.
Medium — Required metadata and validation-message lookup can target different fields
ValidationFieldExpression prefers ValidationFieldFor, but CreateRequiredMessageCondition defaults to the component's FieldIdentifier in FluentInputBase.Validation.cs. For wrappers where the text binding differs from the model field, required-attribute lookup and validation-message suppression can therefore inspect different properties, causing a generic message or duplicate feedback.
Suggested fix: derive one FieldIdentifier from ValidationFieldExpression and use it consistently for both metadata lookup and EditContext.GetValidationMessages.
| /// Indicates whether the starship design has been validated. | ||
| /// </summary> | ||
| [Required] | ||
| [Required(ErrorMessage = "Validation is required")] |
Collaborator
There was a problem hiding this comment.
Medium — The new boolean Required annotations do not validate false
The added/updated [Required] attributes on non-nullable bool properties in Starship.cs always pass because a non-nullable Boolean is never null. The adjacent [Range(typeof(bool), "true", "true")] attributes perform the actual “must be true” validation, so the new RequiredAttribute.ErrorMessage cannot demonstrate the behavior this PR intends to showcase.
Suggested fix: remove [Required] from these Boolean properties and keep the enforcement/message on [Range], or use a nullable Boolean if null is the state being validated.
This branch has not been deployed
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.
This pull request introduces a unified and extensible approach to required-field validation messages across Fluent UI input components. The changes centralize the logic for displaying required messages, ensure proper error messaging from model annotations, and update sample data and documentation to reflect these improvements.
Core validation improvements:
FluentInputBase, including theCreateRequiredMessageConditionmethod and helpers to extract error messages fromRequiredAttributeannotations. This ensures all input components display consistent and customizable required messages based on model validation attributes.FluentCheckbox,FluentSwitch,FluentRadioGroup,FluentCalendar,FluentNumberInput,FluentTextInput,FluentTextArea) to use the new required message condition logic, replacing duplicated or ad-hoc implementations. For composed components, like AutoComplete, a separate PR will be created (which can only be merged after this one)Validation message display behavior:
FluentField.razorto show required-field messages only when there are no validation messages from theEditContext, preventing duplicate or conflicting messagesCreateFieldIdentifiermethod inFluentField.razor.csinternal for broader use in validation logic.Sample data and documentation updates:
Test updates: