-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Print CSS rule to fix potential visual issues with auto-sizes #7813
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 Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
* @see https://html.spec.whatwg.org/multipage/rendering.html#img-contain-size | ||
* @see https://core.trac.wordpress.org/ticket/62413 | ||
*/ | ||
function wp_print_auto_sizes_contain_css_fix() { |
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 right. What would you think about bailing early if auto-sizes has been disabled via a filter similar to what is being proposed in #7812?
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.
Makes sense to me. Let's either add this here or in #7812, depending on which of the two gets committed first. We can coordinate that once we're confident about the changes.
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.
Added in 3b2a48e
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
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.
LGTM, optional suggestion on adding return type.
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
Committed in https://core.trac.wordpress.org/changeset/59415 |
This PR implements what's described in https://core.trac.wordpress.org/ticket/62413#comment:36. It also introduces a filter to opt out of
sizes=auto
.With this fix:
sizes=auto
(by default all lazy-loaded images in WordPress) and are affected by a CSS rule that sets theirwidth
to eitherauto
orfit-content
will no longer appear in a smaller size than intended.Trac ticket: https://core.trac.wordpress.org/ticket/62413
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.