petyosi / react-virtuoso

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

Missing items when scrolling up/down #166

Closed jaapbakker88 closed 4 years ago

jaapbakker88 commented 4 years ago

First off, thanks for creating this awesome library. We're using it to build chat and we're trying to implement virtualised scrolling on it. We know there's issues regarding this, but it seems we've got it working. We use prepend items when we detect the startIndex going below 10:

rangeChanged={({ startIndex }) => {
  if (mounted.current && hasMore && startIndex < 10)
    loadMore().then(virtuoso.current.adjustForPrependedItems);
  }
}

the issue

The issue we're having though is the following: when you scroll up a bit and down again. Sometimes a few(2/3) items are missing. Scrolling up and down again shows them again. We tried hacking around it by increasing the totalCount by 2/3. Which improves it, but the issue is still there.

See our implementation here: https://github.com/GetStream/stream-chat-react/blob/virtuoso-test/src/components/MessageList/FastMessageList.js

And you can see it running here: https://codepen.io/team/stream/pen/vYGLwOy?editors=0010 Changing the hackNumber prop on FastMessageList increases the totalCount by whatever number you pass to it.

I don't feel we're doing anything crazy here, and any pointers to why this may be happening are very appreciated.

petyosi commented 4 years ago

Hey @jaapbakker88 ,

First, I am currently working on a new iteration of the library that is specifically designed for chat interfaces. It is not released yet; if you are interested in using it in your company's product, please reach me at the email in my profile, so that we can discuss it further.

Up to your example - I haven't seen that behavior in the simplistic examples I used when I tested the prepend behavior. I suspect that chat bubbles changing size can cause that, but that's just a wild guess. Can you reproduce it with fewer moving parts? Cheers,

jaapbakker88 commented 4 years ago

@petyosi thank you for your swift response, I emailed you about the iteration of the library for chat interfaces. I also created a version of the library where we've disabled prepending messages completely. And removed all styling from the messages as well. You can enable loading more by removing the disableLoadMore prop from FastMessageList.

https://codepen.io/team/stream/pen/vYGLwOy?editors=0010

As you can see it seems to have to do with the height of the container, if you make it smaller, it actually won't show the last few messages anymore. If you make it bigger it does a little better, but once you start adding more messages it get's confused again.

Here's also a video that show's it a bit better: https://drive.google.com/file/d/1HuiCcp1TByBEIy0mYp6yGU60AXoLn9NH/view?usp=sharing

and our current virtuoso code:

<div className="str-chat__fast-list">
      <Virtuoso
        ref={virtuoso}
        style={{ width: width || '100%', height: height || '100%' }}
        totalCount={messages.length + (hackNumber || 0)}
        item={(index) => <p>message: {messages[index]?.text}</p>}
        // item={(index) => itemRenderer(messages[index], messages[index - 1], messages[index + 1])}
        rangeChanged={({ startIndex }) => {
          if (!disableLoadMore && mounted.current && hasMore && startIndex < 10) {
            loadMore().then(virtuoso.current.adjustForPrependedItems);
          } 
        }}
        // scrollSeek={{
        //   enter: (velocity) => Math.abs(velocity) > 220,
        //   exit: (velocity) => Math.abs(velocity) < 30,
        //   change: () => null,
        //   placeholder: ScrollSeekPlaceholder,
        // }}
      />
    </div>
petyosi commented 4 years ago

@jaapbakker88 Thanks, I spotted a problem in the demo - the messages contain paragraphs with margins:

image

Margins are the kryptonite of Virtuoso's logic, as they bleed outside of the container and don't bump its offsetHeight. As you can see in the screenshot, the component has measured the div to be 18px, which is not true ;).

As an easy fix, can you remove the margins of the paragraph and use padding instead?

Cheers,

jaapbakker88 commented 4 years ago

@petyosi that seems to fix the issue yeah, seems we have a working inplementation now as long as we keep messages super simple.

axeljeremy7 commented 4 years ago

Hey I am having the same issue and my other issue is that @jaapbakker88 I dont think you do re calculate the heights before render. I am building a chat too and having issue because each message has different height, any help on this @petyosi

petyosi commented 4 years ago

@axeljeremy7 check if the fix I provided works for you. The chat specific version is under development.

axeljeremy7 commented 4 years ago

@petyosi sorry if i misunderstood, what is the fix?

petyosi commented 4 years ago

As an easy fix, can you remove the margins of the paragraph and use padding instead?

@axeljeremy7 examine if you have any margins of the elements inside the items.