petyosi / react-virtuoso

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

[BUG] Grid - "endReached" doesn't get called #924

Closed schroda closed 1 year ago

schroda commented 1 year ago

Describe the bug There are cases where "endReached" never gets called when using Grid. The following cases are the ones I noticed:

  1. Initial render wit not enough items for the scrollbar to appear
  2. Initial render with scrollbar depending on the number of items, height and/or overscan

Reproduction The following codesandbox is a fork of the "Grid - Responsive Columns" example of the documentation with slight changes to cause the bug https://codesandbox.io/s/falling-darkness-gcgnp7?file=/App.js

Expected behavior (use-case: infinite scrolling)

  1. In case the initial items aren't enough to cause the scrollbar to appear (e.g. on a big screen) I would expect "endReached" to get called which then could be used to trigger the load of the next page (same as it works for lists)
  2. When reaching the end "endReached" should get called to be able to load more items

Desktop (please complete the following information):

Same codesandbox has no issues for the 2nd case on iPhone 13 Pro - Chrome (Versoin 114.0.5735.99)

petyosi commented 1 year ago

Duplicate of https://github.com/petyosi/react-virtuoso/issues/919

schroda commented 1 year ago

Maybe I'm missing something and am completely wrong, but I don't think this is a duplicate of the mentioned ticket.

For the 1st case: For the case (for Lists) of the mentioned ticket "endReached" does get called one time and triggers loading more items. In my case (for Grids) "endReached" event never gets fired. I've updated the code sandbox and added the same case using a List, where "endReached" gets fired.

If the event should never get fired then it looks like the List and Grid behave differently.

For the 2nd case: There are enough items for the scrollbar to appear and to have to scroll down to reach the end of the Grid. But if the end is reached the event doesn't get fired.

I've updated the code sandbox and added an example where just increasing the height by 25px will cause the event to not get fired anymore. For both cases there is a scrollbar and you have to scroll down to reach the end. But for one example it works, for the other it doesn't

petyosi commented 1 year ago

The list in your example is "broken" by the styling you apply - see how the items reside next to each other. See the sandbox from 919.

At any case - you should not design your logic around passing small amount of items and expecting the component to "pull" additional ones with endReached being called on immediately on render. I consider this to be an anti-pattern that can lead to recursive re-rendering and unresponsive UI.

To clarify further on your variations: end reached gets called when the last item is rendered. Overscan setting can cause this to happen with the initial render.

schroda commented 1 year ago

The list in your example is "broken" by the styling you apply - see how the items reside next to each other. See the sandbox from 919.

My bad, I fixed the list, still the same behaviour (without overscan applied).

At any case - you should not design your logic around passing small amount of items and expecting the component to "pull" additional ones with endReached being called on immediately on render. I consider this to be an anti-pattern that can lead to recursive re-rendering and unresponsive UI.

I understand that, but due to the List behavior I did expect it to atleast get called once.

To clarify further on your variations: end reached gets called when the last item is rendered. Overscan setting can cause this to happen with the initial render.

Okay so that is where my misunderstanding is coming from. I thought it gets called when the end (of the component) is reached. Which is also a bit confusing when you have a scrollbar and scroll down to the end and the event doesn't get fired to be honest.