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

MultiGrid falls out of sync when scrolling #971

Closed
OleksandrNechai opened this issue Jan 16, 2018 · 9 comments
Closed

MultiGrid falls out of sync when scrolling #971

OleksandrNechai opened this issue Jan 16, 2018 · 9 comments

Comments

@OleksandrNechai
Copy link
Contributor

When scroll (with a scroll wheel on the mouse, Windows 10, Chrome, Edge, Firefox) during a very short period of time parts of MultiGrid are out of syn, then they get back to sync.

This is how it looks like irrespective of the size of the grid:

Here is a screenshot of this moment when main data grid and it's fixed part are out of sync:

Here the CodePen to reproduce.

@bvaughn
Copy link
Owner

bvaughn commented Jan 17, 2018

This looks like a duplicate of #647 and #291. Check out my responses to those issues (like this one).

I don't think there's anything react-virtualized can do about this, short of moving everything in the the UI thread (like fixed-data-table does) but this incurs a very serious cost of making scrolling feel jumpy and unresponsive if rendering and layout takes longer than the 16ms window, as described here. (The decision to use async, "scroll" event was a conscious one from the start for this project.)

There are some techniques that you could try (discussed on #291) to force the event handling to be sync for your synced grids, if you'd like. I haven't tried them myself so I can't vouch for them.

I always welcome PRs to improve performance and developer experience for react-virtualized, and fresh eyes often have new ideas- so I'd welcome you to contribute if you have ideas for ways to improve this. As it is though, I think this issue is "working as designed" (for the reasons mentioned above).

@bvaughn bvaughn closed this as completed Jan 17, 2018
@OleksandrNechai
Copy link
Contributor Author

Brian, thanks a lot for this project and nice fast response to this issue. And sorry for duplication, I have tried to avoid that but failed :-).

I have checked out fixed-data-table and it scrolls very well for me. So just out of curiosity, is this performance penalty for using UI thread for scrolling only noticeable when cells have a complex and slow layout to render? If so, why not have a prop which controls whether to use/not use UI thread? I am very new to web front-end world, sorry if my question is stupid :-).

@bvaughn
Copy link
Owner

bvaughn commented Jan 17, 2018

No worries. It's hard when there are hundreds of issues. I have trouble deduping myself sometimes, and I've seen them all before.

So just out of curiosity, is this performance penalty for using UI thread for scrolling only noticeable when cells have a complex and slow layout to render?

Yes-ish. This will vary based on the specs of the computer running it, how much work it is to render the cells, what else is being done at that time in the page (eg other JavaScript and rendering), etc.

If so, why not have a prop which controls whether to use/not use UI thread?

Complexity! 😄 I don't want to build two implementations.

PS. No worries. It's not at all a stupid question.

@wtfil
Copy link

wtfil commented Aug 9, 2018

@OleksandrNechai
I found the solution for Chrome. It looks like browser optimization rather than solution, but still, it works fine

on root node of your component you have to add this css

#example::after {
    content: '';
    position: absolute;
    top: 0;
    left: 0;
    width: 100%;
    height: 100%;
    pointer-events: none;
}

Going to dig into rendering theory and found solutions for other browsers and I can make PR if I am lucky

@OleksandrNechai
Copy link
Contributor Author

@wtfil , thank you very much for looking at this problem :-) It still bugs me. My users, however, do not complain yet :-)

@tuchk4
Copy link

tuchk4 commented Sep 24, 2018

@wtfil nice workaround. Scroll is really synced but for some reason this solution makes grid slow to be slow. But synced with fixed columns and rows :)

@bvaughn I have question about fixed columns / rows feature.
Maybe for simple cases (1 fixed row / 1 fixed column) don't need to render multiple Grids?
Always render cells:

  • fixed row ( rowIndex=0) and all it's columns. Provide style where top === Grid.scrollTop
  • fixed column (each rowIndex, columnIndex=0). Provide style where left === Grid.scrollLeft

It should be very simple in runtime and without scroll sync problems because there is nothing to sync.

@tuchk4
Copy link

tuchk4 commented Sep 25, 2018

I made some experiments

  • this solution with position: absolute will not work because of _debounceScrollEnded();. Fixed columns will be scrolled out and then appear again on scroll end.
  • It is possible to implement only if set them as position: sticky but as I understand this way is not stable :(

@aiingstan
Copy link

aiingstan commented Dec 2, 2018

I made some experiments

  • this solution with position: absolute will not work because of _debounceScrollEnded();. Fixed columns will be scrolled out and then appear again on scroll end.
  • It is possible to implement only if set them as position: sticky but as I understand this way is not stable :(

@tuchk4
I did exactly those 2 things today and had almost the same conclusions with you!
Can you share with me how do you use the position: sticky way to make the left column be fixed? I found that if cells inside the left column are scrolled out of the window they'll be cleared?

As for me, I plan to append a list inside the outter div to achieve something like this CodePen Demo

@wtfil
Copy link

wtfil commented Dec 2, 2018

@tuchk4 The problem with async scroll is not something directly related to the react-virtualized, but rather browser internal rendering algorithms. Despite the fact that columns might looks like they have different scrollTop while rapid scrolling, the browser will keep them the same, but render asynchronously.
I guess my "trick" tells browser to render entire box as the single unit, so it does. However, this does not work in FF and on Windows, so this is definitely not the best fix

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

No branches or pull requests

5 participants