-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Block Bindings: Add support to image caption attribute in block bindings #70642
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
Size Change: +230 B (+0.01%) Total Size: 1.91 MB
ℹ️ View Unchanged
|
3b1b85a
to
013493f
Compare
Flaky tests detected in e8f6ded. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/16879288790
|
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. |
905144a
to
e8f6ded
Compare
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 ); |
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 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 🤔
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.
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:
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.
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 🤔
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. |
@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 |
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 |
It might be possible with |
What?
Supersedes #61255 with a slight different approach.
Instead of using the HTML to determine the markup of the image content to land the
figcaption
in 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.js
file. 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.