petyosi / react-virtuoso

The most powerful virtual list component for React
https://virtuoso.dev
MIT License
5.25k stars 301 forks source link

Is it possible to pre-compute all row heights? #89

Closed dilizarov closed 3 years ago

dilizarov commented 4 years ago

I'm trying to build a chat app, but the size of messages could vary wildly. The chat starts from the bottom (so I want to use initialTopMostItemIndex={messages.length - 1}.

The problem is that because you can have large differences message heights, when you scroll up it is very janky.

This is only a problem because obviously the height of each row has not been computed, so it is computing them on the way up.

44 I checked this out and had hope for the <Virtuoso style={{rotate(180deg) scaleX(-1)}}> solution presented by @iJimmyWei, but unfortunately, it messes up the scrollbar direction, so it isn't usable.

Possibly a feature request, but Is there a way to pre-compute all row heights? Perhaps via a prop? It's not feasible for all products, but for some, it won't actually impact performance much I imagine.

Another change would be to measure all the rows on the way down in initialTopMostItemIndex but I fear that could be expensive.

I'm exploring this project as a replacement for react-virtualized in my current project which while great can be a nightmare to get some features to work.

petyosi commented 4 years ago

@dilizarov I have explored the scenario you describe - the jank is there indeed, and I have not found a proper way to fix it. Pre-computing all heights is not an option - you have to render everything. And the same operation should be performed if the list width changes (for example due to browser resize, content change, device rotation).

A creative way I have seen for apps to get around that problem is placeholders or even a loading overlay. Teams does that, and Twitter does something similar if you scroll deep down.

dilizarov commented 4 years ago

Hmm. Teams? I've tried replicating it with Twitter and I don't seem to see a loading overlay :/. I do see that if you scroll fast enough though that it's basically all white in twitter (like the items don't render. It takes them some milliseconds to appear on screen). Is that what you mean?

petyosi commented 4 years ago

Yes, this is the behavior I referred to. Microsoft Teams is a chat app.

jmeistrich commented 4 years ago

I was encountering a similar problem and I noticed scroll was stuttering only if the initially topmost item was unusually large. Setting defaultItemHeight to my lists's minimum item size solved the problem for me. @dilizarov Maybe that will fix it for you? @petyosi I can't quite follow the internal logic of how defaultItemHeight is used, but if no default is provided would using the minimum size of all rendered items as the expected default work better?

utileetdulce commented 4 years ago

Thank you very much for this great library! In our use case we have a list of objects with varying heights, but the total height can easily be calculated in advance. @petyosi Is there a way to pass this total height to Virtuoso instead of using the internally calculated totalHeight?

petyosi commented 4 years ago

@jmeistrich - I could not come up with a universally right way to come up with the default height. Picking the smallest one might be too much of a sandbagging. Imagine 200 items with 2 lines of text, being spoiled by a single 1 line item.

@utileetdulce - no, there is no such capability. The purpose of the component is to avoid such calculations.

jmeistrich commented 4 years ago

Is there a performance implication with having a small defaultItemHeight? I am not seeing any performance implications in my usage. But if it's too big there's a lot of stuttering while scrolling up when using initialTopMostItemIndex.

petyosi commented 4 years ago

Setting a small defaultItemHeight will render many more items than necessary. To give an example, if items are usually 30px tall, setting defaultItemHeight to 10px will render 3 times more items than necessary. Depending on the content of the items and the height of the list, this can cause performance implications.

Your observation is also correct. Setting it to a large number is also bad. The list has to re-render multiple times to compensate for the insufficient height.

jmeistrich commented 4 years ago

I had found that setting defaultItemHeight to the minimum height of an item in my list appeared to fix the scroll jumping, but it also caused some scenarios where no items would render at all. Then I found that removing:

if (initialTopMostItemIndex) {
  initialOffsetList = initialOffsetList.setInitialIndex(initialTopMostItemIndex);
}

from the top of offsetListEngine appears to fix it. It doesn't seem to cause any significant issues for me. But is there some problem this will cause that I'm not seeing?

petyosi commented 4 years ago

Nah, this is a critical piece of the code. I am not sure if this issue is also the right one to discuss the problem. Why don't you open a new one with a repro?

petyosi commented 3 years ago

The problem is that because you can have large differences message heights, when you scroll up it is very janky.

This should work fine in v1, see #203 for instructions on upgrade.