Accurate sizes: Pass parent alignment context to images #1701
Conversation
mukeshpanchal27
changed the title
mukeshpanchal27
added
[Type] Enhancement
…ors' into update/pass-alignment-by-context
joemcgill
added
[Plugin] Enhanced Responsive Images
…context POC - Pass group block alignment context to image block
mukeshpanchal27
added
the
no milestone
mukeshpanchal27
removed
the
no milestone
mukeshpanchal27
requested review from
felixarntz and
thelovekesh
as code owners
|
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. |
|
I think this still needs some iteration before it's ready for review, so I'm marking it as a draft currently. |
| * See https://github.com/WordPress/gutenberg/blob/938720602082dc50a1746bd2e33faa3d3a6096d4/packages/block-library/src/cover/style.scss#L82-L87. | ||
| */ | ||
| if ( 'core/cover' === $block_name ) { | ||
| $image_width = $image_width * 0.5; |
There was a problem hiding this comment.
Good idea! I think that instead we could pass this as the function as the $size or $resize_width value for auto_sizes_calculate_better_sizes() and avoid needing to add another argument to the function signature. Alternately, we could consider allowing arbitrary values to be passed to the $max_alignment value, which then overrides any alignment value, which may come in handy whenever we want to pass a maximum width value based on things like columns or grid values.
Also, small nitpick, but this calculation isn't the same as what is used for the cover block, instead it calculates 0.5 * $content-width, which is a CSS variable that is defined in this package. So it should always end up being '420px'.
There was a problem hiding this comment.
Went with the $size idea and took advantage of the fact that you can use an array for $size when passing to wp_get_attachment_image_src(). See, d3a60d0.
| array( 'left', 'sizes="(max-width: 540px) 100vw, 540px' ), | ||
| array( 'right', 'sizes="(max-width: 540px) 100vw, 540px' ), |
There was a problem hiding this comment.
These were correct prior, based on https://github.com/WordPress/gutenberg/blob/938720602082dc50a1746bd2e33faa3d3a6096d4/packages/base-styles/_variables.scss#L125.
|
@felixarntz @westonruter The PR is ready for review. |
| $sizes = sprintf( '(max-width: %1$s) 100vw, %1$s', $width ); | ||
| } | ||
| $alignment = auto_sizes_get_layout_width( 'default' ); | ||
| $layout_width = sprintf( '%1$spx', min( (int) $alignment, $image_width ) ); |
There was a problem hiding this comment.
The return value of auto_sizes_get_layout_width() seems like it could be a non-pixel value, like 100vw. Is that a problem?
There was a problem hiding this comment.
Yes. This is a concern that I've been thinking about and plan to address in a follow-up PR.
There was a problem hiding this comment.
Documented in #1511 (comment)
Co-authored-by: Weston Ruter <[email protected]>
Co-authored-by: Weston Ruter <[email protected]>
joemcgill
merged commit 59e86f8
into
feature/1511-incorporate-layout-constraints-from-ancestors
Summary
Related #1511.
Fixes #1725
This PR adds functionality to pass layout info from a parent block to an image block. For example, if an image is constrained by a group block, even if the image block has alignment set to "full", it will not stretch wider than the default content width unless the parent group block is also aligned to "full".
Relevant technical choices
core/imageandcore/coverblocks are filtered to add 'max_alignment' to their 'uses_context' valuesauto_sizes_filter_image_tagis used to modify the rendered block markup using block context.auto_sizes_calculate_better_sizes()is used to calculate bettersizesvalues based on the image characteristics and the 'max_alignment' constraint.