-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Fix : Image caption blur in Gallery block #74063
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
base: trunk
Are you sure you want to change the base?
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. |
|
Thank you, @Vrishabhsk, for working on the fix! It fixes the issue in my testing, but I'd like a formal review from someone more familiar with the file. Hi @jasmussen, would you be able to review this? |
|
Thank you for the issue, thank you for the PR, thank you for the ping! In a quick test, this does seem to fix the issue. I also personally like the idea of the blur being attached to the However, back when I worked on this, I recall specifically moving it from one element to the other based on feedback. The conversation is here, and I recall that it related to cases where you have extremely long captions, so long that they actually need to scroll, in which case we use minimal scrollbars. But that is to say, I think this one is important to test with extreme captions, and with Windows based scrollbars, lest we risk regressing something. And if that happens to be the case, there are other things we can do, such as change the 40% max-height value to a clamp-based value so that it has a max-height in ems. Or just go straight up ems. The text-shadow ultimately ensures there's some contrast. For completeness, here's a before and after in my test, and both work reasonably for the extreme case. Edit: note that these are poor quality GIFs that don't capture the intricate shadows at play here. Before: After: Where things start to lose a little character, unfortunately, is the captions we're more likely to see, regular 1-line captions. Moving the blur to the figcaption minimizes the mask gradient, making the blur stop abrubtly:
Compare that with trunk for the same use case:
Yes, the above is a bit tall, that is the main problem to fix. But notice how the blur itself is masked, and the blur is lessened moving upwards, slowly disappearing rather than abruptly disappearing. Trunk also adds a black overlay color to darken the gradient, whereas this PR removes that and only has the blur. Importantly though, if we want to move the blur to the figcaption, I'd love first input from @t-hamano, as I recall he was instrumental in finding the rough edges of my previous implementation relating to scrollbars. However, I also have another solution. We keep the caption on the pseudo element, but replace
And a crazy multi-line caption:
Let me know what you think! |
SirLouen
left a comment
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.
And if that happens to be the case, there are other things we can do, such as change the 40% max-height value to a clamp-based value so that it has a max-height in ems
I'm not sure if this is the best solution. If there is a caption that goes over the full image, I don't think its terrible idea that the whole image blurs with the caption upfront.
The only alternative is to limit the caption to a % of the image, say 40%, as it was supposed to cover with the blur and adding the scrollable bar, but this will probably be significantly complex, and personally, I believe that it's an unexpected behavior for such a degree of complexity.
It is unclear to me why anyone would like to add so much caption to an image instead of just text over the image. The image should not look good with such an amount of caption because it's clearly a “doing it wrong” behavior. Expecting the same behavior with 1 or 2 lines or the entire image should be reasonable. Expecting a different behavior just because the caption is so extensive is unrealistic.
I'm not sure why you cannot reproduce in those images what it's being reported. Images with a very vertical aspect ratio get a massive blur. But now that I look closely in the skyline image it's true that it's too abrupt the cut. So maybe going for a |








What?
Closes
Why?
How?
backdrop-filter: blur(3px);directly to all figcaptions.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast