Skip to content

Conversation

@oandregal
Copy link
Member

@oandregal oandregal commented Sep 15, 2025

Part of #71500

What?

Add support for required and custom validation in toggle group control.

Why?

So that all controls support basic validation.

How?

Use the existing ValidatedToggleGroupControl component.

Testing Instructions

In the storybook (npm run storybook:dev) verify that it can take required and custom validation in the "DataViews > DataForm > Validation" story:

Screen.Recording.2025-09-15.at.16.51.47.mov

@github-actions
Copy link

github-actions bot commented Sep 15, 2025

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: oandregal <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: andrewserong <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@oandregal oandregal self-assigned this Sep 15, 2025
@oandregal oandregal added [Type] Enhancement A suggestion for improvement. [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond labels Sep 15, 2025
@oandregal oandregal mentioned this pull request Sep 15, 2025
4 tasks
@oandregal oandregal requested a review from mirka September 15, 2025 14:52
);
return (
<ToggleGroupControl
<ValidatedToggleGroupControl
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mirka Unless I'm doing something wrong, it looks like the required validation is not working for the ValidatedToggleGroupControl. It should be triggered when:

  • the control starts with no value selected
  • it gets focus and then loses it

The same happens in the component story.

Do you have any thoughts here (feel free to push to this PR if that helps)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the report. Should be fixed as of b3eb849. (It was a small workaround for an upstream bug, but that was already fixed in #69969.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In testing, it looks like if I tab to the control and tab away without selecting anything, it appears to correctly fire the validation. The validation doesn't appear to fire on the initial load of the story. That sounds fine to me, as the label indicates it's a required field, but just thought I'd double check that's what you were expecting, André?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation doesn't appear to fire on the initial load of the story. That sounds fine to me, as the label indicates it's a required field, but just thought I'd double check that's what you were expecting, André?

Yeah, that's how validation works at the component level: it runs on blur the 1st time, then upon every user change.

Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests well for me, and the code change looks straightforward:

Invalid Valid
image image

LGTM!

);
return (
<ToggleGroupControl
<ValidatedToggleGroupControl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In testing, it looks like if I tab to the control and tab away without selecting anything, it appears to correctly fire the validation. The validation doesn't appear to fire on the initial load of the story. That sounds fine to me, as the label indicates it's a required field, but just thought I'd double check that's what you were expecting, André?

@oandregal oandregal force-pushed the update/dataform-validated-toggle-group branch from b3eb849 to 3630af8 Compare September 16, 2025 07:14
@oandregal oandregal enabled auto-merge (squash) September 16, 2025 07:22
@oandregal oandregal merged commit dba52d8 into trunk Sep 16, 2025
79 of 81 checks passed
@oandregal oandregal deleted the update/dataform-validated-toggle-group branch September 16, 2025 08:29
@github-actions github-actions bot added this to the Gutenberg 21.7 milestone Sep 16, 2025
@github-actions
Copy link

Flaky tests detected in e3ec46f.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/17757849558
📝 Reported issues:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants