Skip to content

useFormValidity: fix nesting levels #72588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
oandregal merged 8 commits into from
Nov 4, 2025
Merged

useFormValidity: fix nesting levels #72588

oandregal merged 8 commits into from
Nov 4, 2025

Conversation

@oandregal
Copy link
Member

@oandregal oandregal commented Oct 22, 2025 •
edited
Loading

Follow-up to #71412, #72848

What?

This PR makes the useFormValidity hook work for any nested level.

Why?

It was only working for fields up to one nesting level.

All fields here were validated:

fields: [
  'topLevelField',
  {
    id: 'level1',
    children: [ 'fieldAtLevel1' ],
  }
]

In this case, all fields but fieldAtLevel2 were validated:

fields: [
  'topLevelField',
  {
    id: 'level1',
    children: [ 'fieldAtLevel1' ],
  },
  {
    id: 'level1',
    children: [
      { id: 'level2', children: [ 'fieldAtLevel2' ] }
    ],
  }
]

How

No API changes.

  • useFormValidity has been updated to work recursively and work at any nesting level.
  • The story has been updated to consider fields up to the 4th nesting level.

Testing Instructions

  • Automated tests should pass.
  • Open the storybook locally (npm run storybook:dev), and navigate to "DataViews > DataForm > Validation". Verify the validation messages are triggered as expected.

The story has been updated so that some fields have up to 4 nesting levels. If you want to reproduce the issue in trunk paste the existing form config there and observe how fields with nested levels > 2 were not validated.

@oandregal oandregal self-assigned this Oct 22, 2025
@oandregal oandregal added [Type] Bug An existing feature does not function as intended [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond labels Oct 22, 2025
@oandregal oandregal force-pushed the fix/useformvalidity-nesting branch 3 times, most recently from 6df982a to 9a5462c Compare October 30, 2025 13:38
@oandregal oandregal marked this pull request as ready for review October 30, 2025 13:47
@github-actions
Copy link

github-actions bot commented Oct 30, 2025 •
edited
Loading

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: youknowriad <[email protected]>
Co-authored-by: jorgefilipecosta <[email protected]>

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

@oandregal oandregal mentioned this pull request Oct 30, 2025
Merged
@oandregal
Copy link
Member Author

oandregal commented Oct 30, 2025

Extracted #72848 to simplify this PR.

@oandregal oandregal force-pushed the fix/useformvalidity-nesting branch from 9a5462c to 2631768 Compare October 31, 2025 13:16
oandregal
oandregal commented Oct 31, 2025
View reviewed changes
packages/dataviews/src/hooks/use-form-validity.ts
*
* @return Record of field IDs to error messages (undefined means no error).
*/
export function useFormValidity< Item >(
Copy link
Member Author

@oandregal oandregal Oct 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This function has been completely refactored, so the diff would not be super helpful to understand what's actually changed. For review, I recommend just taking a look at how it works now and ignore the previous version.

Copy link
Contributor

@youknowriad youknowriad Oct 31, 2025

Choose a reason for hiding this comment

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

I think this PR would have been a good opportunity to refactor to delegate the validation behavior to the field types instead of centralizing it here.

Copy link
Member Author

@oandregal oandregal Nov 3, 2025

Choose a reason for hiding this comment

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

This PR updates the core flow to fix the existing issue. Extracting the existing logic to the field type definitions is an extra thing on top of what this PR already does. The PR is already big as it is, so I thought I'd control scope by focusing just on fixing the issue.

oandregal
oandregal commented Oct 31, 2025
View reviewed changes
packages/dataviews/src/stories/dataform.story.tsx
) => {
return <DateControl { ...props } operator="between" />;
};
const makeAsync = (
Copy link
Member Author

@oandregal oandregal Oct 31, 2025

Choose a reason for hiding this comment

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

None of these little functions were updated, just moved around.

oandregal
oandregal commented Oct 31, 2025
View reviewed changes
packages/dataviews/src/stories/dataform.story.tsx
required,
elements: elements !== 'none' ? true : false,
custom: maybeCustomRule( customTelephoneRule ),
{
Copy link
Member Author

@oandregal oandregal Oct 31, 2025

Choose a reason for hiding this comment

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

Also moved some fields around to better highlight and group them in the validation story.

oandregal
oandregal commented Oct 31, 2025
View reviewed changes
packages/dataviews/src/stories/dataform.story.tsx
required,
elements: elements !== 'none' ? true : false,
{
id: 'array',
Copy link
Member Author

@oandregal oandregal Oct 31, 2025

Choose a reason for hiding this comment

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

changed countries to array because any other field type just uses the type as the id/label.

oandregal
oandregal commented Oct 31, 2025
View reviewed changes
packages/dataviews/src/stories/dataform.story.tsx

const form = useMemo(
() => ( {
fields: [
Copy link
Member Author

@oandregal oandregal Oct 31, 2025

Choose a reason for hiding this comment

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

The fields array is the relevant change for this story. Added some nested fields.

jorgefilipecosta
jorgefilipecosta reviewed Nov 3, 2025
View reviewed changes
jorgefilipecosta
jorgefilipecosta reviewed Nov 3, 2025
View reviewed changes
jorgefilipecosta
jorgefilipecosta reviewed Nov 3, 2025
View reviewed changes
jorgefilipecosta
jorgefilipecosta approved these changes Nov 3, 2025
View reviewed changes
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

I left some comments for consideration but things tested well for me 👍

@oandregal oandregal force-pushed the fix/useformvalidity-nesting branch from 2631768 to e2c279c Compare November 4, 2025 09:05
@oandregal
Copy link
Member Author

oandregal commented Nov 4, 2025

Thanks for your review @jorgefilipecosta! I've addressed all your comments, and set this up for auto-merge.

@oandregal oandregal enabled auto-merge (squash) November 4, 2025 09:06
@github-actions
Copy link

github-actions bot commented Nov 4, 2025

Flaky tests detected in e2c279c.
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/19063367555
📝 Reported issues:

@oandregal oandregal merged commit 0568def into trunk Nov 4, 2025
34 checks passed
@oandregal oandregal deleted the fix/useformvalidity-nesting branch November 4, 2025 09:41
@github-actions github-actions bot added this to the Gutenberg 22.1 milestone Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jorgefilipecosta jorgefilipecosta jorgefilipecosta approved these changes

@youknowriad youknowriad Awaiting requested review from youknowriad

Assignees

@oandregal oandregal

Labels

[Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond [Type] Bug An existing feature does not function as intended

Projects

None yet

Milestone

Gutenberg 22.1

Development

Successfully merging this pull request may close these issues.

4 participants

@oandregal @youknowriad @jorgefilipecosta