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

High CLS without actual content shifting #10873

Closed
marto55555 opened this issue May 29, 2020 · 11 comments
Closed

High CLS without actual content shifting #10873

marto55555 opened this issue May 29, 2020 · 11 comments

Comments

@marto55555
Copy link

marto55555 commented May 29, 2020

Provide the steps to reproduce

  1. Run LH on https://bonusi.soft-press.com/ https://splighthouse.marti.bg (or on https://bonusi.soft-press.com/all https://splighthouse.marti.bg/all)
    • DevTools: with Simulated throttling turned off
    • web.dev: as it is
  2. Observe the CLS metric and "Avoid large layout shifts."

What is the current behavior?

The CLS jumps from 0 to seemingly random higher value, despite no visible layout shifts in the preview pane. The value is higher on web.dev than on DevTools. "Avoid large layout shifts" returns "Error!", or gives different elements on every run. See screen recordings.zip

What is the expected behavior?

The CLS should stay at 0, as it does with Simulated throttling on.

Environment Information

  • Affected Channels: DevTools, web.dev "Test my site"
  • Lighthouse version: 6
  • Chrome version: 85.0.4159.0 (Official Build) canary (64-bit) (cohort: Clang-64)
  • Node.js version:
  • Operating System: Windows 10 64bit

Related issues

@patrickhulce
Copy link
Collaborator

patrickhulce commented May 29, 2020

Thanks very much for filing this @marto55555! Lots going on here I think we might want to break apart this into a few separate issues.

  1. High CLS - I'm inclined to agree with you I have no idea where the layout shifts are coming from. There is a lot of animated movement as elements slide in but such animations shouldn't be resulting in a high CLS or affected by throttling settings.
  2. The Error! case we know exactly why it's happening and should fix. cc @Beytoven we'll probably want to catch those errors and surface a "Element was removed from the DOM" or something if we couldn't find it. Maybe file a separate issue? At least this provides a clue that whatever elements are being flagged for layout shifts have been removed from the DOM by the time the page is done loading :)
  3. The Largest Contentful Paint element Error! is problematic. If the LCP element was removed AFAIK that should mean the LCP was invalidated yet we're still providing a value for it. We might need to look into that a little deeper. This is just a result of bullet 2, LCP was found just fine.

@marto55555
Copy link
Author

marto55555 commented Jun 1, 2020

I looked into the high CLS through DevTools’ layout shift regions highlighting. I thought this might be useful for you too, so here's what I found, from most to least significant:

  1. When the Polymer library loads, and right before the loading splash screen fades out, the largest alleged layout shift occurs (0.09-0.1). However, nothing visible actually moves. I suppose situations like these should not be counted since there’s no user impact, and the splash screen is covering the Polymer elements underneath.
    Note that if I delay the fade-out by 1ms using setTimeout, the shift is no longer reported. If the dark theme is turned on, such delay is already applied. Because of that, the light theme (with no fade-out delay) produces a larger CLS.
  2. I have confirmed the shifts related to the content are not because of the slide-in animations (they use transform: translateY). Instead it’s the bonusContainer div. Before the first cards are loaded from the server, the div has dimensions of 0x0. However, expanding its width does not move anything else, so why is it counted in?
  3. The loading spinner below the cards is also contributing. Every time new cards are fetched, the <polypwa-spinner> moves down, accumulating shifts. Since that’s a genuine shift, what should the procedure be for spinners which are always below infinitely scrolling content?
  4. Finally, the Roboto dependency from Google Fonts causes FOIT, which is understandably reported in the CLS. But I was curious: if users already have the font on their systems and it gets swapped with itself, and paragraphs of text don’t become longer/shorter, shouldn’t this be excluded from the CLS?

For now, I won’t push any updates to the web app, to make it easier for you to fully investigate :)

@patrickhulce
Copy link
Collaborator

Thanks for the additional information @marto55555, great investigation!

  1. Ah, that's a really interesting edge case for CLS to consider. Nice find!
  2. That's odd. I'm not sure why just yet, but thanks for pinpointing the issue.
  3. Is this happening while the spinner is below the viewport too? If so that's a bug. Only in-viewport shifts should be counted in CLS in Lighthouse. While it's in viewport, the metric is basically WAI. Any "fix" here feels like it's not particularly improving the user experience and is just to satisfy the metric. It's not intended to be important content or have any click handler or anything, and the spinner area should be pretty small, so I'd expect to just live with the 0.0X CLS score once we can get to the bottom of the others.
  4. If elements don't see a shift in their position, I wouldn't expect FOIT to impact anything. Might need to look into what's going on here.

Thanks very much for your offer to leave the site as-is. I don't want to block your dev flow though :) If it's possible to throw it onto a free static hosting site such as http://surge.sh/ where it can sit without bothering anyone that would be amazing, but for now I've taken a snapshot of the Lighthouse artifacts so at least we can observe the trace events at a later date.

High CLS latest-run.zip

@marto55555
Copy link
Author

  1. Is this happening while the spinner is below the viewport too?

No, it's only while in the viewport. As you've said, the small size of the spinner should have a negligible effect on CLS. However, LH currently "tracks" the spinner area as if it was much larger (size of a sliding card + slightly more). This is particularly noticeable for viewport width>900px -- when 2 cards are on the same row, with the network set to Fast/Slow 3G.

Thanks for being considerate of dev flow! I've uploaded this version to https://splighthouse.marti.bg for your future investigations :)

@patrickhulce
Copy link
Collaborator

However, LH currently "tracks" the spinner area as if it was much larger (size of a sliding card + slightly more)

Ah, do you have the spinner wrapped in a div that's the size of a card? If the spinner were implemented with margin I wouldn't expect the area tracked to be very large. This to me is one of those stupid "fixes" that doesn't change the UX in anyway but makes the metric happy.

@marto55555
Copy link
Author

marto55555 commented Jun 5, 2020

Ah, do you have the spinner wrapped in a div that's the size of a card?

Nope, it's directly placed after the bonusContainer div with the cards, and highlighting it in devtools reports a size of 28x34 (which is small enough). It's centered with position: absolute; left: 50%; transform: translateX(-50%);

Apparently, I was wrong about the spinner being the problem. Instead, it's 2 things around it:

  1. There's a <br> right before it, creating a whole-width layout shift line as new cards appear. It's fixable (by applying a margin-top to the spinner)
  2. <bonus-grid>, the container of bonusContainer and the spinner, has an ::after { content: ''; padding: 60px; }. This was the larger CLS contributor (square on the left + full-width line), and I need it to have some space below everything (for a floating action button, as it shouldn't overlap the last card when the page is scrolled to the bottom). Why is this counted in CLS, since its content is empty quotes?

after-padding

@patrickhulce
Copy link
Collaborator

Why is this counted in CLS, since its content is empty quotes?

Because CLS doesn't account for empty content. Sorry, but I don't know what else to tell ya I didn't design CLS :)

I think it would make sense to raise this in Chromium though in the layout stability API.

https://melodic-class.glitch.me/cls-empty.html is a particularly damning example of CLS being completely unrelated to perceived user layout shifts.

@patrickhulce
Copy link
Collaborator

Filed WICG/layout-instability#45 👍

@widmanski
Copy link

widmanski commented Oct 27, 2020

admin edit: moved to dedicated issue: #11601

@patrickhulce
Copy link
Collaborator

Thanks @widmanski! That appears to be a separate issue that's specific to web.dev/PSI, would you mind moving your comment to a new issue we could track separately? :)

@brendankenny
Copy link
Member

Looks like this was resolved in WICG/layout-instability#96

Closing!

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

No branches or pull requests

7 participants