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

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

Closed
schroda opened this issue Jun 12, 2023 · 4 comments
Closed

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

schroda opened this issue Jun 12, 2023 · 4 comments
Labels
duplicate This issue or pull request already exists wontfix This will not be worked on

Comments

@schroda
Copy link

schroda commented Jun 12, 2023

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):

  • OS: Windows 11 Pro (10.0.22621 Build 22621)
  • Browser:
    • Microsoft Edge (Version 114.0.1823.43 (Official Build) (64-Bit))
    • Firefox (114.0.1 (64-Bit))
    • Chrome (Version 114.0.5735.110 (Official Build) (64-Bit))

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

@schroda schroda added the bug Something isn't working label Jun 12, 2023
@petyosi petyosi added duplicate This issue or pull request already exists wontfix This will not be worked on and removed bug Something isn't working labels Jun 13, 2023
@petyosi
Copy link
Owner

petyosi commented Jun 13, 2023

Duplicate of #919

@petyosi petyosi marked this as a duplicate of #919 Jun 13, 2023
@petyosi petyosi closed this as not planned Won't fix, can't repro, duplicate, stale Jun 13, 2023
@schroda
Copy link
Author

schroda commented Jun 13, 2023

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
Copy link
Owner

petyosi commented Jun 13, 2023

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
Copy link
Author

schroda commented Jun 13, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants