Validated form controls: Separate validation state and message display state #73559
+394
−665
Conversation
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
mirka
added
[Type] Breaking Change
|
Size Change: -151 B (-0.01%) Total Size: 2.57 MB
ℹ️ View Unchanged
|
mirka
commented
Nov 25, 2025
| ### Delegate elements | ||
|
|
||
| The implementations for `ToggleGroupControl` and `CustomSelectControl` use a "delegate" element for validation, due to upstream limitations that prevent us from using the actual underlying elements for constraint validation. A delegate element in this context is a visually hidden form element that we "delegate" the Constraint Validation API concerns to. | ||
| The implementations for `ToggleGroupControl`, `CustomSelectControl`, and `FormTokenField` use a "delegate" element for validation, due to upstream limitations that prevent us from using the actual underlying elements for constraint validation. A delegate element in this context is a visually hidden form element that we "delegate" the Constraint Validation API concerns to. |
Member
Author
There was a problem hiding this comment.
Forgot to add FormTokenField when that was added to the collection.
Comment on lines
+112
to
+115
| <style> | ||
| { ` | ||
| .my-control:has(:invalid[data-validity-visible]) .my-control__help:not(.is-visible) { | ||
| display: none; |
Member
Author
There was a problem hiding this comment.
This "Help text replacement" story is one case where the devex becomes noticeably worse. And I don't think we can have a nice generic API for it, because the conditions for which a consumer may want to replace the help text can be varied.
This pattern was added by designer requests, but I'm still not sure about it's scalability, given how complex the considerations can get in certain cases.
Member
There was a problem hiding this comment.
This pattern was added by designer requests, but I'm still not sure about it's scalability, given how complex the considerations can get in certain cases.
Yeah this was my first thought: Whether this is something we should even want to be encouraging. It assumes redundancy between help text and the error message, where ideally they'd be distinct enough to not need this kind of substitution. Maybe I'd need to see some more real-world examples, but I don't think the "duplication" is a terrible default experience that we should focus too much on improving the developer experience around this.
| >( undefined ); | ||
|
|
||
| const timeoutRef = useRef< ReturnType< typeof setTimeout > >(); | ||
| const previousValidationValueRef = useRef< unknown >( '' ); |
Member
Author
There was a problem hiding this comment.
I think I was able to make tweaks in the Async story to make the interactions feel a lot more intuitive and snappier.
| } | ||
| return label; | ||
| } | ||
| const VALIDITY_VISIBLE_ATTRIBUTE = 'data-validity-visible'; |
Member
Author
There was a problem hiding this comment.
I had trouble naming this. It's not "touched", because messages can be visible without the field being touched. It's also more like "visibility is activated", rather than "a message is visible right now".
Member
There was a problem hiding this comment.
This seems okay to me as well. If seeking an alternative, we could adapt language around "report"ing, for alignment to browser native APIs like reportValidity.
| >(); | ||
| const [ showMessage, setShowMessage ] = useState( false ); | ||
| const [ isTouched, setIsTouched ] = useState( false ); | ||
| const previousCustomValidityType = usePrevious( customValidity?.type ); |
Member
Author
There was a problem hiding this comment.
This wasn't necessary. It was just masking some unideal state management in the async story.
Comment on lines
-201
to
-209
| const onChange = ( ...args: unknown[] ) => { | ||
| children.props.onChange?.( ...args ); | ||
|
|
||
| // Only validate incrementally if the field has blurred at least once, | ||
| // or currently has an error message. | ||
| if ( isTouched || errorMessage ) { | ||
| onValidate?.(); | ||
| const message = () => { | ||
| if ( errorMessage ) { | ||
| return ( | ||
| <ValidityIndicator type="invalid" message={ errorMessage } /> | ||
| ); | ||
| } | ||
| }; |
Member
Author
There was a problem hiding this comment.
No longer necessary. Now we just pass through the onChange.
Comment on lines
-211
to
217
| const onKeyDown = ( event: React.KeyboardEvent< HTMLDivElement > ) => { | ||
| // Ensures that custom validators are triggered when the user submits by pressing Enter, | ||
| // without ever blurring the control. | ||
| if ( event.key === 'Enter' ) { | ||
| onValidate?.(); | ||
| if ( statusMessage?.type ) { | ||
| return ( | ||
| <ValidityIndicator | ||
| type={ statusMessage.type } | ||
| message={ statusMessage.message } | ||
| /> | ||
| ); | ||
| } | ||
| return null; | ||
| }; |
Member
Author
There was a problem hiding this comment.
No longer necessary to handle this case explicitly.
| input:invalid[data-validity-visible] | ||
| ):not(:has([aria-expanded="true"])) { | ||
| border-color: $alert-red; | ||
| --wp-components-color-accent: #{$alert-red}; |
Member
Author
There was a problem hiding this comment.
Fixed a small visual bug here. The previous version wasn't correctly overriding a box-shadow color.
| .components-validated-control { | ||
| // For components based on InputBase | ||
| &:has(:is(input, select):user-invalid) | ||
| &:has(:is(input, select):invalid[data-validity-visible]) |
Member
Author
There was a problem hiding this comment.
Changed these selectors from :user-invalid (won't match our "touched" logic exactly) to :invalid plus an explicit data attribute that we add based on our own logic of when to start showing the error messages.
Member
There was a problem hiding this comment.
Changed these selectors from
:user-invalid(won't match our "touched" logic exactly) to:invalidplus an explicit data attribute that we add based on our own logic of when to start showing the error messages.
I think I'd missed this part of the current implementation until now. What's different between our logic and the logic of :user-invalid ? I quite like the idea if there's any way we could tap into or align with this native browser behavior. It seems pretty close to what we've implemented?
Member
Author
There was a problem hiding this comment.
The big reason we can't use :user-invalid is that we sometimes need to show the error even when the user has never interacted with the field.
A more subtle reason is that, in certain compound components, it can be more intuitive for the field to be considered touched only when the focus has left the "component boundary" completely, rather than simply leaving the input element. An example is a password field component with a show/hide button in the suffix. If the user only blurred the input element to toggle the show/hide button and resume editing the input, then they haven't left the component boundary and they're still in the process of editing the password field. This kind of "component atomicity" cannot be captured by a :user-invalid.
|
Flaky tests detected in c08c23b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19685140641
|
getdave
reviewed
Dec 2, 2025
Contributor
getdave
left a comment
getdave
left a comment
There was a problem hiding this comment.
Thank you for working on this.
Landing this would help unblock the problem I'm experiencing right now in #73486 whereby me triggering a custom validation does not show up until after the field is blurred.
I'm glad this means we remove onValidate because as someone trying to implement a validated form, it was quite confusing to understand why this was necessary.
| } | ||
| return label; | ||
| } | ||
| const VALIDITY_VISIBLE_ATTRIBUTE = 'data-validity-visible'; |
packages/components/src/validated-form-controls/control-with-error.tsx
Outdated
Show resolved
Hide resolved
packages/components/src/validated-form-controls/control-with-error.tsx
Outdated
Show resolved
Hide resolved
mirka
force-pushed
the
streamline-validation
branch
from
26e3a19 to
cc54e12
Compare
mirka
force-pushed
the
streamline-validation
branch
from
cc54e12 to
98c2c4e
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Member
Author
|
@oandregal I don't expect these changes to affect internal DataViews implementations, but heads up anyway. |
This was referenced Dec 4, 2025
aduth
approved these changes
Dec 4, 2025
Member
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
This feels a lot simpler and more predictable than the callback approach 👍
Comment on lines
40
to
41
| // TODO: Upstream limitation - When form is submitted when value is undefined, it will | ||
| // automatically set a clamped value (as defined by `min` attribute, so 0 by default). |
Member
There was a problem hiding this comment.
Is this comment still relevant?
Suggested change
| // TODO: Upstream limitation - When form is submitted when value is undefined, it will | |
| // automatically set a clamped value (as defined by `min` attribute, so 0 by default). |
Member
Author
| } | ||
| validityTarget?.addEventListener( 'invalid', handler ); | ||
| return () => validityTarget?.removeEventListener( 'invalid', handler ); | ||
| } ); |
Member
There was a problem hiding this comment.
Not technically introduced here, but should we be defining dependencies for this effect so that we're not binding / unbinding on every render? Maybe validityTarget should be assigned in the main render path and that becomes the dependency (along with state setter)?
Member
Author
There was a problem hiding this comment.
Shouldn't matter that much because I don't expect people to be memoizing their getValidityTarget functions, but let's add the dep so readers don't have to wonder about it. (3714604)
Comment on lines
+112
to
+115
| <style> | ||
| { ` | ||
| .my-control:has(:invalid[data-validity-visible]) .my-control__help:not(.is-visible) { | ||
| display: none; |
Member
There was a problem hiding this comment.
This pattern was added by designer requests, but I'm still not sure about it's scalability, given how complex the considerations can get in certain cases.
Yeah this was my first thought: Whether this is something we should even want to be encouraging. It assumes redundancy between help text and the error message, where ideally they'd be distinct enough to not need this kind of substitution. Maybe I'd need to see some more real-world examples, but I don't think the "duplication" is a terrible default experience that we should focus too much on improving the developer experience around this.
| } | ||
| return label; | ||
| } | ||
| const VALIDITY_VISIBLE_ATTRIBUTE = 'data-validity-visible'; |
Member
There was a problem hiding this comment.
This seems okay to me as well. If seeking an alternative, we could adapt language around "report"ing, for alignment to browser native APIs like reportValidity.
| .components-validated-control { | ||
| // For components based on InputBase | ||
| &:has(:is(input, select):user-invalid) | ||
| &:has(:is(input, select):invalid[data-validity-visible]) |
Member
There was a problem hiding this comment.
Changed these selectors from
:user-invalid(won't match our "touched" logic exactly) to:invalidplus an explicit data attribute that we add based on our own logic of when to start showing the error messages.
I think I'd missed this part of the current implementation until now. What's different between our logic and the logic of :user-invalid ? I quite like the idea if there's any way we could tap into or align with this native browser behavior. It seems pretty close to what we've implemented?
# Conflicts: # packages/components/CHANGELOG.md
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.
What?
Separates the HTML-level validity state from the visible display state of the validity messages.
Why/How?
Our validity states can be conceptualized as having two states:
As per the agreed upon spec, we only want to start showing the validity messages to the user after a field has been first blurred, marking it as "touched".
In our previous implementation, standard constraints (like
required) updated HTML-level validity state in real time, while custom validity tried to update HTML-level validity state in sync with the message visibility state. This meant that, at the HTML level, custom validity messages set by the consumer weren't marking the field as invalid until the first blur.This caused complications when a form is submitted with a field that would be considered invalid by the consumer's custom validity logic, but the HTML does not know that yet because the field has never blurred. We would have to trigger the custom validity logic imperatively on the
submitevents of the parent form.This was a bad choice in hindsight, so here we are restructuring the state so all validity state is updated real time at the HTML level. This gives us some benefits:
onValidateprop can be removed entirely, because now the consumer's custom validation logic should be triggered on every change (onChange).Punted to follow up
How to delay or prevent form submission when an async validation is still in flight (#71310).
Testing Instructions
Check stories for Validated Form Controls. Behaviors are basically the same, except for some minor improvements in the async validation example.
For code review, a commit-by-commit review should be easier. The bulk of the changed files are 8bfa861, 35d6513, and 6187246, which are just mechanical refactors and not very interesting.