-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Pattern Overrides: Image block doesn't render on frontend when it's saved as a placeholder in a pattern #58425
Comments
EDIT: Nevermind, it seems just the processing should solve the issue. I must have been doing something wrong in my tests. I've been triaging this and, although the change in the bindings processing will help, I believe the issue is a bit different. And I believe it is easier to solve. If I am not mistaken, just changing this conditional to check if bindings exist should solve it: if ( ! $processor->next_tag( 'img' ) || ( empty( $attributes['metadata']['bindings']['url'] ) && null === $processor->get_attribute( 'src' ) ) ) {
return '';
} The problem we are facing is that, in the case of the image, in the database we are storing an empty image without any URL.
So it doesn't matter how we change the bindings processing because the I believe it is safe to assume that, if the Let me know what you think. I can work on a pull request. |
Have you tried testing it with WordPress/wordpress-develop#6059? The purpose of this refactoring is to run block bindings processing before |
Nevermind, I must have been doing something wrong, probably registering the custom fields. It seems that it will work just with the changes in the processing. Sorry for the noise 😄 |
I haven’t tested the PR yet, and there are still no unit tests in place so I can't guarantee it works with this case. In theory, it definitely should unless I missed some nuances. |
In my tests, the WordPress/wordpress-develop#6059 PR doesn't seem to address the current issue with the Image placeholder. Does it work for you, @SantosGuillamot ? |
I've been testing if with a fresh instance this morning and there are two different use cases that I believe are causing the differences in the testing:
EDIT: I believe I discovered the potential issue with patterns. It seems in I can confirm that, with the latest version of Gutenberg I made a video showing it: Testing.new.processing.mp4Pattern.potential.fix.mp4 |
I was able to confirm that WordPress/wordpress-develop#6059 resolves the issue on the Block Bindings level. I coded a new test case that asserts whether the URL gets correctly replaced during rendering when the placeholder gets saved for the Image block. I still think that we should set some sort of fallback URL immediately after enabling Pattern Overrides for the Image block to better represent the fact that the fallback image will show up on the frontend unless it gets modified by the user. Otherwise, there is no way to let the user crafting the pattern to style the image. |
That shouldn't be the case, it was reverted before ever being released. edit: Just tested, and you're right. That indicates that packages haven't been updated since the last stable release of gutenberg. |
Is it still an issue with all the latest changes in the Gutenberg plugin and WordPress core? The setting for the Image block when in the placeholder state can be very limited, but it should be functional. |
Thanks, seems to be resolved now 🎉 |
Description
See the title and the steps to reproduce.
This seems to happen because the image block saved in the pattern doesn't have complete markup (because no image is set). The image block also dynamically renders nothing in its php render callback when it detects there's no image.
The server html replacement that pattern overrides uses can't reproduce block markup, so this causes a problem.
Step-by-step reproduction instructions
Expected: the image set in step 5 should render
Actual: the image doesn't render.
Screenshots, screen recording, code snippet
No response
Environment info
No response
Please confirm that you have searched existing issues in the repo.
Yes
Please confirm that you have tested with all plugins deactivated except Gutenberg.
Yes
The text was updated successfully, but these errors were encountered: