Content-only: remove mapping and args in favor of DataForm API
#74575
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oandregal
added
[Status] In Progress
github-actions
bot
added
[Package] Block library
oandregal
added
the
[Type] Code Quality
oandregal
commented
Jan 13, 2026
| const { allowedTypes = [], multiple = false } = fieldDef.args || {}; | ||
|
|
||
| // Check if featured image is supported by checking if it's in the mapping | ||
| const hasFeaturedImageSupport = |
Member
Author
There was a problem hiding this comment.
This is the kind of thing that's hidden behind a custom API: we were using the mapping instead of the Edit config to decide whether to enable featuredImage for the media control. See cover block.
|
Size Change: +196 B (+0.01%) Total Size: 3.05 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 7252f85. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/20951670703
|
oandregal
requested review from
ajitbohra,
fabiankaegy and
tellthemachines
as code owners
|
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. |
oandregal
commented
Jan 13, 2026
Comment on lines
+105
to
+131
| // These should be custom Edit components, not replaced here. | ||
| // | ||
| // - richtext control: it needs clientId | ||
| // - link control: does not need anything extra | ||
| // - media control: needs the Edit config | ||
| if ( | ||
| 'string' === typeof fieldDef.Edit && | ||
| fieldDef.Edit === 'richtext' | ||
| ) { | ||
| const RichText = CONTROLS[ 'richtext' ]; | ||
| field.Edit = createConfiguredControl( RichText, { | ||
| clientId, | ||
| } ); | ||
| } else if ( | ||
| 'string' === typeof fieldDef.Edit && | ||
| fieldDef.Edit === 'link' | ||
| ) { | ||
| const Link = CONTROLS[ 'link' ]; | ||
| field.Edit = createConfiguredControl( Link ); | ||
| } else if ( | ||
| 'object' === typeof fieldDef.Edit && | ||
| fieldDef.Edit.control === 'media' | ||
| ) { | ||
| const Media = CONTROLS[ 'media' ]; | ||
| field.Edit = createConfiguredControl( Media, { | ||
| ...fieldDef.Edit, | ||
| } ); |
Member
Author
There was a problem hiding this comment.
All of this should be removed in favor of the fields declaring a custom Edit. For example:
{
id: 'image',
label: __( 'Image' ),
type: 'media',
Edit: ( props ) => ( <CustomEdit {...props} allowedTypes={['image']} /> ),
},
{
id: 'link',
label: __( 'Link' ),
type: 'url',
Edit: ( props ) => ( <CustomLink {...props} /> ),
},I wanted to do it, but the current media/link controls are too intertwined with block-editor. These controls may either become the default dataform controls or live in a wordpress-specific package such as fields.
The case of richtext is a bit more special and needs more thinking. But this approach highlights that this is one of the core "problems to solve".
oandregal
commented
Jan 13, 2026
| url: 'href', | ||
| rel: 'rel', | ||
| linkTarget: 'linkTarget', | ||
| destination: 'linkDestination', |
Member
Author
There was a problem hiding this comment.
I could not find any destination in the Link control, so this was never updated.
Member
Author
|
@talldan @andrewserong this PR is ready to land from my point of view, unless something is breaking in your testing or I've missed anything? |
oandregal
changed the title
mapping and args in favor of DataForm API
Contributor
|
Thanks for working on this! I haven't tested this yet, but just sharing a question from @mcsf on #73374 (#73374 (comment)) to make sure it's captured here, too: should we use |
talldan
reviewed
Jan 14, 2026
Comment on lines
+50
to
+57
| getValue: ( { item } ) => ( { | ||
| id: item.id, | ||
| url: item.src, | ||
| } ), | ||
| setValue: ( { value } ) => ( { | ||
| id: value.id, | ||
| src: value.url, | ||
| } ), |
Contributor
There was a problem hiding this comment.
One of the reasons I'd avoided moving these functions here is that a lot of this config would ideally be migrated to the attribute definition in block.json where it isn't possible to define a function.
The same goes for Edit.control where a string would have to be used.
Contributor
|
Thanks for the PR, I was planning to address this too, but thought about waiting until after #74409. Happy to move forwards with this though the biggest question I have is Related discussion here - #73863 (comment). |
Contributor
There was a problem hiding this comment.
Thanks for the continued work on honing in on a better API for all this!
I noticed a couple of issues with the Cover block, where the mapping appears to be off. I haven't tested blocks exhaustively as Dan pointed out this mightn't necessarily be the right path forward (i.e. do we want to stick with functions for now?), but hopefully the Cover block example points to the kind of category of bug we might have here. Let me know if you need a more thorough look tomorrow / later on in the week!
All that said, I'm not opposed to the idea of using getValue and setValue in the shorter-term if it helps us feel comfortable with the API shape as an interim step, or if it helps give some breathing room to the discussions re: a group/object control type.
Comment on lines
+77
to
+90
| getValue: ( { item } ) => ( { | ||
| id: item.id, | ||
| url: item.url, | ||
| alt: item.alt, | ||
| mediaType: item.mediaType, | ||
| useFeaturedImage: item.featuredImage, | ||
| } ), | ||
| setValue: ( { value } ) => ( { | ||
| id: value.id, | ||
| url: value.url, | ||
| alt: value.alt, | ||
| mediaType: value.mediaType, | ||
| featuredImage: value.useFeaturedImage, | ||
| } ), |
Contributor
There was a problem hiding this comment.
mediaType doesn't appear to be working properly here. I think it needs to map to backgroundType? Currently if I set it to a video background it doesn't work as it's attempting to use the mp4 url for the background image instead of as a video background:
Similarly, the block attribute for this block is useFeaturedImage and not featuredImage, so the toggle doesn't appear to work for that:
Whereas it should be selected like this (trunk):
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow-up to #73374 (comment)
What?
This PR removes the custom APIs (
mapping,args) from the content-only prototype, in favor of DataForm's API.Screen.Recording.2026-01-13.at.14.46.45.mov
Why
To surface "problems to solve" as soon as possible.
How?
mappingandargs. Fields implement the mapping viagetValue/setValue.Testing Instructions
Prerequisite: visit "Gutenberg > Experiments" and enable:
In the post editor:
Follow-ups
This PR highlights the most pressing follow-up: look for ways to use the controls from the block-library.
Other follow-ups:
objecttype #74409