-
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
Post Featured Image block: Enable gradient overlay #43838
Post Featured Image block: Enable gradient overlay #43838
Conversation
'overlay_element_markup' to 'overlay_markup'
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @RahiDroid! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
@mikachan as this is a Theme issue for TT3 would you be able to help out with a review? I'm also conscious that the Cover block now supports the featured image so it might be possible to achieve the same effect that way. |
Just a thing to note, the Cover block requires a fixed height and can not have a dynamic height based on the featured image's original height. Using the Cover block in place of the Post Featured Image block could render it cropped. And by design, we can't assign dynamic height to the Cover block unless we do it conditionally when it has no inner blocks. |
Cool! Here's what I'm seeing: Kapture.2022-09-08.at.11.21.57.mp4This is working really well IMO. I was worried that the flow might be awkward without the ability to define the dimratio on the overlay in theme.json, but this PR gets around that nicely by setting it to 0 to start. Nice work :) |
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.
Thank you for working on this! I've tested this out over various scenarios and it's working really well for me across the post editor, site editor, and front end.
I'm also conscious that the Cover block now supports the featured image so it might be possible to achieve the same effect that way.
We're currently including 3 alternative template options in TT3: with no image, with a featured image, and with a cover block. This PR could enable us to reduce that back down to one template. There's some more info over in this discussion.
When combined with the Duotone, it behaves similarly to the Cover block.
This is the only thing that gives me pause, in that the cover block and the post featured image block are now very similar. I don't think it's a blocker at all, but perhaps a discussion for the future.
The Cover block doesn't show the overlay on the placeholder inside the site editor. Either we add the overlay on the placeholder for the cover block, too, or we remove it from Post featured block to ensure consistent UX.
I'd personally like to see the cover block show the overlay in the editor, as that seems like an improved UX, but I think this could be a follow-up PR so as not to block this one.
It looks like there's some code here that's also in the cover block. It might be worth considering if we can make this into a general solution that all blocks could use, rather than a specific per-block implementation. |
I agree with you @scruffian, and I was wondering what that place would be though. And if we could do that extraction in a separate PR where we could also cover some refactoring to the Cover block, both in terms of code and feature (Placeholder overlay color). If we could ship this PR for the 14.1 RC1 release, it would be great, as this one could help us with some style variations on the TT3 theme? |
Hi @mikachan 👋 Hope you had a good weekend, any hints on what the next steps would be for this one? |
It would be great if we could consolidate the functionality that overlaps with the cover block, but I agree this could happen in a follow-up PR along with any refactoring. We should also revisit the accessibility issue you mentioned here. We should create an issue for this refactoring work so it's easy to track. Otherwise, I think this is good to bring in now! 🎉 |
Hi @mikachan, thank you for guiding me through this. I have created an issue for the accessibility issue #44115. I will create an issue for the Cover block's refactoring once this PR gets merged as before we work on that, we could get some inputs on this accessibility issue as well. Please let me know what would be the next step on this. Do we need any more approvals on this? Do you think we can land this in 14.1.0 RC? 🙈 |
Thanks for creating that issue, and your plan for the refactoring PR sounds great! Please feel free to tag me, and I can help find other appropriate folks for review/input as well. @c4rl0sbr4v0 has kindly merged this PR and it's scheduled to be included in Gutenberg 14.1, so that's all we need to do here 🎉 |
What?
Overlay
settings (color & opacity) to the Post featured block.Why?
How?
Testing scenarios covered
Testing Instructions
Post Featured Block
.Overlay
controls in the inspector controls of the block.Screenshots or screencast
Important notes