[DataGrid] Better fix the scrollToIndexes logic (#1969)

mui · Jun 29, 2021 · 2b0daf2 · 2b0daf2
1 parent 5c11ab8
commit 2b0daf2
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 21 deletions.
diff --git a/packages/grid/_modules_/grid/hooks/features/virtualization/useGridVirtualRows.ts b/packages/grid/_modules_/grid/hooks/features/virtualization/useGridVirtualRows.ts
@@ -217,33 +217,22 @@ export const useGridVirtualRows = (apiRef: GridApiRef): void => {
         }
       }
 
-      let isRowIndexAbove = false;
-      let isRowIndexBelow = false;
-
+      // Logic copied from https://www.w3.org/TR/wai-aria-practices/examples/listbox/js/listbox.js
       if (params.rowIndex != null) {
         const elementIndex = !options.pagination
           ? params.rowIndex
           : params.rowIndex - paginationState.page * paginationState.pageSize;
 
-        const currentRowPage = elementIndex / gridState.containerSizes!.viewportPageSize;
-        const scrollPosition = currentRowPage * gridState!.viewportSizes.height;
-        const viewportHeight = gridState.viewportSizes.height;
-
-        isRowIndexAbove = windowRef.current!.scrollTop > scrollPosition;
-        isRowIndexBelow =
-          windowRef.current!.scrollTop + viewportHeight < scrollPosition + rowHeight;
-
-        if (isRowIndexAbove) {
-          scrollCoordinates.top = scrollPosition; // We put it at the top of the page
-          logger.debug(`Row is above, setting top to ${scrollCoordinates.top}`);
-        } else if (isRowIndexBelow) {
-          // We make sure the row is not half visible
-          scrollCoordinates.top = scrollPosition - viewportHeight + rowHeight;
-          logger.debug(`Row is below, setting top to ${scrollCoordinates.top}`);
+        const scrollBottom = windowRef.current!.clientHeight + windowRef.current!.scrollTop;
+        const elementBottom = rowHeight * elementIndex + rowHeight;
+        if (elementBottom > scrollBottom) {
+          scrollCoordinates.top = elementBottom - windowRef.current!.clientHeight;
+        } else if (rowHeight * elementIndex < windowRef.current!.scrollTop) {
+          scrollCoordinates.top = rowHeight * elementIndex;
         }
       }
 
-      const needScroll = !isColVisible || isRowIndexAbove || isRowIndexBelow;
+      const needScroll = !isColVisible || typeof scrollCoordinates.top !== undefined;
       if (needScroll) {
         apiRef.current.scroll(scrollCoordinates);
       }

diff --git a/packages/grid/x-grid/src/tests/rows.XGrid.test.tsx b/packages/grid/x-grid/src/tests/rows.XGrid.test.tsx
@@ -452,19 +452,44 @@ describe('<XGrid /> - Rows', () => {
       it('should scroll correctly when the given index is partially visible at the bottom', () => {
         const headerHeight = 40;
         const rowHeight = 50;
+        const offset = 10;
         const border = 1;
         render(
           <TestCaseVirtualization
             hideFooter
             headerHeight={headerHeight}
-            height={headerHeight + 4 * rowHeight + 10 + border * 2}
+            height={headerHeight + 4 * rowHeight + offset + border * 2}
             nbCols={2}
             rowHeight={rowHeight}
           />,
         );
         const gridWindow = document.querySelector('.MuiDataGrid-window')!;
         apiRef.current.scrollToIndexes({ rowIndex: 4, colIndex: 0 });
-        expect(gridWindow.scrollTop).to.equal(rowHeight);
+        expect(gridWindow.scrollTop).to.equal(rowHeight - offset);
+      });
+
+      it('should scroll correctly when the given index is partially visible at the top', () => {
+        const headerHeight = 40;
+        const rowHeight = 50;
+        const offset = 10;
+        const border = 1;
+        render(
+          <TestCaseVirtualization
+            hideFooter
+            headerHeight={headerHeight}
+            height={headerHeight + 4 * rowHeight + border + border * 2}
+            nbCols={2}
+            rowHeight={rowHeight}
+          />,
+        );
+        const gridWindow = document.querySelector('.MuiDataGrid-window')!;
+        gridWindow.scrollTop = offset;
+        apiRef.current.scrollToIndexes({ rowIndex: 2, colIndex: 0 });
+        expect(gridWindow.scrollTop).to.equal(offset);
+        apiRef.current.scrollToIndexes({ rowIndex: 1, colIndex: 0 });
+        expect(gridWindow.scrollTop).to.equal(offset);
+        apiRef.current.scrollToIndexes({ rowIndex: 0, colIndex: 0 });
+        expect(gridWindow.scrollTop).to.equal(0);
       });
     });
   });