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. |
I understand wanting to separate things and arguing that it's (A scenario that just occurred to me is the ability to define these fields dynamically depending on the backend version, or depending on the block schema version, etc. The form can be defined more semantically, while the field definitions can absorb imperfections or transitions in the data sources.) Perhaps what "group" means is "if I had full control — or had no backcompat to deal with — this would have been an 'object' from the start". 🤔 As always, it's a double-edged sword. The downside of "group" is that it could be seen as an indirection. But on the other hand, without it, we risk delegating too much to the form API and muddy its semantics. For example, As an aside, it occurred to me while talking to André that the group type could also be named mapping. This would reduce the confusion if we choose to have both |
In that case, it's the responsibility of the "form" property to render the form in the way we want. The "fields"/"schema" should be just about the shape of the data IMO |
Exactly, in other words, it's the responsibility of the blocks to migrate to a new version potentially (or accept some temporary migration code in the framework), but this shouldn't force us to break the DataForm and Fields API abstractions. |
|
I actually don't mind some kind of "mapping" to exist but that should be in the "form" object. |
Yeah, I also suggested this approach. However, this is not enough to support custom Edit interactions like the ones we already support in the inspector. Form grouping only enables us to either display all controls up-front (regular layout) or behind a modal (panel layout) that we open via a generic button provided by the framework. This is the link as a regular layout (click to see diff):diff --git a/packages/block-library/src/image/index.js b/packages/block-library/src/image/index.js
index ec3ab836244..d45baacf393 100644
--- a/packages/block-library/src/image/index.js
+++ b/packages/block-library/src/image/index.js
@@ -90,22 +90,22 @@ if ( window.__experimentalContentOnlyInspectorFields ) {
caption: value.caption,
} ),
},
- {
- id: 'link',
- label: __( 'Link' ),
- type: 'url',
- Edit: 'link', // TODO: replace with custom component
- getValue: ( { item } ) => ( {
- url: item.href,
- rel: item.rel,
- linkTarget: item.linkTarget,
- } ),
- setValue: ( { value } ) => ( {
- href: value.url,
- rel: value.rel,
- linkTarget: value.linkTarget,
- } ),
- },
+ // {
+ // id: 'link',
+ // label: __( 'Link' ),
+ // type: 'url',
+ // Edit: 'link', // TODO: replace with custom component
+ // getValue: ( { item } ) => ( {
+ // url: item.href,
+ // rel: item.rel,
+ // linkTarget: item.linkTarget,
+ // } ),
+ // setValue: ( { value } ) => ( {
+ // href: value.url,
+ // rel: value.rel,
+ // linkTarget: value.linkTarget,
+ // } ),
+ // },
{
id: 'caption',
label: __( 'Caption' ),
@@ -117,9 +117,24 @@ if ( window.__experimentalContentOnlyInspectorFields ) {
label: __( 'Alt text' ),
type: 'text',
},
+ {
+ id: 'href',
+ label: __( 'Link' ),
+ type: 'url',
+ },
+ {
+ id: 'linkTarget',
+ label: __( 'Open in new tab' ),
+ type: 'boolean',
+ },
];
settings[ formKey ] = {
- fields: [ 'image', 'link', 'caption', 'alt' ],
+ fields: [
+ 'image',
+ { id: 'link', children: [ 'href', 'linkTarget' ] },
+ 'caption',
+ 'alt',
+ ],
};
}Screen.Recording.2026-01-15.at.18.22.57.movThis is the link as a panel layout (click to see diff):diff --git a/packages/block-library/src/image/index.js b/packages/block-library/src/image/index.js
index ec3ab836244..1b2fcdd9785 100644
--- a/packages/block-library/src/image/index.js
+++ b/packages/block-library/src/image/index.js
@@ -90,22 +90,22 @@ if ( window.__experimentalContentOnlyInspectorFields ) {
caption: value.caption,
} ),
},
- {
- id: 'link',
- label: __( 'Link' ),
- type: 'url',
- Edit: 'link', // TODO: replace with custom component
- getValue: ( { item } ) => ( {
- url: item.href,
- rel: item.rel,
- linkTarget: item.linkTarget,
- } ),
- setValue: ( { value } ) => ( {
- href: value.url,
- rel: value.rel,
- linkTarget: value.linkTarget,
- } ),
- },
+ // {
+ // id: 'link',
+ // label: __( 'Link' ),
+ // type: 'url',
+ // Edit: 'link', // TODO: replace with custom component
+ // getValue: ( { item } ) => ( {
+ // url: item.href,
+ // rel: item.rel,
+ // linkTarget: item.linkTarget,
+ // } ),
+ // setValue: ( { value } ) => ( {
+ // href: value.url,
+ // rel: value.rel,
+ // linkTarget: value.linkTarget,
+ // } ),
+ // },
{
id: 'caption',
label: __( 'Caption' ),
@@ -117,9 +117,29 @@ if ( window.__experimentalContentOnlyInspectorFields ) {
label: __( 'Alt text' ),
type: 'text',
},
+ {
+ id: 'href',
+ label: __( 'Link' ),
+ type: 'url',
+ },
+ {
+ id: 'linkTarget',
+ label: __( 'Open in new tab' ),
+ type: 'boolean',
+ },
];
settings[ formKey ] = {
- fields: [ 'image', 'link', 'caption', 'alt' ],
+ fields: [
+ 'image',
+ {
+ id: 'link',
+ label: 'Link',
+ layout: { type: 'panel', labelPosition: 'top' },
+ children: [ 'href', 'linkTarget' ],
+ },
+ 'caption',
+ 'alt',
+ ],
};
}Screen.Recording.2026-01-15.at.18.28.00.movNow, compare that to the existing interaction for links in the image block: Screen.Recording.2026-01-15.at.17.55.22.mov
Or this other example with media for the cover block: Screen.Recording.2026-01-15.at.17.56.50.mov
Supporting this level of interaction needs a custom Edit. We could make BlockField/ContentOnly work only with the Taking all of this into account, I feel we need to be data-shape agnostic (support both neatly grouped objects and flat structures). |
Yes, but we already are, the data is defined in "fields" and the agnostic part (adapting the data to the rendering) is the "form". I think we start mixing these concerns, we're going to break the abstractions, it becomes very unclear what should go into "fields" and what should go into "form". (we kind of already started with some of the configs, but we should probably no go far) Would it be possible to actually update the "form" config (maybe new configs there) to overcome the limitations that you mention? |
|
I'm exploring an alternative to remove getValue/setValue functions from the blocks at #74900 |
|
Isn't the |

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.