ValidatedToggleControl: Fix validation timing with callback ref #75095
+16
−10
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
ntsekouras
added
[Type] Bug
|
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. |
ntsekouras
requested review from
aduth,
gigitux,
jorgefilipecosta,
mirka and
oandregal
|
Size Change: +15 B (0%) Total Size: 3 MB
ℹ️ View Unchanged
|
aduth
reviewed
Jan 30, 2026
Member
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
Could we add some test coverage for the bug?
| const validityTargetRef = useRef< HTMLInputElement >( null ); | ||
| const mergedRefs = useMergeRefs( [ forwardedRef, validityTargetRef ] ); | ||
|
|
||
| const validityTargetRef = useRef< HTMLInputElement | null >( null ); |
Member
There was a problem hiding this comment.
What's the reason for this change? And similarly in the setRequiredRef argument typings below, would it ever actually be passed null, or could it be HTMLInputElement ?
| }, [ required ] ); | ||
| // so we need to set it manually. Using a callback ref ensures `required` is set | ||
| // synchronously when the element mounts, before any validation effects run. | ||
| const setRequiredRef = useCallback( |
Member
There was a problem hiding this comment.
Does this need to be a useCallback, or could it just be a plain function?
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?
Closes: #72321
This PR implements the last item of the linked issue around DataForms layout validation.
Why?
ValidatedToggleControlwas setting the required attribute on the underlying input via a useEffect. WhenreportValidity()was called (e.g., when a card form expands), the effect hadn't run yet, so the input didn't have therequiredattribute. Withoutrequired, an unchecked checkbox is considered valid by the browser, so no invalid event fired and no error indicator/message appeared.How?
Replace the useEffect with a callback ref. The callback ref sets required synchronously when the element mounts, ensuring it's in place before any validation effects run.
Possible alternative approach
We could make
ToggleControlpass rest props down toFormToggle(similar toTextControletc..). This would allowrequiredto flow through naturally via ControlWithError's cloneElement. Should we implement that? --cc @WordPress/gutenberg-componentsTesting Instructions
?path=/story/dataviews-dataform--validation&args=layout:card-collapsible) compare trunk and this PR (npm run storybook:dev) by going toBoolean fieldsand collapse the card2 validation issuesbadge, but when opening the card again you can see both validation messages at the inputs, only in this PR (see the videos below).Before
Screen.Recording.2026-01-30.at.5.08.05.PM.mov
After
Screen.Recording.2026-01-30.at.5.06.48.PM.mov