-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Field API: add support for object type
#74409
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
base: trunk
Are you sure you want to change the base?
Conversation
|
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. |
|
@talldan @andrewserong what would be a good attribute that represents an I looked at the current status for auto-generating forms from content-only blocks, specifically this link example. But then I realized Screen.Recording.2026-01-07.at.20.35.49.movWith the following diff:diff --git a/packages/block-library/src/image/index.js b/packages/block-library/src/image/index.js
index fc65703a23c..be43ffeac5f 100644
--- a/packages/block-library/src/image/index.js
+++ b/packages/block-library/src/image/index.js
@@ -84,15 +84,14 @@ if ( window.__experimentalContentOnlyInspectorFields ) {
},
},
{
- id: 'link',
- label: __( 'Link' ),
- type: 'link',
- mapping: {
- url: 'href',
- rel: 'rel',
- linkTarget: 'linkTarget',
- destination: 'linkDestination',
- },
+ id: 'url',
+ label: __( 'URL' ),
+ type: 'url',
+ },
+ {
+ id: 'linkTarget',
+ label: __( 'Link target' ),
+ type: 'boolean',
},
{
id: 'caption',
@@ -106,7 +105,15 @@ if ( window.__experimentalContentOnlyInspectorFields ) {
},
];
settings[ formKey ] = {
- fields: [ 'image' ],
+ fields: [
+ 'image',
+ {
+ id: 'link',
+ label: __( 'Link' ),
+ layout: { type: 'panel', labelPosition: 'top' },
+ children: [ 'url', 'linkTarget' ],
+ },
+ ],
};
}I've got this UI: Screen.Recording.2026-01-07.at.21.35.33.movWould you be able to look at that direction to remove |
Interesting point, thanks for mentioning that!
Media is the best example. For example, when adding a media library image to an image block it'll usually set As part of this we also need to look at aligning the media field used for blocks with the one @ntsekouras has been working on (#74336). |
|
@oandregal I've attempted to use the It does work, but there's still some mapping required from the field keys to the block attribute keys. It'd be interesting to know if it's possible to remove the need for any mapping. Block attributes also don't tend to be in a nested structure like this: They're instead all at the root: So in setValues/getValues the values have to be flattened. |
|
This PR makes me wonder why "properties" is not named "fields". I guess to stay close to JSON Schema. This is good I think. But I wonder we should rename the "fields" prop of DataViews and DataForm to "schema". It feels like a better name and maybe offer a deprecation for some time. Just food for thoughts. |
122b8ab to
3742e56
Compare
|
At 3742e56 I've pushed an additional field type: {
id: 'imageGroup',
type: 'group',
label: 'Image',
description: 'Group field with flat data (not nested).',
properties: {
imageId: { id: 'imageId', type: 'text', label: 'Image ID' },
imageUrl: { id: 'imageUrl', type: 'url', label: 'URL' },
imageCaption: { id: 'imageCaption', type: 'text', label: 'Caption' },
},
},I'd like to think more about this but wanted to push anyway for you to play with, so timezones work to our favor. What I like about having both is that it's easy to reason about
|
|
Thanks for making that change @oandregal. I think it works. Is the idea that the I think it'd be great if we can find a way to support something like So for example, the image block stores media like this - The const { id, url } = field.getValue( { item: { mediaId: 25, mediaUrl: 'picture.jpg' } } );
field.setValue( { item: data, value: { id: 26, url: 'test.jpg' } } ); // { mediaId: 26, mediaUrl: 'test.jpg' }I thought it may be possible to achieve it with a field definition like this (the property key is the internal key the Edit component uses and the {
id: 'media',
type: 'group', // could also be 'object',
properties: {
id: { id: 'mediaId', type: 'number', label: __( 'Id' ) },
url: { id: 'mediaUrl', type: 'url', label: __( 'Url' ) }
}
}Something else that comes to mind is whether property ids should have parity with regular field ids, and support the dot notation. That might be useful with a group, as it'd allow grouping values that are not colocated into a single field: {
id: 'media',
type: 'group',
properties: {
id: { id: 'deeply.nested.media.id', type: 'number', label: __( 'Id' ) },
url: { id: 'shallowUrl', type: 'url', label: __( 'Url' ) }
}
}However, one outcome of this is that it means {
id: 'media',
type: 'group',
properties: {
id: { id: 'media.id', type: 'number', label: __( 'Id' ) },
url: { id: 'media.url', type: 'url', label: __( 'Url' ) }
}
}So perhaps that hints at not needing two separate types for object and group. |
I guess this could be achieved by the field's
@jameskoster right now the designs for MediaEdit have no concept of handling an external image. Has this been considered? |
|
I don't like the "group" type personally. The schema/fields should be about defining the shape of the data (not defining something for the form to render). Which means if a property is top level, it shouldn't use the "object" type (or group), it should just be defined as a "top" level field. The "group" field breaks that, and IMO this is a concern of the DataForm |
It has not. Do you think it should be part of the same control? My initial reaction is that selecting from (and uploading to) the Media Library feels like a separate consideration to inserting an external image. |
Not sure yet. I agree that it feels like a separate consideration. I'll see to explore though in that area. |
Yes it does :) |
|
I've prepared #74575 to remove I'd like to land that PR and then revisit the conversation on this one. By landing 74575 first, we have more clarity about the problem to solve in this PR. |

What?
This PR adds support for the
objectfield type in the Field API.Example of an
objectfield type:Why?
The work for content-only has surfaced this as a priority, see #73374 (comment)
How?
objectfield type.objectdataform control.Testing Instructions
npm install && npm run storybook:dev).There are two objects in the story,
linkandaddressfields:Screen.Recording.2026-01-07.at.19.42.46.mov
TODO
objecttypes.