Block Bindings: Add support to image caption attribute in block bindings #61255
Conversation
|
Size Change: +10 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
gziolo
added
[Feature] Block bindings
[Block] Image
|
Hi is this still planned for 6.6? Or is it blocked? |
|
if it is not planned can you please remove it from the board? |
|
Unfortunately, it didn't make it in time for 6.6. We focused on other aspects related to the block bindings API, and the approach taken in this draft pull request would need to be discussed in detail. Additionally, after postponing it, we might be able to rely more on the HTML API. I just removed it from the board. |
|
As Andrea points out, not supporting this results in bugs in Pattern Overrides, so it's fair to catalog this as a bugfix for 6.6 rather than an enhancement. |
priethor
added
[Type] Bug
|
Okay, I will work on it during the beta phase then and try to support the image attributes to solve those issues. This is what I have in mind so far:
Apart from that, we are working on the compatibility code here, but these changes would also be needed in the core. |
SantosGuillamot
force-pushed
the
add/support-all-image-attributes-in-block-bindings
branch
from
542b6f9 to
54f3eb1
Compare
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core GitHub repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/compat/wordpress-6.5/blocks.php ❔ lib/compat/wordpress-6.6/blocks.php ❔ packages/block-library/src/image/index.php |
|
Flaky tests detected in 94e4020. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9468598992
|
|
I've been exploring the different options to cover the wanted use cases, and I am not sure which one is better. I am not convinced about either of them:
Following a similar approach to this pull request, I tried to always return the link element wrapper and the figcaption and remove them in the render if they should be empty. You can see the code here.
Another approach could be keeping the save logic as it is and add logic to the render to handle both case:
I was testing it here, although it would also need the removal part. Any opinions? Apart from that, I started working on this branch to include these changes in core as well. Right now, the files we are modifying are just part of the compat folder for versions older than 6.5, where bindings in core didn't exist. |
I personally prefer the first option, purely because it keeps the block responsible for the majority of the markup of the caption. With the second option the block bindings code is taking lots of responsibility for the block's html. I feel it's better if the bindings code is only lightly updating the html as much as possible. BTW, I pushed a couple of commits, the first one updates another The second one allows for editing of captions in patterns. A similar change will probably also have to be made for the link, though I think it's good to focus on the caption first (and perhaps these changes can be split into two separate PRs?). |
Would this require to create a |
|
I got the caption working on the editor, but not in the frontend, I'll take a look at that:
|
That's a good question. It doesn't cause any noticeable issues when testing, but it looks like that's because the saved markup matches the existing latest deprecation for the image block, so it successfully upgrades. It does seem to work fine, but it might cause unexpected issues, so it would probably be best to add a new deprecation. |
SantosGuillamot
changed the title
|
I've been thinking more about this and made a few changes. These are some comments about them:
|
SantosGuillamot
force-pushed
the
add/support-all-image-attributes-in-block-bindings
branch
from
1b20b0c to
f16e3e4
Compare
SantosGuillamot
force-pushed
the
add/support-all-image-attributes-in-block-bindings
branch
from
f4b229a to
be4c4c2
Compare
|
What is the long term plan here? Are we going to keep making exceptions for some blocks and some attributes? Imo, a more long term solution should be explored, for example templates in the block API, so the server knows how to serialise the block. |
The plan is to use the HTML API once it is ready to create an abstraction that works for all block attributes. The main functionalities missing are:
Once that is in place, we should be able to create an abstraction that works for all use cases just by reading the type and selector of each block attribute. |
|
@SantosGuillamot how does that solve adding or removing attributes? |
Oh, sorry. I thought you meant exceptions in the logic used to replace the HTML based on the block attributes: link. I assume you are talking about the changes we are making to the image render (or other blocks), right? I totally agree that we should explore a long-term solution for that. To be honest, I don't have a clear idea how it could be solved, so any ideas are more than welcome. The initial plan was to fully explore potential paths before opening block bindings for more blocks.
Could you elaborate more on what you mean by templates in the block API? I'd love to understand that better. |
I'm talking about everything :)
Some sort of templating language that works both client and server side. Maybe something like what @youknowriad created a while ago. But we probably need logic in templates. Maybe something like Liquid? Or maybe Mustache is just enough for blocks. |
That would definitely help with this particular problem, and I'd love to see something like that in the future. If I am not mistaken, a templating language was also discussed at some point as a possibility while working on the Interactivity API. Maybe @luisherranz has some insights. EDIT: Thinking about it a bit more, it could have other use cases and it seems a big topic. Maybe it deserves a separate issue/discussion to start gathering feedback? |
It has indeed an issue from 5 years ago: |
|
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. |
Yes, we want to start experimenting with that 🙂 |
Indeed, that deserves a better place to discuss all the details, as using any templating solution might not be straightforward with the currently existing extensibility model. There is a lot of nuances even in how the block editor injects additional class names and styles to the serialized HTML with block supports. If we were to use a templating system, we would have to ensure that hooks offered on the client, have a matching implementation on the server so the reconstructed HTML from attributes and the template wouldn't miss any details. I also wanted to highlight the proposal from @dmsnell about HTML API templating: https://make.wordpress.org/core/2023/08/19/progress-report-html-api/#templating. Prototype: WordPress/wordpress-develop#5949. That's something that could be re-implemented in JavaScript. However, it's also a good reminder that conditions and loops are very important aspects here. Overall, it's a bigger topic that this PR only surfaces by trying to address the limitations on the current saving mechanism for static blocks. |
What?
Add support for the image caption attribute in block bindings.
Why?
There is an issue caused by this: #62287.
How?
These are the main changes made to the pull request:
Change the core bindings logic
The core pull request adds the logic to support caption in the bindings replacement logic. Right now, it uses an anonymous class simulating
set_inner_textuntil the HTML API provides a similar method. It is done this way to avoid using regex.Change the image render logic
This pull request changes the image render logic to cover two use cases:
This is done again creating an anonymous class with a couple of methods until this can be easily done with HTML API core code. It is done this way to avoid using regex.
Change the compat folder for versions previous to 6.5
Apart from that, this pull request adapts the code that runs when a WordPress version below 6.5 is used. They look like big changes, but most of them are just code reorganization. It covers:
gutenberg_is_valid_block_for_block_bindingsandgutenberg_is_valid_block_binding.render_block_datafilter to modify the attributes with the binding value. This is done in core from 6.5, but there wasn't an equivalent in Gutenberg compat, and it is needed for this use case.Testing Instructions
Keep in mind that in order to test it you need the code from this core pull request apart from this one.
Mainly, three scenarios need to be tested in three different environments:
Scenarios
Enviroments
6.5branch in core.6.4branch in core.How to test it
Best way to test it, after having a local environment with the relevant Gutenberg and core branches:
Issues before this pull request
Before.supporting.caption.mp4
As can be seen in the video:
Issues fixed after this pull request
After.caption.support.mp4
As can be seen in the video:
Image caption can be replaced, added, or removed from the editor
Caption.demos.mp4
As can be seen in the video, the following scenarios are possible after this PR: