-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Shadow support enable skip serialization for dynamic blocks #59887
Shadow support enable skip serialization for dynamic blocks #59887
Conversation
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. |
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @colinduwe! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
@aaronrobertshaw Here's a PR for improving the /supports/shadow as discussed at #59616 |
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.
Thanks for splitting this out @colinduwe 👍
This is testing well for me. I used the dynamic Cover and Post Featured Image blocks for the majority of testing.
✅ Code change looks good
✅ Dynamic blocks that don't skip serialization still get styles on their wrapper
✅ Dynamic blocks that do skip serialization no longer get styles on the wrapper
✅ Static blocks continue to function correctly
LGTM 🚢
I'll merge this PR for you shortly.
The next step is to backport this PHP change to WordPress Core to be included in the WP 6.5 release. Is that something that you'd like to do?
There's some documentation available on how to do that and you can ping me in case you need a little help.
Thanks @aaronrobertshaw! Here's an attempt at the backport https://core.trac.wordpress.org/ticket/60784#ticket |
This seems like an enhancement (add support for skipping serialization) to a new block support. So not really a regression or bug fix, so I'm removing the backport label (this needs to land in 6.6) |
Happy to go with your call on this one 👍 I still see this as a bug given the support for skipping serialization was added in #58306 which lands in GB 17.7 & WP 6.5. If a consumer tries to use that for a dynamic block, as the block.json schema indicates is available, it won't work without this fix. That said, I appreciate it can be argued that "support for dynamic blocks" is the enhancement. |
What?
When a dynamic block defines __experimentalSkipSerialization in its block.supports.shadow the styles continue to be printed to the block wrapper element. This PR corrects that behavior so that shadow behaves like border, color, and others.
Why?
This provides the expected behavior for dynamic blocks that need to opt out of having the shadow styles printed to the block wrapper.
How?
Added a check at the start of the render function to return an empty array of the block has set __experimentalSkipSerialization
Testing Instructions
There is a risk of a deprecation/regression so also check the core blocks that already support shadow
Core/image uses __experimentalSkipSerialization so verify that there is no change in the editor or front end with this PR.
These are not dynamic blocks so should be unaffected by this change.