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

Cell caching (while scrolling) is not working with WindowScroller #638

Closed
olavk opened this issue Apr 3, 2017 · 7 comments
Closed

Cell caching (while scrolling) is not working with WindowScroller #638

olavk opened this issue Apr 3, 2017 · 7 comments

Comments

@olavk
Copy link
Contributor

olavk commented Apr 3, 2017

Problem: Scrolling with WindowScroller is sluggish, because all cells on screen are re-rendered on every scroll event. There is an internal caching mechanism that should work while scrolling, but not if WindowScroller is used.

defaultCellRangeRenderer checks for cell cache before rendering a cell, but only when isScrolling is true (also something about scaling?). If isScrolling == false, it will always re-render cells.
This is passed in from Grid::_calculateChildrenToRender, which reads isScrolling from Grid.state. Grid.state.isScrolling is only set true, when onScroll is called from div.ReactVirtualized__Grid.

If WindowScroller is used, Grid.state.isScrolling is always set to false.

Proposed solution: allow isScrolling to be overridden from Grid.props. That way any component which renders Grid (and they all pass ...this.props to Grid) can read isScrolling from WindowScroller, and pass that down. E.g.

<WindowScroller>
  {({height, scrollTop, isScrolling}) => (
    <Table
      autoHeight
      height={height}
      scrollTop={scrollTop}
      isScrolling={isScrolling}
      ...
    >
      {columns}
    </Table>
  )}
</WindowScroller>

Will make a pull request.

@dfdeagle47
Copy link
Contributor

Good catch. I'm currently having performance issues with the WindowScroller as well.

Until the pull request is merged, a temporary solution for the List might be

import { defaultCellRangeRenderer, List } from 'react-virtualized;
//...

<WindowScroller>
  {({height, scrollTop, isScrolling}) => (
    <List
      autoHeight
      height={height}
      scrollTop={overrideScrollTop || scrollTop}
      defautCellRangeRenderer={params => defaultCellRangeRenderer({ ...params, isScrolling })}
      ...
    >
      {columns}
    </Table>
  )}
</WindowScroller>

The only thing missing from the hack is (Grid.js)

              pointerEvents: isScrolling ? 'none' : '',

@bvaughn
Copy link
Owner

bvaughn commented Apr 3, 2017

Great issue write-up. Thanks for digging in and writing up a proposal.

defaultCellRangeRenderer checks for cell cache before rendering a cell, but only when isScrolling is true (also something about scaling?)

These couple of slides give a little background info on the scaling issue. Basically I'm compressing positions and adjusting offset slightly as a user scrolls- so if I were to cache style (eg cache position) in this case, cells would get misaligned slightly during scroll. It's complicated. ☹️

@dfdeagle47 is right that you can temporarily work around this by passing a decorated defaultCellRangeRenderer as the cellRangeRenderer prop. I like your proposed solution though so I don't see any reason we can't move forward with it.

@dfdeagle47
Copy link
Contributor

@bvaughn: thanks for the link, that slide deck is golden.

@bvaughn
Copy link
Owner

bvaughn commented Apr 3, 2017

👍

@bvaughn
Copy link
Owner

bvaughn commented Apr 3, 2017

This change has been pushed via 9.5.0. Thanks for your help!

@bvaughn bvaughn closed this as completed Apr 3, 2017
@olavk
Copy link
Contributor Author

olavk commented Apr 4, 2017

Thank you!

@mifozski
Copy link

mifozski commented Mar 28, 2019

Just a heads-up for anyone who's still having a problem with rows re-rendering on each scroll even after passing the isScrolling prop to List: isScrollingOptOut prop also has to be set to true for the List's caching mechanism to work properly.

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

4 participants