DataForm: simplify form normalization #72848
Conversation
oandregal
added
[Type] Enhancement
| <div className="dataforms-layouts-row__field-control"> | ||
| <RegularLayout | ||
| data={ data } | ||
| field={ fieldDefinition } |
There was a problem hiding this comment.
This was a bug: we were passing a field definition, but the regular layout expected the form field.
There was a problem hiding this comment.
This is the type of field that would have used it:
{
id: 'name',
layout: { type: 'row' },
},
It still worked fine at runtime because the only thing we check is the field.id (and that's the same as the field definition).
I'm not sure why TypeScript didn't raise this issue. The type expected by RegularLayout is a FormField (see), but the fieldDefinition is a NormalizedField.
| export type SimpleFormField = { | ||
| id: string; | ||
| layout?: Layout; | ||
| label?: string; |
There was a problem hiding this comment.
While unsupported by the type, label and description were passed down in the normalization. This just clarifies the types to support them by having a single FormField type.
| onChange, | ||
| validity, | ||
| }: DataFormProps< Item > ) { | ||
| const normalizedForm = useMemo( () => normalizeForm( form ), [ form ] ); |
There was a problem hiding this comment.
By doing this at this level, layouts and any other components do not need to normalize anything. They can simply operate with the normalized versions of layout and fields.
| const { fields } = useContext( DataFormContext ); | ||
| const layout = field.layout as NormalizedCardLayout; | ||
|
|
||
| const layout: NormalizedCardLayout = normalizeLayout( { |
There was a problem hiding this comment.
This is the kind of computation we get rid of. It means developing a layout is simpler.
oandregal
force-pushed
the
update/normalize-form-fields
branch
from
bc00954 to
eee6d62
Compare
| Array.isArray( field.children ) && { | ||
| children: normalizeForm( { | ||
| fields: field.children, | ||
| layout: DEFAULT_LAYOUT, |
There was a problem hiding this comment.
Another win: clarifying and add test for how the layout is inherited across nested levels. Only the top-level fields inherit the form layout. Any other layers, are normalized to use the default (regular).
|
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. |
|
Flaky tests detected in ee3bdf8. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18971898579
|
|
I'm going to merge this refactor to continue work at #72588 |
What?
This PR is a refactor: doesn't change the behavior, just the internal structure of the code.
It normalizes all the form fields at any nested level in a single place, while before this normalization happened at several layers (the top-level ones in DataForm, some others in the specific layout, etc.).
Why?
To simplify the code. And to support this follow-up PR #72588
How?
normalizedFormmethod that substitutes tonormalizeFormFieldsandnormalizeLayout.normalizeFormmethod recursively normalizes any nested level.SimpleFormFieldandCombinedFormFieldinto oneFormField.Testing Instructions
npm run storybook:dev) as compared to the storybook for trunk (see online).