-
-
Notifications
You must be signed in to change notification settings - Fork 314
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
Comments
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. |
Whoops, sorry! I updated the code, and indeed it's not related; the CLS issue is still there.
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.
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:
It may be that the 2nd issue is related more to the render cycles and not the CLS issue.
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? |
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. |
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:
Expected behavior
The CLS should be 0.
Additional context
The text was updated successfully, but these errors were encountered: