-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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.
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
DataForm - Add combined fields support #65399
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { | ||
__experimentalHStack as HStack, | ||
__experimentalVStack as VStack, | ||
__experimentalHeading as Heading, | ||
__experimentalSpacer as Spacer, | ||
} from '@wordpress/components'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import type { DataFormCombinedEditProps, NormalizedField } from '../../types'; | ||
|
||
function Header( { title }: { title: string } ) { | ||
return ( | ||
<VStack className="dataforms-layouts__dropdown-header" spacing={ 4 }> | ||
<HStack alignment="center"> | ||
<Heading level={ 2 } size={ 13 }> | ||
{ title } | ||
</Heading> | ||
<Spacer /> | ||
</HStack> | ||
</VStack> | ||
); | ||
} | ||
|
||
function DataFormCombinedEdit< Item >( { | ||
field, | ||
data, | ||
onChange, | ||
hideLabelFromVision, | ||
}: DataFormCombinedEditProps< Item > ) { | ||
const className = 'dataforms-combined-edit'; | ||
const visibleChildren = ( field.children ?? [] ) | ||
.map( ( fieldId ) => field.fields.find( ( { id } ) => id === fieldId ) ) | ||
.filter( | ||
( childField ): childField is NormalizedField< Item > => | ||
!! childField | ||
); | ||
const children = visibleChildren.map( ( child, index ) => { | ||
return ( | ||
<div className="dataforms-combined-edit__field" key={ child.id }> | ||
{ index !== 0 && hideLabelFromVision && ( | ||
<Header title={ child.label } /> | ||
) } | ||
<child.Edit | ||
data={ data } | ||
field={ child } | ||
onChange={ onChange } | ||
hideLabelFromVision={ hideLabelFromVision } | ||
/> | ||
</div> | ||
); | ||
} ); | ||
|
||
const Stack = field.direction === 'horizontal' ? HStack : VStack; | ||
|
||
return ( | ||
<> | ||
{ ! hideLabelFromVision && <Header title={ field.label } /> } | ||
<Stack spacing={ 4 } className={ className }> | ||
{ children } | ||
</Stack> | ||
</> | ||
); | ||
} | ||
|
||
export default DataFormCombinedEdit; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
.dataforms-layouts-panel__field-dropdown { | ||
.dataforms-combined-edit { | ||
&__field:not(:first-child) { | ||
border-top: $border-width solid $gray-200; | ||
padding-top: $grid-unit-20; | ||
} | ||
} | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -7,6 +7,7 @@ import { useState } from '@wordpress/element'; | |||||||||||||||||||||||||||
* Internal dependencies | ||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||
import DataForm from '../index'; | ||||||||||||||||||||||||||||
import type { CombinedFormField } from '../../../types'; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const meta = { | ||||||||||||||||||||||||||||
title: 'DataViews/DataForm', | ||||||||||||||||||||||||||||
|
@@ -76,6 +77,11 @@ const fields = [ | |||||||||||||||||||||||||||
{ value: 'published', label: 'Published' }, | ||||||||||||||||||||||||||||
], | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
id: 'password', | ||||||||||||||||||||||||||||
label: 'Password', | ||||||||||||||||||||||||||||
type: 'text' as const, | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
]; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
export const Default = ( { type }: { type: 'panel' | 'regular' } ) => { | ||||||||||||||||||||||||||||
|
@@ -118,3 +124,44 @@ export const Default = ( { type }: { type: 'panel' | 'regular' } ) => { | |||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
export const CombinedFields = ( { type }: { type: 'panel' | 'regular' } ) => { | ||||||||||||||||||||||||||||
const [ post, setPost ] = useState( { | ||||||||||||||||||||||||||||
title: 'Hello, World!', | ||||||||||||||||||||||||||||
order: 2, | ||||||||||||||||||||||||||||
author: 1, | ||||||||||||||||||||||||||||
status: 'draft', | ||||||||||||||||||||||||||||
} ); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const form = { | ||||||||||||||||||||||||||||
fields: [ 'title', 'status_and_visibility', 'order', 'author' ], | ||||||||||||||||||||||||||||
layout: { | ||||||||||||||||||||||||||||
combinedFields: [ | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
id: 'status_and_visibility', | ||||||||||||||||||||||||||||
label: 'Status & Visibility', | ||||||||||||||||||||||||||||
children: [ 'status', 'password' ], | ||||||||||||||||||||||||||||
direction: 'vertical', | ||||||||||||||||||||||||||||
render: ( { item } ) => item.status, | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this function about? The thing that I'm wondering personally is whether for "forms" these are combined fields or more things like "groups"... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think having more example of how this translates in actual forms and panels would help make the right abstraction. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Its suppose to be the same as the
Groups would probably fit this use case better, the main example is the status field that includes the status radio control, posts schedule, the password field and also the sticky checkbox:
password-conditional.mp4We could also create a single status field that includes all of this logic, we did have to remove the gutenberg/packages/edit-site/src/components/post-fields/index.js Lines 283 to 294 in 3ec1ced
This is mostly a use case for the panel view. For the regular form one could render multiple regular data forms to create the grouping. Although there would be some additional use cases like the column use case for displaying two fields beside each other ( we have a similar use case in the product form for regular and sale price ): There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Would you envision "groups" to be like field types that accept children, or act in a similar manner as the combinedField API? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Do you think we can guess this from the "fields" or is the "render" function mandatory here? I'm also curious how the "combined fields" or "groups" feature translate in regular forms. What's the output there? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In this specific example it only shows the
In regular forms the main difference would be a title for the grouped fields and the option to display them in a column layout. Similar to the JSON Forms layout examples: https://jsonforms.io/examples/layouts/#group There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think that makes sense. Is it already working in this PR? I think my main concern here is to avoid special cases for layouts (panel or default) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't update the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I updated it to support the |
||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
] as CombinedFormField< any >[], | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||
<DataForm | ||||||||||||||||||||||||||||
data={ post } | ||||||||||||||||||||||||||||
fields={ fields } | ||||||||||||||||||||||||||||
form={ { | ||||||||||||||||||||||||||||
...form, | ||||||||||||||||||||||||||||
type, | ||||||||||||||||||||||||||||
} } | ||||||||||||||||||||||||||||
onChange={ ( edits ) => | ||||||||||||||||||||||||||||
setPost( ( prev ) => ( { | ||||||||||||||||||||||||||||
...prev, | ||||||||||||||||||||||||||||
...edits, | ||||||||||||||||||||||||||||
} ) ) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
}; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,29 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Internal dependencies | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { normalizeCombinedFields } from '../normalize-fields'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import type { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Field, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
CombinedFormField, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
NormalizedCombinedFormField, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} from '../types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export function getVisibleFields( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fields: Field< any >[], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
formFields: string[] = [], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
combinedFields?: CombinedFormField< any >[] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
): Field< any >[] { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const visibleFields: Array< | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Field< any > | NormalizedCombinedFormField< any > | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
> = [ ...fields ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if ( combinedFields ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
visibleFields.push( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
...normalizeCombinedFields( combinedFields, fields ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return formFields | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.map( ( fieldId ) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
visibleFields.find( ( { id } ) => id === fieldId ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.filter( ( field ): field is Field< any > => !! field ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+11
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest maintaining consistency with the current typing by using
Suggested change
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,8 +2,14 @@ | |||||
* Internal dependencies | ||||||
*/ | ||||||
import getFieldTypeDefinition from './field-types'; | ||||||
import type { Field, NormalizedField } from './types'; | ||||||
import type { | ||||||
CombinedFormField, | ||||||
Field, | ||||||
NormalizedField, | ||||||
NormalizedCombinedFormField, | ||||||
} from './types'; | ||||||
import { getControl } from './dataform-controls'; | ||||||
import DataFormCombinedEdit from './components/dataform-combined-edit'; | ||||||
|
||||||
/** | ||||||
* Apply default values and normalize the fields config. | ||||||
|
@@ -66,3 +72,29 @@ export function normalizeFields< Item >( | |||||
}; | ||||||
} ); | ||||||
} | ||||||
|
||||||
/** | ||||||
* Apply default values and normalize the fields config. | ||||||
* | ||||||
* @param combinedFields combined field list. | ||||||
* @param fields Fields config. | ||||||
* @return Normalized fields config. | ||||||
*/ | ||||||
export function normalizeCombinedFields< Item >( | ||||||
combinedFields: CombinedFormField< Item >[], | ||||||
fields: Field< Item >[] | ||||||
): NormalizedCombinedFormField< Item >[] { | ||||||
return combinedFields.map( ( combinedField ) => { | ||||||
return { | ||||||
...combinedField, | ||||||
Edit: DataFormCombinedEdit, | ||||||
fields: normalizeFields( | ||||||
combinedField.children | ||||||
.map( ( fieldId ) => | ||||||
fields.find( ( { id } ) => id === fieldId ) | ||||||
) | ||||||
.filter( ( field ): field is Field< any > => !! field ) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
), | ||||||
}; | ||||||
} ); | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this check necessary?