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

[BUG] Footer causes a layout shift #760

Closed
pwlmaciejewski opened this issue Oct 4, 2022 · 3 comments
Closed

[BUG] Footer causes a layout shift #760

pwlmaciejewski opened this issue Oct 4, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@pwlmaciejewski
Copy link

Describe the bug
Virtuoso with a defined Footer component causes a CLS.

Reproduction
Code: https://codesandbox.io/s/react-virtuoso-cls-issue-with-footer-577cue?file=/App.js
Preview: https://577cue.csb.app/

To Reproduce
Steps to reproduce the behavior:

  1. Install the Lighthouse browser extension
  2. Go to the bug reproduction preview
  3. In the Developer Tools (F12) go to the Lighthouse tab
  4. Set the analysis mode to "Navigation" and select Category "Performance" and click "Analyze Page Load" button
  5. 💥 The report shows that the Cumulative Layout Shift metric has a poor grade
    Zrzut ekranu 2022-10-4 o 19 27 27

Expected behavior
The CLS should be 0.

Additional context

  1. It might be related to [BUG] Cumulative Layout Shift issue in React 18 #714, but we can reproduce it with React 16.
  2. If you go to the code sandbox and remove the Footer component, CLS drops to 0.
    Zrzut ekranu 2022-10-4 o 19 30 50
  3. If you go to the code sandbox and remove the initial Virtuoso render delay, CLS also drops to 0.
@pwlmaciejewski pwlmaciejewski added the bug Something isn't working label Oct 4, 2022
@petyosi
Copy link
Owner

petyosi commented Oct 4, 2022

First, the sandbox uses margins. This is not supported, but I doubt that the problem is due to that.

Second, I suspect that the cumulative layout shift report happens due to the footer being moved down by the dynamically calculated list. Since the contents are constrained from the scroller, this is unlikely to affect the rest of the page.

While this is technically correct, I don't think that in this case, this affects the user experience - but correct me if this is not so.

Apart from that, I don't think this is "solvable." Virtuoso does two intermediate render cycles, where the viewport size and a single, "probe item" is used to obtain measures before the list is filled in. This is likely causing the complaint of lighthouse. Of course, you may explore this further - perhaps my understanding is incorrect.

I am closing this issue for now - but feel free to provide further information if you understand the subject better. I am open to PRs.

@petyosi petyosi closed this as completed Oct 4, 2022
@pwlmaciejewski
Copy link
Author

First, the sandbox uses margins. This is not supported, but I doubt that the problem is due to that.

Whoops, sorry! I updated the code, and indeed it's not related; the CLS issue is still there.

Second, I suspect that the cumulative layout shift report happens due to the footer being moved down by the dynamically calculated list. Since the contents are constrained from the scroller, this is unlikely to affect the rest of the page.

That's my guess also. That said, I can't fully confirm it just yet because my measurements show a suspiciously small shift of the footer. It would need to be investigated further.

While this is technically correct, I don't think that in this case, this affects the user experience - but correct me if this is not so.

Apart from that, I don't think this is "solvable." Virtuoso does two intermediate render cycles, where the viewport size and a single, "probe item" is used to obtain measures before the list is filled in. This is likely causing the complaint of Lighthouse. Of course, you may explore this further - perhaps my understanding is incorrect.

I can't confidently say that this CLS uptick reported by Lighthouse is degrading UX, but there's more to it, so let me elaborate.

I mentioned that my measurements show a suspiciously small layout shift that I can't observe with my bare eye. That said, I can see an unpleasant jump in the content also. The more computationally expensive the first render of the list is, the bigger and easier to notice the jump is. Interestingly enough, I would say it's bigger than the one reported by Lighthouse, so I'm careful with making any final judgments yet. It may be that those are two different, unrelated issues.

My take on this bug is the following:

  • Increased CLS is a problem in itself for applications that measure the metric actively. In recent years Web Vitals became the de facto standard for (broadly speaking) measuring application performance, so I suppose that the perfect situation is when Virtuoso doesn't affect those metrics in any negative way.
  • The bigger problem is that in our case when Footer is defined, I can observe the degraded UX for long lists.

It may be that the 2nd issue is related more to the render cycles and not the CLS issue.

I am closing this issue for now - but feel free to provide further information if you understand the subject better. I am open to PRs.

I'm happy to contribute. I was wondering what would be the best way to move forward with it. One of the possible fixes I was considering was to show the footer only when the items are rendered on the screen. From what you wrote, I assume there are 2 render cycles: one "off-screen" to measure the stuff and the second to actually render the items?

@petyosi
Copy link
Owner

petyosi commented Oct 5, 2022

Yes, you can certainly try what you describe. FWIW, you can even short-circuit that check to render the footer only if more than 1 item is present in the list. This will skip the initial measuring cycles (2 of them - one empty and one with a single probe item). If this makes Lighthouse happy (it crashes my machine btw), then we can figure out a robust solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants