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

Re-think isScrolling param and debounce behavior #863

Closed
bvaughn opened this issue Oct 31, 2017 · 3 comments
Closed

Re-think isScrolling param and debounce behavior #863

bvaughn opened this issue Oct 31, 2017 · 3 comments

Comments

@bvaughn
Copy link
Owner

bvaughn commented Oct 31, 2017

The isScrolling parameter can be used to render a light-weight representation of a cell while the user is scrolling to avoid eg doing too much paint and layout work, making unnecessary network requests, etc.

Trouble is, the value of isScrolling is one of the factors used to determine whether cellCache is used or not - meaning that, if it's not used, this parameter can have a net negative impact on performance.

This behavior can be worked around currently by setting the value of scrollingResetTimeInterval to 0 to disable the debounce behavior. Perhaps the API should change to make isScrolling something that is opt-in instead of opt-out?

@MrNice
Copy link

MrNice commented Nov 1, 2017

I can say that I tried to use isScrolling and it significantly degraded performance (when tested on
a macbook pro retina). It seemed like it caused more work to happen in one chunk once scrolling stopped. Sorry if this is unhelpful.

@bvaughn
Copy link
Owner Author

bvaughn commented Nov 1, 2017

No worries. Feedback is helpful. That's why I tagged this issue "discussion".

It seemed like it caused more work to happen in one chunk once scrolling stopped.

It would require one final render after scrolling stops (to switch from isScrolling true -> false). This may or may not be net positive, depending on what your app is doing.

@djeeg
Copy link
Contributor

djeeg commented Dec 18, 2017

This is definitely an interesting point.

I spent a fair bit of time going down the isScrolling path, trying to improve the perf of the scrolling Grid. Did do a fair bit of debugging however couldnt really get the performance close to what I wanted.
#728
#717

The demo page works really well (probably because its a very simple box to render). As soon as you start throwing a more complex cell at the scroll, the render performance takes a hit.

I currently have that special render logic for isScrolling disabled and am more reliant upon CellMeasurerCache to provide a smooth scrolling experience. Specifically I can luckly use both fixedWidth=true and fixedHeight=true.

@github-actions github-actions bot added the Stale label Oct 15, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants