-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Use get_the_post_thumbnail
function in the cover block.
#40853
Conversation
get_the_post_thumbnail
function is used for cover block. get_the_post_thumbnail
function in the cover block.
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.
@spacedmonkey Great catch! This does not only come with improvements for consistency and performance, but also accessibility. 🎉
Please check the CI failures. Other than that I'm happy to approve it.
|
||
$image = sprintf( | ||
$image_template, | ||
esc_attr( get_the_post_thumbnail_caption() ), |
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.
This was in fact incorrect and could result in accessibility problems, since it was using the caption as the alt
attribute, when in fact WordPress images have their own dedicated field to define the alternative text.
So fixing that is another benefit of 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.
Great catch, I think it resulted like this as I didn't consider that featured images are their own kind of entity in WP, instead treated it as another simple image added to the cover block.
); | ||
|
||
$image = get_the_post_thumbnail( null, 'post-thumbnail', $attr ); |
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.
This makes total sense, we shouldn't manually create img
tags in WordPress core when dealing with attachments.
Excellent, thanks for working on this @spacedmonkey! |
if ( in_the_loop() ) { | ||
update_post_thumbnail_cache(); | ||
} |
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.
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.
This looks good to me.
The change adds several attributes to the image tag
loading
srcset
sizes
width
height
class
changes fromwp-block-cover__image-background
towp-block-cover__image-background wp-post-image
I think all these are good things provided the width and height don't affect object fit.
No changes when the image is used as a background URL.
I fixed a couple of PHP Coding-Standards issues, once test pass this can be merged 👍 |
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.
I just want to leave the 3rd approval here :D
I cherry-picked this PR for WordPress 6.0 RC2 with 93befd1. |
* Improve logic. * Improve logic. * Coding Standards: Using single-quotes if double-quotes are not required Co-authored-by: Ari Stathopoulos <[email protected]>
What?
Follow on from #39658.
Why?
Improved logic, to that if cover is not an image, then do not both to load feature image.
Ensure that
get_the_post_thumbnail
function is called, so that filters are run correctly.Ensure that
update_post_thumbnail_cache
is called, so that feature image are loaded into memory.How?
Testing Instructions
Screenshots or screencast