-
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
Fix overlay issue when empty featured image is used in Cover Block #59855
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. |
@draganescu need you to review this PR. |
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 putting this PR together @sunil25393 👍
It's testing nicely for me.
✅ Could replicate original issue
✅ This PR displays the overlay in the editor
Before | After |
---|---|
It looks like the overlay was deliberately omitted previously in this scenario in favour of the placeholder for the featured image.
Perhaps it is worth getting some quick design feedback as if we are looking for parity here between the editor and the frontend, having both the overlay and the background color for the placeholder results in a different color in the editor when the overlay is semi-transparent.
cc @jasmussen given your past work around image placeholders do you have any thoughts?
I don't think that small discrepancy is a blocker. This PR is a step in the right direction.
I've left one small suggestion as an inline comment but this PR looks almost ready to me.
Thank you for the PR 🙏 This tentatively looks good to me. Before: After: I believe some of the challenges were around border and radius, that appears to still work as intended: — even if there's still a bug around how Chromium renders backdrop-filter/blur causing the edges to blur inwards. That's separate. The following is entirely separate, but in testing this I'm noticing that the color tint from duotone, which used to affect the Featured Image block is no longer working either there or on the site logo. I wonder when that regressed. It still works on the Image block, but that is now smaller than it used to: I'll investigate. But tentative 👍 👍 on this one, and thanks again. |
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 taking a look at this one @jasmussen 👍
I've given it another smoke test after the latest tweaks and it still tests well for me.
I'm happy to approve this one. The noted issues or regressions (unrelated) can be iterated on in follow-ups.
I don't remember having a specific requirement to not allow overlay over featured image, it's just how it was implemented at the time. I think the addition is good and makes the experience of using the cover block more consistent. |
…ordPress#59855) * Fix overlay issue on placeholder * Addressed feedback
What?
Fixes #57887
Why?
Overlay doesn't show when empty featured image is used in cover block.
Testing Instructions
Screenshots or screencast