-
-
Notifications
You must be signed in to change notification settings - Fork 299
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] Scroll flickering with expandable cards #418
Comments
I am not sure how much this example follows your actual source code, but to optimize this scenario, you do need to apply proper memoization on a few levels, like the parameters of the Virtuoso component as well as the rendering of your items. Take a look at this fork for more details. |
Perhaps I am missing something - can you be more specific about what do you all a scroll flickering? There is some scroll repositioning due to item resize, and in reverse mode, this is not fully avoidable. |
I couldn't find a specific case where scroll jump occurs, but what can be seen on the above gif is something like that:
Unfortunately this does not occur every time, but is frequent enough to be uncomfortable. It happens frequent enough so you could reproduce this behavior. |
Which browser and OS is this? I am doing similar steps and it works on my side. Chrome, MacOS. Screen.Recording.2021-07-26.at.19.19.00.mov |
Hey, I'm using Windows 10 with latest Chrome. It does not occur every time. My way to reproduce was expand 3 cards while scrolling, collapse bottom card, start scrolling up. Sometimes scroll jumped. |
I've seen such issues related to specific scrolling devices - mice with wheels where the inertia scrolling on windows does not get reported properly through the DOM events, there are a few closed issues related to these. Do you have a mac available? if so, try to repro on it, so that I can confirm that this is the same problem. |
I will ask someone to confirm on macOS device. Meanwhile I can confirm that this behavior is also reproducible on android device (my phone) - I have opened codesandbox on Samsung Galaxy S8 device. |
Can't reproduce on Motorolla here: https://txuow.csb.app/ |
Strange ;/ I have played for ~30 seconds with scrolling, expanding and collapsing and I got scroll jump ;/ Maybe I could look into code to see if I can find something. Is react-virtuoso somehow changing scrollposition at any moment? |
Yes, it does. In reverse scrolling, when it encounters an element with an unexpected size, it readjusts the scroll location. https://github.com/petyosi/react-virtuoso/blob/master/src/upwardScrollFixSystem.ts |
Thanks, I will give it a closer look |
I think (but it might not be true) that if offset change occurs for item that is below our current scrolltop position, browser reflow will do all things for us, and extra deviation push (and later scrollby) does no good. So modifying this system as so: should cover my use case and be safe to implement (hopefully) @petyosi sorry for tons of messages. What do you think? Edit: I have no clue how to properly pass scrollTop position to u.scan |
Before we go nuts down this rabbit hole - can you try disabling animations? I feel like something might come down to the fact that scrolling is combined with the animation changing offsets. |
Cool, we might be on to something here. scrollFix should not kick in, but it does. Can you push that into a branch somewhere? I will try to look into it in the next few days. I would appreciate it if you can as much additional config as possible that should not be not relevant to the example, like MUI components, overscan, headers, etc, and retest. Thank you very much for your help and patience with the code. |
Yeap. I will do it tomorrow morning and let you know. Thanks for helping me on resolving this issue! |
Ok, so I have removed code not relevant and published it here: I can confirm that bug still occurs after code cleaning. |
Hey! Have you maybe found some time to take a closer look into this issue? If I could provide some further help just let me know ;-) |
Hey, no, I haven't. Busy daytime job ATM unfortunately - I will likely take a look in the following days though. |
@cryptoethic can you try this on your side? |
Hey @petyosi. I gave it a try and it does work but when adding initialTopMostIndex > 0 causes same error to exist. Here is an example where I start with initial topmost item index = 10. Unfortunately we are using initialTopMostIndex in our app at some places. |
The entire setup in the system is related to reverse lists that start from a certain location rather than the top. It tries to detect items that have a height different than expected, and adjust the scroll location so that the list does not jump. There are a few test cases in the e2e directory that check that. Expressing the above is easier in prose than in code. In certain moments, your test case (expandable cards) checks all the checks of the system, causing it to apply the re-positioning (which, in my case, works as intended - it keeps the items below correctly positioned). I did manage to reproduce the repositioning but not the jump in the gif. Chances are, this is related to a specific scroll optimization on Windows, where I am afraid that, at this point, I can't help any further based on what I know. I have not discovered any way to overcome the |
Thanks for your time with this issue. I will spend more trying to solve this problem tommorow morning. As I understand this system should jump in when we scroll upwards and there is new item loaded for the first time with height different than expected. Without that system our scroll position is pushed by layout reflow and there is a scroll jump. If I'm right this system could execute only for the topmost loaded item as only this item can cause scroll jump. (so we have to check if item below the top has moved its offset) - there is no need for checking all loaded items. What's more if initial topmost item index is zero then we can safely disable that system. |
Up to your assumptions: loaded the first time - no. It can happen at any moment. Items can change size due to resizing, business logic change, etc. |
Thanks for your input. So what I can suggest is my first idea. execute scrollBy only if it is caused by items above our current scrollTop position. If it is caused by items below current scrollTop - browser reflow should handle correct scroll position. |
Hi!
I got something hard to reproduce which cause us a lot of pain.
https://codesandbox.io/s/react-virtuoso-reverse-scrolling-header-issue-forked-fj5ys?file=/src/App.js
Sometimes when i scroll up/down reflow occurs and my scroll position is moved from place where I have been frame before.
I have prepared sandbox with expandable cards (onClick), and this gif showing the issue (it does not happen all the time but is frequent enough to be painful)
![virtuoso flickering](https://user-images.githubusercontent.com/44468286/126914596-92135e55-7474-479f-85e6-ad296390aefe.gif)
Any idea what can cause this issue? I will appreciate any help ;-)
The text was updated successfully, but these errors were encountered: