petyosi / react-virtuoso

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

Reset endReached #182

Closed WillSquire closed 1 year ago

WillSquire commented 4 years ago

Using the infinite loader implementation, if I scroll to the end of the list and then change something to retrieve a newly filtered or sorted list, then it may or may not load more if I scroll to the bottom of the list again. I'm not sure if this has something to do with the count being used to determine if the bottom of a list has been reached and then not being triggered again if it doesn't change?

petyosi commented 4 years ago

Hey,

endReached is triggered only once when a certain index is reached - so yes, if the total count does not change, you won't get notified about it. This made sense for the use case I had in mind, but it might not work for another one. Can you post an example so that I can take a look? Thanks.

WillSquire commented 4 years ago

Hi @petyosi ,

Ah, that sounds like the case. I've dissected some of the code to be as minimal as possible, but I'm not sure how useful it will be. If I change sort it will probably be the same sized list if it's cached from previous. apollo can serve cached data on the next render so it'll remain the same without a break (i.e. without setting it to [] or null)

const {
  data,
} = useQuery({
  variables: {
    sort,
  },
})

<Virtuoso
  overscan={500}
  item={Item}
  endReached={onLoadMore}
  totalCount={data?.length}
/>

Hope this helps?

WillSquire commented 3 years ago

@petyosi Just to give a bit more info, I think this is relevant:

This would come back as less than the first and would still require another page to be loaded.

petyosi commented 3 years ago

Thank you - I need to think this through. In the case you describe, the endReached should be reset - but I am not sure how to do this in a predictable manner. You can try atBottomStateChange event in the meantime.

WillSquire commented 3 years ago

Thanks, I'll give that's a try! Not sure if this is applicable, but would an array comparison help in this situation? (i.e. oldData !== newData or useMemo)

jiminy-billy-bob commented 3 years ago

+1 I have run into the same issue.

My use case: a list of items in a marketplace. When adding keywords in the searchbar, the items in the list are replaced, but the item count doesn't change (except if the users has already scrolled, but by default it doesn't).

ArjobanSingh commented 3 years ago

+1 I also run into same issue. So i am using it to make kind of endless scrolling grid, which fetches more data over the api, after reaching the end. But if, due to some error, api doesn't give new data or maybe due to some network error( as user lost connection in between), the data count remains same, but when, connection again gets established and user tries to load more data by reaching end, the onEndReached does not fire again And further atBottomStateChange is not available in VirtuosoGrid

petyosi commented 3 years ago

@WillSquire @jiminy-billy-bob @ArjobanSingh Would exposing a resetEndReached() method work for you? I can't think of any reasonable way to detect the logic flow you refer to. Please 👍/👎.

jiminy-billy-bob commented 3 years ago

Yes, that would totally be usable.

ArjobanSingh commented 3 years ago

@WillSquire @jiminy-billy-bob @ArjobanSingh Would exposing a resetEndReached() method work for you? I can't think of any reasonable way to detect the logic flow you refer to. Please /.

i guess, if on resetting end reached, virtuoso grid fires onEndReached again on reaching end, than ofcourse, this method will work.

ArjobanSingh commented 3 years ago

or is there a way, where we can attach onScroll method, which will give us offset in params - like normal onScroll in react element, this way we can count when offsettop is 0, and invoke our load more function.

petyosi commented 3 years ago

@ArjobanSingh - onScroll callback is coming. In fact, it is already working in v0.21.0-alpha.3 which I am promoting for an early feedback.

ArjobanSingh commented 3 years ago

cool, is alpha version released for usage?

petyosi commented 3 years ago

@ArjobanSingh Yes, It is. Notice that there are certain deprecated properties, so watch your console for warnings.

ArjobanSingh commented 3 years ago

Ok thanks man, is it safe to use alpha version on production now?

petyosi commented 3 years ago

@ArjobanSingh If possible, please try and upgrade your project to it and report any issues you encounter. I have tested it on all the examples from the site and it works as expected, with certain modifications related to the deprecations.

github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version 1.2.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

petyosi commented 3 years ago

Hopefully, the introduced change addresses the problem - refreshing the data resets the endReached state, even if the data has the same length as the previous one.

felippepuhle commented 3 years ago

@petyosi I feel like sometimes endReached is not called and I'm not sure why... we're using apollo too, using data instead of totalCount seems to help, but the issue still there... is there anything I can do to prevent it?

petyosi commented 3 years ago

@felippepuhle Nothing I can recommend without a repo. If you get one, please open a new issue. Thank you!

felippepuhle commented 3 years ago

@petyosi not sure if I should open a new issue as I don't know exactly how to create/reproduce it(it's not constant but it happens a lot sometimes)... I was digging for a bit and this condition seems to be causing it... for example, after an initial load items[items.length - 1].originalIndex is 9 but totalCount - 1 is -1...

atBottomStateChange works 100% of the time but it's not exactly what we want... is there anything else I can do so we can better investigate and fix this?

thanks!

petyosi commented 3 years ago

@felippepuhle I can't say for sure what happens. You can try throwing a throttleTime (like startReached here) before the filter and see if it makes any difference.

felippepuhle commented 3 years ago

It does seem to help @petyosi, can we add it and release a new version(I can open a PR if you want)? thanks!

petyosi commented 3 years ago

Reopening this as a reminder. I will look into it shortly.

muyiwaoyeniyi commented 3 years ago

@petyosi Not sure this object reference change fix extends to grouped virtuoso since data prop isn't passed in when using it. Will changing the groupCounts also reset the endReached? Also, do you reset startReached too?

petyosi commented 3 years ago

@muyiwaoyeniyi no, the scope of this discussion does not include grouped mode. It would help me if you can report your problem in a separate issue, including reproduction and some context on what you are trying to achieve (and why it does not work).

muyiwaoyeniyi commented 3 years ago

@petyosi Thanks for quick response and awesome package. I will open another issue on grouped mode.

For resetting endReached, do you also reset startReached?

petyosi commented 3 years ago

For resetting endReached, do you also reset startReached?

No - I am not sure why that would be necessary. Please provide an example + context - thanks!

muyiwaoyeniyi commented 3 years ago

The context is similar to what was described in this comment https://github.com/petyosi/react-virtuoso/issues/182#issuecomment-729567934 .. I have a slack-like scrolling container that let's a user scroll and fetch data in both directions. For any number of reasons, it's possible to want the startReached (like the endReached) to be re-triggered.

heypran commented 2 years ago

@petyosi endReached issue seems to be unresolved... the solution suggested by you earlier for throttle time, can we create a fix based on that?

yahayahu commented 2 years ago

Try it tableRef.current?.scrollToIndex({index: 0});. Worked for me 😉

martinchrzan-workday commented 1 year ago

I am facing the same problem - my scenario is slightly different - I have a list which loads only 2 items at a time and you can fit 10+ items on the screen. When I load this list, I fetch 2 items and get endReached callback - I call fetch to get another 2 items but this is it, endReached will not be called again even though I still have a space on the screen to load more items. Any remedy for this? atBottomStateChange is not working for me because the list never reached bottom

oyalhi commented 1 year ago

I have similar issue. atBottomStateChange seems like a hack that we don't want to implement. A resetEndReached would work pretty nicely if available.

petyosi commented 1 year ago

@oyalhi reproduce your problem in a codesandbox. That would help.

oyalhi commented 1 year ago

@petyosi here is the issue in CodeSandbox: https://codesandbox.io/s/virtuoso-potential-bug-wq84um

something to do with the fetch size, also screen size (number of rows shown) has some effect. setting the overflow to 0 should make the issue reproducible on your display as well.

demo might be useful as well: https://drive.google.com/file/d/1JMceJwZlT-sFBL9u7p-6tc4s2yW2DgO2/view?usp=sharing

you can see that when overflow is 0, meaning we fetch exactly as the batch size for trip2, returning back to trip1 do not initiate a fetch. changing overflow value makes it work as normal.

overflow value decides the number of items trip2 & trip3 has in addition to the batch size.

in the example, trip 1 has 500 items, trip 2 & trip 3 has batchSize + overflow items.

petyosi commented 1 year ago

@oyalhi when switching between datasets (in your case, trips), you should reset the component state by passing a different key property for each set.

With that, I am closing this issue. If anyone experiences something related, open a new issue with clear and reliable reproduction.