PLANET-6742 Add right sizes attributes for some core blocks #837
Conversation
Inwerpsel
force-pushed
the
core-images-sizes
branch
2 times, most recently
from
0e4df68 to
764c65b
Compare
/unhold 9d24d40b-6e49-4e42-bd02-473c8d3f4756
Inwerpsel
changed the title
Test instance is ready 🚀🌑 oberon | admin | blocks report | CircleCI | composer-local.json ⌚ 2022.04.26 14:57:49 |
Inwerpsel
changed the title
| $sizes_attr = 'sizes="' . implode( ', ', $sizes ) . ', 100vw"'; | ||
|
|
||
| // Assume all images are full width in a container. | ||
| $block_content = preg_replace( '/sizes=".*"/', $sizes_attr, $block_content ); |
There was a problem hiding this comment.
This is safe to do as long as the Query Loop block has no other images than the post featured image. This is the case for now, and the result shouldn't be that bad if other image are affected.
Perhaps we can make assumptions about the sizes attributes we're matching to get more certainty. e.g. match only those that default to 100vw.
/unhold 37862df1-174e-4efb-8f5c-25ea099a038c
/unhold 6eb1fc4e-ed42-4681-a93c-d50171b2fa45
Inwerpsel
force-pushed
the
core-images-sizes
branch
from
f9d7537 to
5adb089
Compare
/unhold 3cf6119d-1d60-45ff-b736-04a3fbe7560c
/unhold 669da4da-f46d-4c45-96f5-a64e5dc932ed
/unhold 8f2188b2-af23-4d72-bdcd-6eccbb9f46ea
Inwerpsel
force-pushed
the
core-images-sizes
branch
from
13d5c35 to
2c5ebd3
Compare
/unhold 5828594f-d6e3-412f-9f13-12eeafa8888c
|
Interestingly desktop benefits more from this. On mobile the images affected here are shown almost full screen width, so the On desktop however.... after |
* Makes the assumption that the only images inside the block are post featured images spanning the full width of the container. * A similar fix can be done for media and text, and possibly columns. * It gets hard when multiple blocks that force a width are nested in each other. In that case it might be possible to access block context, at least for a few cases.
* It's a different case from the Query Loop block, which already has sizes and srcset attribute. Media and Text relies and a filter running after it to add the right things. However if the filter already finds both attributes, it leaves them in place. * Support media width attribute, as it's always a percentage of the viewport width.
* Didn't properly calculate paddings.
Inwerpsel
force-pushed
the
core-images-sizes
branch
from
2c5ebd3 to
aff3d98
Compare
/unhold 41ddc8e2-f3fb-496b-9533-2d4c0d5edad7
|
The image linting now completely passes on each variant of Media and Text, and Query Loop with any amount of items per row. You can validate by running |
Inwerpsel
force-pushed
the
core-images-sizes
branch
2 times, most recently
from
e8717ea to
7b7f9af
Compare
/unhold c3e0034b-1ca3-4919-af35-82c8aec8c880
| 'width' => '540px', | ||
| ], | ||
| [ | ||
| 'screen' => 577, |
There was a problem hiding this comment.
Correct me if I'm wrong, but if this value refers to min-width, because of this, which is the value for lower screens?
There was a problem hiding this comment.
For lower screens it goes to full width of the screen - padding (or no padding for full width blocks). I didn't add it here yet because the default is a "special case" where the width is not specified.
* The padding is there if the whole Query Loop block isn't set as full width. If it is set as full width, the image size calculation is very different.
* Ugh.
* Already handled by default case.
* Now it gives right sizes for both. * Improve breakpoint for change from full width to not full width.
Inwerpsel
force-pushed
the
core-images-sizes
branch
from
7b7f9af to
993c448
Compare
/unhold 7619cd9d-8468-4009-8fbf-befc667fe8db
Ref: PLANET-6742
We found while checking the Brasil site that image handling in core is still not great, not just on the Query Loop block. With some small fixes we can reduce the amount of unnecessary data being sent by a lot.
These 2 blocks are relatively easy to provide the right
sizesattribute, which helps browsers avoid downloading oversized images. Image handling is still an open issue in core and doesn't seem to have much progress lately.Investigating:
sizes.How I tested these changes
https://github.com/ausi/respimagelint can scan a page's images and report on missing/incorrect data in srcset and sizes attributes.
On the master branch, running this bookmarklet clearly shows errors of images being multiple times smaller than their sizes attributes indicates.
On this branch, it should not give any more errors for images inside Query Loop (grid) and Media and Text. (page is identical)
Media and Text
This one is easier and likely has more benefits. However if the block is inside a container that forces a width, it's still way off. At least page header images.
Query Loop