Skip to content

useFormValidity: fix nesting levels - #72588

Merged
oandregal merged 8 commits into
trunkfrom
fix/useformvalidity-nesting
Nov 4, 2025
Merged

useFormValidity: fix nesting levels#72588
oandregal merged 8 commits into
trunkfrom
fix/useformvalidity-nesting

Conversation

@oandregal

@oandregal oandregal commented Oct 22, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

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

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

Copy link
Copy Markdown

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 <oandregal@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org>

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

oandregal commented Oct 30, 2025

Copy link
Copy Markdown
Member Author

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
Comment thread packages/dataviews/src/hooks/use-form-validity.ts
*
* @return Record of field IDs to error messages (undefined means no error).
*/
export function useFormValidity< Item >(

@oandregal oandregal Oct 31, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

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.

@youknowriad youknowriad Oct 31, 2025

Copy link
Copy Markdown
Contributor

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.

@oandregal oandregal Nov 3, 2025

Copy link
Copy Markdown
Member Author

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
Comment thread packages/dataviews/src/stories/dataform.story.tsx
) => {
return <DateControl { ...props } operator="between" />;
};
const makeAsync = (

@oandregal oandregal Oct 31, 2025

Copy link
Copy Markdown
Member Author

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
Comment thread packages/dataviews/src/stories/dataform.story.tsx
required,
elements: elements !== 'none' ? true : false,
custom: maybeCustomRule( customTelephoneRule ),
{

@oandregal oandregal Oct 31, 2025

Copy link
Copy Markdown
Member Author

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
Comment thread packages/dataviews/src/stories/dataform.story.tsx
required,
elements: elements !== 'none' ? true : false,
{
id: 'array',

@oandregal oandregal Oct 31, 2025

Copy link
Copy Markdown
Member Author

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
Comment thread packages/dataviews/src/stories/dataform.story.tsx

const form = useMemo(
() => ( {
fields: [

@oandregal oandregal Oct 31, 2025

Copy link
Copy Markdown
Member Author

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
Comment thread packages/dataviews/src/hooks/use-form-validity.ts Outdated
Comment thread packages/dataviews/src/hooks/use-form-validity.ts Outdated
jorgefilipecosta
jorgefilipecosta reviewed Nov 3, 2025
View reviewed changes
Comment thread packages/dataviews/src/hooks/use-form-validity.ts Outdated
jorgefilipecosta
jorgefilipecosta reviewed Nov 3, 2025
View reviewed changes
Comment thread packages/dataviews/src/hooks/use-form-validity.ts
jorgefilipecosta
jorgefilipecosta reviewed Nov 3, 2025
View reviewed changes
Comment thread packages/dataviews/src/hooks/use-form-validity.ts
jorgefilipecosta
jorgefilipecosta reviewed Nov 3, 2025
View reviewed changes
Comment thread packages/dataviews/src/hooks/use-form-validity.ts Outdated
jorgefilipecosta
jorgefilipecosta reviewed Nov 3, 2025
View reviewed changes
Comment thread packages/dataviews/src/stories/dataform.story.tsx
jorgefilipecosta
jorgefilipecosta approved these changes Nov 3, 2025
View reviewed changes

@jorgefilipecosta jorgefilipecosta left a comment

Copy link
Copy Markdown
Member

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

oandregal commented Nov 4, 2025

Copy link
Copy Markdown
Member Author

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

github-actions Bot commented Nov 4, 2025

Copy link
Copy Markdown

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
@ntsekouras ntsekouras mentioned this pull request Aug 11, 2026
Merged
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.

3 participants

@oandregal @youknowriad @jorgefilipecosta