Skip to content
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

Content Layout Shift (CLS) for non-default breakpoints with differing ratios and with lazy-loading #216

Closed
univers-andrew opened this issue Apr 20, 2023 · 7 comments

Comments

@univers-andrew
Copy link

univers-andrew commented Apr 20, 2023

Bug description

First of all, I'd like to preface my issue by saying how much of a massive fan I am of this addon — it's a joy to use when templating, and makes using Statamic so much more joyful! It's been a while since I've had the chance to use it, and I particurly appreciate some of the more recent developments.

In regards to the CLS issues I've observed, there are two distinct issues I've noticed on a site I'm currently building. Hopefully by bringing them to your attention they can be resolved within the addon, or you can point me in the direction of a suitable workaround if it's a configuration issue...

The first issue I noticed, is relevant when using breakpoint based ratios. In the following example I've used the following snippet.

{{ responsive:image alt="{alt}" ratio="1.3" md:ratio="0.75" loading="lazy" }}

Untitled.mov

Unfortunately, this seems to be setting the '1.3' ratio dimensions for the width and height attributes on the image on larger screens, which causes quite a large layout shift when placeholder="false is enabled. Inspecting the img element in the browser shows width="850" height="654" in this instance.

This issue only occurs if placeholders are disabled. If placeholders are used though, I am also obesrving slight CLS which I have now documented in a new issue for maintainability, which can be found here.

With thanks,
Andrew

How to reproduce

{{ responsive:image alt="{alt}" ratio="1.3" md:ratio="0.75" placeholder="false" }}

Environment

Statamic 3.4.7 Pro
Laravel 8.83.27
PHP 8.1.13
Stache Watcher Enabled
Static Caching Disabled
dryven/faviconator 1.0.2
spatie/statamic-responsive-images 3.1.3
statamic/ssg 1.3.0

Installation

Fresh statamic/statamic site via CLI

Antlers Parser

runtime (new)

@ncla
Copy link
Collaborator

ncla commented Apr 20, 2023

Hey,

Breakpoint ratios

Yeah, this indeed is an issue. The non-default (in your case md) breakpoints do not have width and height set. Only the <img> tag has it. With this HTML templating, width and height parameters on <img> are used to consume the space, however for larger breakpoints this does not exist. Hence once the image has loaded in, browser uses the loaded in image dimensions to recalculate the consumed ratio.

According to the specification, <source> tags do accept width and height parameters, which could be used to solve this issue. This is definitely good idea and could be added to the package, and I have thought about this before.

Two ways to solve this right now for you:

  1. If you are using something like Tailwind, create a wrapper aspect ratio element that consumes this space as you expect, per breakpoint. Then set the <img> CSS to object-fit: cover.
  2. Publish and edit responsiveImages.blade.php template. This might be difficult as you may have to wrangle around a bit, but in that template you have access to $breakpoint variable, through which you can access breakpoint parameters that you have passed through the tag (the ratio in this instance). From there you could technically calculate the width and height for the <source> tags.

Placeholder issues

I am seeing two different issues and would love you to create a separate issue for this (separate from breakpoint ratios one), one issue per one bug so it is easier to manage and track for maintainers.

Can you clarify again what is wrong?

I am seeing these issues myself:

  1. Placeholder is not working at all in the first video
  2. The placeholder is working, however there is a slight CLS

Thanks.

@univers-andrew
Copy link
Author

Hey, thank you for your detailed reply, very much appreciate the insight!

Happy to create a separate issue on the placeholder issues, for maintainability's sake. To clarifiy;

Placeholder is not working at all in the first video
The placeholder is working, however there is a slight CLS

  1. In the first video I've disabled placeholders using placeholder="false" to provide a baseline example, to help confirm the issue is created when the placeholder is enabled with placeholder="true"

  2. In the second video, placeholder is enabled with placeholder="true", and you can see the slight CLS occur — on a long page with a lot of images this is can be a significant user experience issue with #anchor links, as the CLS effect becomes magnified.

Thanks again,
Andrew

@univers-andrew univers-andrew changed the title CLS issues CLS issues (Breakpoint ratios) Apr 20, 2023
@ncla ncla changed the title CLS issues (Breakpoint ratios) Content Layout Shift (CLS) for non-default breakpoints with differing ratios Apr 21, 2023
@ncla ncla changed the title Content Layout Shift (CLS) for non-default breakpoints with differing ratios Content Layout Shift (CLS) for non-default breakpoints with differing ratios and with lazy-loading Apr 22, 2023
@ncla
Copy link
Collaborator

ncla commented Apr 22, 2023

I can can reproduce both issues.

I have a fix for this issue. As described before, setting width and height on <source> solves this issue.

For 2nd issue, this is happening because placeholder image is inlined and much smaller (32px width with height calculated from ratio), and for some reason the consumed width and height is used from this image, despite explicitly defining width/height on img/source tag. So once that small image is upscaled in resolution (while respecting it's ratio), the height ends up being off by few pixels, when comparing to the actual loaded in image dimensions.

The fix for both of these will be a breaking one and will require a major release. So for now, my previous recommendation for using parent container with aspect ratios is still good, and it's what the fix would be anyway.

@spatie-bot
Copy link

Dear contributor,

because this issue seems to be inactive for quite some time now, I've automatically closed it. If you feel this issue deserves some attention from my human colleagues feel free to reopen it.

@ncla ncla reopened this Aug 23, 2023
@spatie-bot
Copy link

Dear contributor,

because this issue seems to be inactive for quite some time now, I've automatically closed it. If you feel this issue deserves some attention from my human colleagues feel free to reopen it.

@ncla ncla reopened this Jan 1, 2024
@spatie-bot
Copy link

Dear contributor,

because this issue seems to be inactive for quite some time now, I've automatically closed it. If you feel this issue deserves some attention from my human colleagues feel free to reopen it.

@ncla ncla reopened this May 4, 2024
@spatie-bot
Copy link

Dear contributor,

because this issue seems to be inactive for quite some time now, I've automatically closed it. If you feel this issue deserves some attention from my human colleagues feel free to reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants