Block Bindings: Add support to image caption attribute in block bindings#70642
Closed
cbravobernal wants to merge 16 commits intotrunkfrom
Closed
Block Bindings: Add support to image caption attribute in block bindings#70642cbravobernal wants to merge 16 commits intotrunkfrom
cbravobernal wants to merge 16 commits intotrunkfrom
Conversation
|
Size Change: +230 B (+0.01%) Total Size: 1.91 MB
ℹ️ View Unchanged
|
cbravobernal
force-pushed
the
experiment/add-caption-image-binding-option
branch
3 times, most recently
from
3b1b85a to
013493f
Compare
cbravobernal
requested review from
spacedmonkey
and removed request for
spacedmonkey
cbravobernal
changed the title
|
Flaky tests detected in e8f6ded. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/16879288790
|
cbravobernal
requested review from
ajitbohra,
artemiomorales,
ellatrix,
fabiankaegy,
spacedmonkey 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. |
31 tasks
ockham
force-pushed
the
experiment/add-caption-image-binding-option
branch
from
905144a to
e8f6ded
Compare
ockham
reviewed
Aug 11, 2025
Comment on lines
+16
to
+42
| function gutenberg_replace_pattern_override_default_binding( $parsed_block ) { | ||
| $supported_block_attrs = array( | ||
| 'core/paragraph' => array( 'content' ), | ||
| 'core/heading' => array( 'content' ), | ||
| 'core/image' => array( 'id', 'url', 'title', 'alt', 'caption' ), | ||
| 'core/button' => array( 'url', 'text', 'linkTarget', 'rel' ), | ||
| ); | ||
|
|
||
| $bindings = $parsed_block['attrs']['metadata']['bindings'] ?? array(); | ||
| if ( | ||
| isset( $bindings['__default']['source'] ) && | ||
| 'core/pattern-overrides' === $bindings['__default']['source'] | ||
| ) { | ||
| $updated_bindings = array(); | ||
|
|
||
| foreach ( $supported_block_attrs[ $parsed_block['blockName'] ] as $attribute_name ) { | ||
| $updated_bindings[ $attribute_name ] = isset( $bindings[ $attribute_name ] ) | ||
| ? $bindings[ $attribute_name ] | ||
| : array( 'source' => 'core/pattern-overrides' ); | ||
| } | ||
| $parsed_block['attrs']['metadata']['bindings'] = $updated_bindings; | ||
| } | ||
|
|
||
| return $parsed_block; | ||
| } | ||
|
|
||
| add_filter( 'render_block_data', 'gutenberg_replace_pattern_override_default_binding', 10, 1 ); |
Contributor
There was a problem hiding this comment.
Is this only needed to add caption as a supported attribute of the image block? FWIW, as of WP 6.9, we'll be able to do
add_filter( 'block_bindings_supported_attributes_core/image', function( $supported_attributes ) {
$supported_attributes[] = 'caption';
return $supported_attributes;
} );(For back-compat with 6.8, we cannot do this here yet.)
But I must be missing something else, since I tried using the block_bindings_supported_attributes_core/image filter instead of your gutenberg_replace_pattern_override_default_binding filter, and it didn't replace the captions 🤔
Member
There was a problem hiding this comment.
It won't replace the caption because the filter only handles feeding $attributes with values. The rest of the processing is mostly hardcoded as of today:
Contributor
There was a problem hiding this comment.
Yeah, but that's done by the other function, gutenberg_process_image_caption_binding further below (which is hooked into render_block_core/image', 'gutenberg_process_image_caption_binding), no?
I kept that one around, and yet replacing gutenberg_replace_pattern_override_default_binding with the snippet I shared above didn't work. I wasn't able to figure out what else I was missing 🤔
Contributor
|
This seems to be working well; I have yet to review the code in-depth. Looking at the updated test fixtures has me wonder however if there's anything we can do to avoid all the What's more is that they don't seem to add a lot of semantic information right now: All instances of The gutenberg/packages/block-library/src/image/block.json Lines 38 to 43 in 1b6ee21 ...we can tell from it that the caption is the rich text that's found between a Maybe we can generalize this? If an attribute that matches the above criteria ( I believe this would scale at least to Curious to hear you thoughts @cbravobernal @gziolo. I might give this a try to see how it goes. |
Member
|
@cbravobernal explored using a system similar to Interactivity API directives processing to automate these replacements in WordPress/wordpress-develop#9302. It would be up to the developer to annotate the replacement and handling of empty values using some new mechanism. This would largely simplify the implementation as the system wouldn't have to analyze the attributes and the definition of selectors, because everything would be encoded in the HTML saved in the database. Some examples: Pullquote (citation) { ( shouldShowCitation || hasCitationBinding ) && (
<RichText.Content
tagName="cite"
value={ citation }
data-wp-binding={ hasCitationBinding ? 'citation' : undefined }
/>
) }Image (caption) { ( ! RichText.isEmpty( caption ) || hasCaptionBinding ) && (
<RichText.Content
className={ __experimentalGetElementClassName( 'caption' ) }
tagName="figcaption"
value={ caption }
data-wp-binding={ hasCaptionBinding ? 'caption' : undefined }
/>
) }In that case, We also discussed that a similar mechanism might exist for HTML attributes with <img
src={ url }
alt={ alt }
data-wp-binding--src="url"
/>would mean replace the default value for
#70975 is an example with the less appealing syntax, but the |
Contributor
|
What I meant is a version with no directives at all -- where WP just determines based on the block attribute description that if this is a |
Member
It might be possible with |
Contributor
|
Sorry, this was accidentally closed when referencing from a different repo. 😨 |
Member
|
Actually, it was implemented by @ockham with: |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
What?
Supersedes #61255 with a slight different approach.
Instead of using the HTML to determine the markup of the image content to land the
figcaptionin the correct place. It removes those conditionals by always adding the caption, and deleting it later if there is no caption.Pros: simplicity, scalability with future role
content, bits or data-wp-bind.Cons: Markup added by default on the
save.jsfile. Which means a deprecation and updating tons of tests.How to test
As I copied some part of @SantosGuillamot 's original PR. I will copy also the testing process :-)
Go to a page and add a row with three images: One with a figcaption that will be replaced, another one without figcaption, and other with a caption that will be removed. You can use this snippet:
Backport to Core missing
I intentionally left this part until we discuss the final approach for this PR. Once the PR is approved on the PHP part, I'll create and link the Core backport.