Skip to content

[DataGridPro] Use intersection observer to trigger server-side infinite loading #17369

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

Merged
merged 4 commits into from
Apr 25, 2025

Conversation

arminmeh
Copy link
Contributor

I have extracted IntersectionObserver logic into a separate hook and moved it to the server side feature folder, since the client side feature is deprecated.
Both client and server side infinite loading are now listening to the new internal event from this hook to handle the data update

There are two main reasons for doing this:

  1. With the intersection observer we are covering more cases where the new data fetch should occur. One of them is if the page size is not big enough to fill the whole viewport. With the observer additional data is fetched until the viewport is filled.
  2. IntersectionObserver events are far less frequent than scroll events, so handling those reduces the impact on the grid performance

@arminmeh arminmeh added type: bug A bug or unintended behavior in the components. scope: data grid Changes or issues related to the data grid product feature: Row loading Related to the data grid Row loading features feature: Server integration Better integration with backends, e.g. data source labels Apr 15, 2025
@arminmeh arminmeh requested a review from a team April 15, 2025 11:25
@mui-bot
Copy link

mui-bot commented Apr 15, 2025

Deploy preview: https://deploy-preview-17369--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 14c7c4d

@arminmeh arminmeh force-pushed the align-infinite-loading branch from 1bc8f2e to 9615558 Compare April 16, 2025 04:59
@arminmeh arminmeh added the plan: Pro Impact at least one Pro user label Apr 16, 2025
* Used to trigger infinite loading.
* @ignore - do not document.
*/
onIntersection: {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Event names usually don't contain "on", it's the handler which contains it. For example, "click" event, and the corresponding handler would be "onclick". How about making it "intersectionObserved" or simply "intersection"?

Suggested change
onIntersection: {};
intersection: {};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps calling it rowsScrollEndIntersection would be more meaningful?

Copy link
Contributor Author

@arminmeh arminmeh Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go for intersection because this hook does not know where the trigger element is placed

Update:
Well, this is not entirely true
hook is using scrollEndThreshold prop and adding bottom margin to adjust the intersection area
so, even though you can place the element anywhere, it will not work anywhere

I will use rowsScrollEndIntersection

it is internal event anyway.
once/if we decide to make it generic, we can change this alongside with the other suggestion from #17369 (comment)
in that case, the check #17369 (comment) should also be completely removed

@arminmeh arminmeh force-pushed the align-infinite-loading branch from 9615558 to 77c3be8 Compare April 24, 2025 06:19
@arminmeh arminmeh force-pushed the align-infinite-loading branch from 4f54526 to 14c7c4d Compare April 25, 2025 06:18
@arminmeh arminmeh enabled auto-merge (squash) April 25, 2025 06:18
@arminmeh arminmeh merged commit 1d2a4eb into mui:master Apr 25, 2025
19 checks passed
@arminmeh arminmeh deleted the align-infinite-loading branch April 25, 2025 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: Row loading Related to the data grid Row loading features feature: Server integration Better integration with backends, e.g. data source plan: Pro Impact at least one Pro user scope: data grid Changes or issues related to the data grid product type: bug A bug or unintended behavior in the components.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants