[data grid] onRowsScrollEnd fired without scrolling

[cherniavskii]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Current behavior 😯

• initial 3 rows are rendered
• onRowsScrollEnd is called (even though there was no scroll event)
• newly-added 3 rows are rendered as well
• scroll doesn't trigger onRowsScrollEnd anymore

Screen.Recording.2022-04-05.at.17.02.27.mov

Expected behavior 🤔

• initial 3 rows are rendered
• onRowsScrollEnd is not called, since there's no scroll event

Steps to reproduce 🕹

Steps:

1. Go to https://codesandbox.io/s/infiniteloadinggrid-material-demo-forked-4uve2q?file=/demo.tsx

Context 🔦

I've noticed it while investigating #4184

Your environment 🌎

`npx @mui/envinfo`

  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Order ID 💳 (optional)

No response

[liho00]

#7416 this pr can solve infinite scroll issue, pls take a look ya.

[yaredtsy]

hey @cherniavskii

there was a problem with scrollEnd detection. rowsScrollEnd cant be called multiple times continuously. it has some toggle mechanism. once the event is called it must go to ScrollBottomArea. removing it will fix this issue and #4184

$ git diff 'packages\grid\x-data-grid-pro\src\hooks\features\infiniteLoader\useGridInfiniteLoader.ts'
diff --git a/packages/grid/x-data-grid-pro/src/hooks/features/infiniteLoader/useGridInfiniteLoader.ts b/packages/grid/x-data-grid-pro/src/hooks/features/infiniteLoader/useGridInfiniteLoader.ts
index fbe62f935..4ebdf2c74 100644
--- a/packages/grid/x-data-grid-pro/src/hooks/features/infiniteLoader/useGridInfiniteLoader.ts
+++ b/packages/grid/x-data-grid-pro/src/hooks/features/infiniteLoader/useGridInfiniteLoader.ts
@@ -43,15 +43,11 @@ export const useGridInfiniteLoader = (

       if (scrollPositionBottom < contentHeight - props.scrollEndThreshold) {
         isInScrollBottomArea.current = false;
       }

-      if (
-        scrollPositionBottom >= contentHeight - props.scrollEndThreshold &&
-        !isInScrollBottomArea.current
-      ) {
+      if (scrollPositionBottom >= contentHeight - props.scrollEndThreshold) {
         const rowScrollEndParam: GridRowScrollEndParams = {
           visibleColumns,
           viewportPageSize,

[liho00]

@yaredtsy

hey @cherniavskii

there was a problem with scrollEnd detection. rowsScrollEnd cant be called multiple times continuously. it has some toggle mechanism. once the event is called it must go to ScrollBottomArea. removing it will fix this issue and #4184

$ git diff 'packages\grid\x-data-grid-pro\src\hooks\features\infiniteLoader\useGridInfiniteLoader.ts'
diff --git a/packages/grid/x-data-grid-pro/src/hooks/features/infiniteLoader/useGridInfiniteLoader.ts b/packages/grid/x-data-grid-pro/src/hooks/features/infiniteLoader/useGridInfiniteLoader.ts
index fbe62f935..4ebdf2c74 100644
--- a/packages/grid/x-data-grid-pro/src/hooks/features/infiniteLoader/useGridInfiniteLoader.ts
+++ b/packages/grid/x-data-grid-pro/src/hooks/features/infiniteLoader/useGridInfiniteLoader.ts
@@ -43,15 +43,11 @@ export const useGridInfiniteLoader = (

       if (scrollPositionBottom < contentHeight - props.scrollEndThreshold) {
         isInScrollBottomArea.current = false;
       }

-      if (
-        scrollPositionBottom >= contentHeight - props.scrollEndThreshold &&
-        !isInScrollBottomArea.current
-      ) {
+      if (scrollPositionBottom >= contentHeight - props.scrollEndThreshold) {
         const rowScrollEndParam: GridRowScrollEndParams = {
           visibleColumns,
           viewportPageSize,

I dont think onRowsScrollEnd callback is a proper way for infinite scroll.
For eg. DataGridPro with height 1000px, calling backend infinite scroll with limit 2 items, hence backend returning 2 items and next cursor, the 2 items are rendered on the DataGrid however unable to scroll, as 1 row took 50px only, hence unable handle next cursor infinite scroll.

#7416 check this PR's video...
as the video shown, the items loaded from backend but unable to scroll, hence cant fetch again for another cursor items...

[yaredtsy]

hey @liho00

handleRowsScrollEnd will be called every time the rows.length is changed. so every time data is fetched from the server it checks if scrollPositionBottom is greater than contentHeight - props.scrollEndThreshold and publishes the rowsScrollEnd event and it continues this loop until scrollPositionBottom is not greater than contentHeight - props.scrollEndThreshold. the problem was once rowsScrollEnd is published. it will be locked until scrollPositionBottom is less than contentHeight - props.scrollEndThreshold. and this won't happen until the rows exceed the Datagrid height.

mui-x/packages/grid/x-data-grid-pro/src/hooks/features/infiniteLoader/useGridInfiniteLoader.ts

Lines 46 to 50 in 0203346

	if (scrollPositionBottom < contentHeight - props.scrollEndThreshold) {
	isInScrollBottomArea.current = false;
	}

[liho00]

hey @liho00

handleRowsScrollEnd will be called every time the rows.length is changed. so every time data is fetched from the server it checks if scrollPositionBottom is greater than contentHeight - props.scrollEndThreshold and publishes the rowsScrollEnd event and it continues this loop until scrollPositionBottom is not greater than contentHeight - props.scrollEndThreshold. the problem was once rowsScrollEnd is published. it will be locked until scrollPositionBottom is less than contentHeight - props.scrollEndThreshold. and this won't happen until the rows exceed the Datagrid height.

mui-x/packages/grid/x-data-grid-pro/src/hooks/features/infiniteLoader/useGridInfiniteLoader.ts

Lines 46 to 50 in 0203346

	if (scrollPositionBottom < contentHeight - props.scrollEndThreshold) {
	isInScrollBottomArea.current = false;
	}

@yaredtsy nope,onRowsScrollEnd is not getting called every time when rows.length is changed...

    <DataGridPro
          initialState={{
            pinnedColumns: { left: ["email"], right: ["actions"] },
          }}
          rows={users}
          columns={columns}
          hideFooterPagination
          checkboxSelection
          disableSelectionOnClick
          loading={isLoading || isFetching}
          components={{
            LoadingOverlay: LinearProgress,
          }}
          onRowsScrollEnd={() => {
            console.log("sdjfhsiuf");
          }}
        />

Screen.Recording.2023-01-07.at.8.23.03.PM.mov

the video is the result with onRowsScrollEnd

[yaredtsy]

nope,onRowsScrollEnd is not getting called every time when rows.length is changed...

not onRowsScrollEnd, handleRowsScrollEnd will be called but it is not publishing onRowsScrollEnd event, every time.

mui-x/packages/grid/x-data-grid-pro/src/hooks/features/infiniteLoader/useGridInfiniteLoader.ts

Lines 74 to 79 in 0203346

	const handleGridScroll = React.useCallback<GridEventListener<'scrollPositionChange'>>(
	({ left, top }) => {
	handleRowsScrollEnd({ left, top });
	},
	[handleRowsScrollEnd],
	);

check this demo,
Demo : https://codesandbox.io/s/elegant-river-kws9h7?file=/demo.tsx
git-commit: a3c8fae

[m4theushw]

Passing scrollEndThreshold=1 solves the problem. There's nothing wrong with the logic that decides to publish the event or not. The problem is that, once new rows are added, the current scroll position still falls into the "bottom area" because the default value for this prop (80px) is too high for this example. If more rows were added, the scrollbar would increase and the user could scroll more than scrollEndThreshold, consequentially triggering the event again.

About the event being fired even without scroll, I think it's the correct behavior, although the name might be wrong. I see the event as a way to detect if the bottom edge of the content is visible, also understood as if the last row is visible. In the example given, the number of rows is so small that since the beginning the bottom edge of the content is already visible. The event has nothing to do with scrolling but we use the scroll event to detect if it should be published or not.

[yaredtsy]

I do not think setting scrollEndThreshold=1 will solve it completely. for example demo. In this demo contentHeight is too small its 104 and scrollPositionBottom is 290, setting the scrollEndThreshold=1 still does not make scrollPositionBottom < contentHeight - props.scrollEndThreshold condition true.

[liho00]

https://codesandbox.io/s/gifted-einstein-o1xc0e?file=/demo.tsx

@m4theushw yes scrollEndThreshold=1 is not working...

But I would like to recommend that Intersection Observer js api is the right & simple way for infinite scroll in this PR #7416

[m4theushw]

@liho00 You're using params.viewportPageSize to load new rows, but its value is zero. In this specific demo, with so little initial rows, using this param doesn't work. If you load a fixed number of rows, then scrollEndThreshold=1 solves the problem. See the updated demo: https://codesandbox.io/s/eager-mcnulty-x4uwyk

@yaredtsy You've set the wrong prop.

[liho00]

@liho00 You're using params.viewportPageSize to load new rows, but its value is zero. In this specific demo, with so little initial rows, using this param doesn't work. If you load a fixed number of rows, then scrollEndThreshold=1 solves the problem. See the updated demo: https://codesandbox.io/s/eager-mcnulty-x4uwyk

@yaredtsy You've set the wrong prop.

@m4theushw

nope u are trying 5 items with 400px heigh thats why no issues (second loaded items come with scrollbar which will be working fine), please try load 1 item with Grid height 400px or 2items with Grid height 800px,
the problem arise when loaded 3 times of item but still not scrollable, check this codesandbox again! https://codesandbox.io/s/dreamy-haze-r3xs7r?file=/demo.tsx

please also study my PR's problem statement, thanks in advance! #7416

[m4theushw]

@liho00 I understand the problem. Unfortunately, onRowsScrollEnd has this limitation that the number of loaded rows must be enough to make the scrollbar visible, so next time the user scrolls it works as expected. For this use case it doesn't work and we'll need to rework it. At the same time, this prop is attached to the scroll event and if there's no scroll, then it's not called. The idea of using IntersectionObserver is good, however, to be incorporated into the grid we can't just add a DOM element to the viewport, we need to make the complete solution. I propose to ditch onRowsScrollEnd and add an onLoadMoreRows prop, entirely based on IntersectionObserver instead of the scroll event.

The scrollEndThreshold prop can feed the rootMargin option.

cc @DanailH

[yaredtsy]

Hey @m4theushw, I don't think that changing it to IntersectionObserver will solve this specific issue because the problem is caused by the toggle mechanism, the code that checks if the scroll position has left the bottom scroll area before firing the event again, preventing onRowsScrollEnd from being called infinitely. Therefore, even if it's changed to IntersectionObserver, this toggle mechanism must still be implemented. otherwise, onLoadMoreRows will be continuously called as long as the last row is in the viewport.

[m4theushw]

onLoadMoreRows will be continuously called as long as the last row is in the viewport

@yaredtsy That's the idea when I proposed to use IntersectionObserver. I wanted to simplify the logic to be only "is the end of the list visible? if yes, call onLoadMoreRows until it's not needed".

[DanailH]

A quick note: I've explored the idea of using the IntersectionObserver to replace the clunky check. The main issue is that if you go with the solution of adding an empty element (a div for example) at the bottom of the GridVirtualScrollerRenderZone then the onLoadMoreRows will be called on the first render before the rows are created/loaded. As far as I can see this is the only problem because you do want the callback to be called when you scroll to the bottom, then scroll up and back to the bottom again.

To avoid this another solution is to add a specific className to the last row that is currently loaded but it creates complications with the observable as you need to observe only after the rows are loaded and update which row has the distinct className once the user loads more rows.

[liho00]

A quick note: I've explored the idea of using the IntersectionObserver to replace the clunky check. The main issue is that if you go with the solution of adding an empty element (a div for example) at the bottom of the GridVirtualScrollerRenderZone then the onLoadMoreRows will be called on the first render before the rows are created/loaded. As far as I can see this is the only problem because you do want the callback to be called when you scroll to the bottom, then scroll up and back to the bottom again.

To avoid this another solution is to add a specific className to the last row that is currently loaded but it creates complications with the observable as you need to observe only after the rows are loaded and update which row has the distinct className once the user loads more rows.

@DanailH
Not really... You havent explore the power of IntersectionObserver, and the deep of its concept let me show u an example.
There is no need an extra div for the solution of IntersectionObserver.
Below is the solution that I used to replace onRowScrollEnd, by observing / detecting last visible row to fetch next page, which working as expected.

   <Box sx={{ height: "35rem", width: "100%" }}>
        <DataGridPro
          apiRef={apiRef}
          pinnedColumns={lg ? { left: ["id"], right: ["actions"] } : {}}
          rows={rows}
          columns={columns}
          checkboxSelection={false}
          disableSelectionOnClick
          disableMultipleSelection
          loading={isLoading || isFetching}
          components={{
            LoadingOverlay: LinearProgress,
            Row: (props) => {
              const index = props.index;
              const lastItemIndex = rows.length - 2;
              // console.log("lastItemIndex", lastItemIndex, index);
              if (lastItemIndex === index) {
                return (
                  <div ref={lastOptionElementRef}>
                    <GridRow {...props} className="cursor-pointer"></GridRow>
                  </div>
                );
              }
              return <GridRow {...props} className="cursor-pointer"></GridRow>;
            },
          }}
        />
      </Box>

        const observer = useRef();

  const lastOptionElementRef = useCallback(
    (node) => {
      if (observer.current) observer.current.disconnect();
      observer.current = new IntersectionObserver(async (entries) => {
        if (isFetching) {
          return;
        }
        const [target] = entries;
        if (target.isIntersecting && hasNextPage) {
          fetchNextPage();
        }
      });
      if (node) observer.current.observe(node);
    },
    [fetchNextPage, hasNextPage, isFetching]
  );

btw I think I no need to update my PR #7416, which implemented by using extra div.

[github-actions]

⚠️ This issue has been closed.
If you have a similar problem, please open a new issue and provide details about your specific problem.
If you can provide additional information related to this topic that could help future readers, please feel free to leave a comment.