fix: table moves up when doing selection (#627)

src/table/components/TableWrapper.jsx

@@ -1,5 +1,5 @@
 import PropTypes from 'prop-types';
-import React, { useEffect, useRef, useCallback } from 'react';
+import React, { useRef, useCallback } from 'react';
 import Table from '@mui/material/Table';
 
 import AnnounceElements from './AnnounceElements';
@@ -16,7 +16,6 @@ import useFocusListener from '../hooks/use-focus-listener';
 import useScrollListener from '../hooks/use-scroll-listener';
 import { handleTableWrapperKeyDown } from '../utils/handle-key-press';
 import { updateFocus, handleResetFocus, getCellElement } from '../utils/handle-accessibility';
-import { handleNavigateTop } from '../utils/handle-scroll';
 
 export default function TableWrapper(props) {
   const {
@@ -73,11 +72,6 @@ export default function TableWrapper(props) {
   useFocusListener(tableWrapperRef, shouldRefocus, keyboard);
   useScrollListener(tableContainerRef, direction);
 
-  useEffect(
-    () => handleNavigateTop({ tableContainerRef, focusedCellCoord, rootElement }),
-    [tableContainerRef, focusedCellCoord, rootElement]
-  );
-
   useDidUpdateEffect(() => {
     // When nebula handles keyboard navigation and keyboard.active changes,
     // make sure to blur or focus the cell corresponding to focusedCellCoord
@@ -116,6 +110,7 @@ export default function TableWrapper(props) {
       <AnnounceElements />
       <StyledTableContainer
         ref={tableContainerRef}
+        className="sn-table-container"
         fullHeight={footerContainer || constraints.active || !paginationNeeded} // the footerContainer always wants height: 100%
         constraints={constraints}
         tabIndex={-1}

src/table/utils/__tests__/handle-key-press.spec.js

@@ -9,6 +9,7 @@ import {
 } from '../handle-key-press';
 
 import * as handleAccessibility from '../handle-accessibility';
+import * as handleScroll from '../handle-scroll';
 
 describe('handle-key-press', () => {
   describe('handleTableWrapperKeyDown', () => {
@@ -492,6 +493,7 @@ describe('handle-key-press', () => {
       paginationNeeded = true;
       jest.spyOn(handleAccessibility, 'focusSelectionToolbar').mockImplementation(() => jest.fn());
       jest.spyOn(handleAccessibility, 'announceSelectionState').mockImplementation(() => jest.fn());
+      jest.spyOn(handleScroll, 'handleNavigateTop').mockImplementation(() => jest.fn());
     });
 
     afterEach(() => jest.clearAllMocks());
@@ -503,6 +505,19 @@ describe('handle-key-press', () => {
       expect(evt.target.setAttribute).toHaveBeenCalledTimes(1);
       expect(setFocusedCellCoord).toHaveBeenCalledTimes(1);
       expect(handleAccessibility.announceSelectionState).toHaveBeenCalledTimes(1);
+      expect(handleScroll.handleNavigateTop).not.toHaveBeenCalled();
+    });
+
+    it('when press arrow up key on body cell, should prevent default behavior, remove current focus and set focus and attribute to the next cell and call handleNavigateTop', () => {
+      evt.key = 'ArrowUp';
+
+      runBodyHandleKeyPress();
+      expect(evt.preventDefault).toHaveBeenCalledTimes(1);
+      expect(evt.stopPropagation).toHaveBeenCalledTimes(1);
+      expect(evt.target.setAttribute).toHaveBeenCalledTimes(1);
+      expect(setFocusedCellCoord).toHaveBeenCalledTimes(1);
+      expect(handleAccessibility.announceSelectionState).toHaveBeenCalledTimes(1);
+      expect(handleScroll.handleNavigateTop).toHaveBeenCalledTimes(1);
     });
 
     it('when press shift + arrow down key on body cell, should prevent default behavior, remove current focus and set focus and attribute to the next cell, and select values for dimension', () => {
@@ -516,6 +531,7 @@ describe('handle-key-press', () => {
       expect(setFocusedCellCoord).toHaveBeenCalledTimes(1);
       expect(selectionDispatch).toHaveBeenCalledTimes(1);
       expect(handleAccessibility.announceSelectionState).not.toHaveBeenCalled();
+      expect(handleScroll.handleNavigateTop).not.toHaveBeenCalled();
     });
 
     it('when press shift + arrow down key on the last row cell, should prevent default behavior, remove current focus and set focus and attribute to the next cell, but not select values for dimension', () => {
@@ -529,6 +545,7 @@ describe('handle-key-press', () => {
       expect(setFocusedCellCoord).toHaveBeenCalledTimes(1);
       expect(selectionDispatch).not.toHaveBeenCalled();
       expect(handleAccessibility.announceSelectionState).toHaveBeenCalledTimes(1);
+      expect(handleScroll.handleNavigateTop).not.toHaveBeenCalled();
     });
 
     it('when press shift + arrow up key on body cell, should prevent default behavior, remove current focus and set focus and attribute to the next cell, and select values for dimension', () => {
@@ -543,6 +560,7 @@ describe('handle-key-press', () => {
       expect(setFocusedCellCoord).toHaveBeenCalledTimes(1);
       expect(selectionDispatch).toHaveBeenCalledTimes(1);
       expect(handleAccessibility.announceSelectionState).not.toHaveBeenCalled();
+      expect(handleScroll.handleNavigateTop).toHaveBeenCalledTimes(1);
     });
 
     it('when press shift + arrow up key on the second row cell, should prevent default behavior, remove current focus and set focus and attribute to the next cell, but not select values for dimension', () => {
@@ -557,6 +575,7 @@ describe('handle-key-press', () => {
       expect(setFocusedCellCoord).toHaveBeenCalledTimes(1);
       expect(selectionDispatch).not.toHaveBeenCalled();
       expect(handleAccessibility.announceSelectionState).toHaveBeenCalledTimes(1);
+      expect(handleScroll.handleNavigateTop).toHaveBeenCalledTimes(1);
     });
 
     it('when press space bar key and dimension, should select value for dimension', () => {

src/table/utils/__tests__/handle-scroll.spec.js

@@ -112,38 +112,43 @@ describe('handle-scroll', () => {
   describe('handleNavigateTop', () => {
     let rowHeight;
     let scrollTo;
-    let tableContainerRef;
-    let focusedCellCoord;
+    let cellCoord;
     let rootElement;
 
     beforeEach(() => {
       rowHeight = 100;
       scrollTo = jest.fn();
-      tableContainerRef = { current: { scrollTo } };
-      focusedCellCoord = [0, 0];
-      rootElement = {};
+      cellCoord = [0, 0];
+      rootElement = {
+        getElementsByClassName: () => [{}],
+      };
     });
 
-    it('should not do anything when ref is not setup yet', () => {
-      tableContainerRef.current = {};
-
-      handleNavigateTop({ tableContainerRef, focusedCellCoord, rootElement });
+    it('should not do anything when rootElement is not setup yet', () => {
+      handleNavigateTop(cellCoord, rootElement);
       expect(scrollTo).not.toHaveBeenCalled();
     });
 
     it('should the scrollbar is at its top when you reach the top two rows', () => {
-      focusedCellCoord = [1, 0];
+      cellCoord = [1, 0];
+      rootElement = {
+        getElementsByClassName: () => [{ scrollTo }],
+      };
 
-      handleNavigateTop({ tableContainerRef, focusedCellCoord, rootElement });
-      expect(scrollTo).toHaveBeenCalledWith({ top: 0, behavior: 'smooth' });
+      handleNavigateTop(cellCoord, rootElement);
+      expect(scrollTo).toHaveBeenCalledWith({ top: 0, behavior: 'instant' });
     });
 
     it('should scroll upwards automatically if it detects the cursor gets behind <TableHead />', () => {
       const SCROLL_TOP_IDX = 7;
-      focusedCellCoord = [8, 0];
-      tableContainerRef = { current: { scrollTo, scrollTop: SCROLL_TOP_IDX * rowHeight } };
+      cellCoord = [8, 0];
+
       rootElement = {
         getElementsByClassName: (query) => {
+          if (query === 'sn-table-container') {
+            return [{ scrollTo, scrollTop: SCROLL_TOP_IDX * rowHeight }];
+          }
+
           if (query === 'sn-table-head-cell') {
             return [{ offsetHeight: 128 }];
           }
@@ -159,12 +164,12 @@ describe('handle-scroll', () => {
         },
       };
       // targetOffsetTop = tableContainer.current.scrollTop - cell.offsetHeight - tableHead.offsetHeight;
-      // 700 - 100 - 128 = 472 => so our scrollTo function migth be called with 600
+      // 700 - 100 - 128 = 472 => so our scrollTo function is called with 472
       const targetOffsetTop = 472;
 
-      handleNavigateTop({ tableContainerRef, focusedCellCoord, rootElement });
+      handleNavigateTop(cellCoord, rootElement);
       expect(scrollTo).toHaveBeenCalledTimes(1);
-      expect(scrollTo).toHaveBeenCalledWith({ top: targetOffsetTop, behavior: 'smooth' });
+      expect(scrollTo).toHaveBeenCalledWith({ top: targetOffsetTop, behavior: 'instant' });
     });
   });
 });

src/table/utils/handle-key-press.js

@@ -1,5 +1,6 @@
 import { updateFocus, focusSelectionToolbar, getCellElement, announceSelectionState } from './handle-accessibility';
 import { SelectionActions } from './selections-utils';
+import { handleNavigateTop } from './handle-scroll';
 
 const isCtrlShift = (evt) => evt.shiftKey && (evt.ctrlKey || evt.metaKey);
 
@@ -164,6 +165,7 @@ export const bodyHandleKeyPress = ({
   switch (evt.key) {
     case 'ArrowUp':
     case 'ArrowDown': {
+      evt.key === 'ArrowUp' && handleNavigateTop([cell.rawRowIdx, cell.rawColIdx], rootElement);
       // Make sure you can't navigate to header (and totals) in selection mode
       const topAllowedRow = isSelectionMode ? firstBodyRowIdx : 0;
       const nextCell = moveFocus(evt, rootElement, cellCoord, setFocusedCellCoord, topAllowedRow);

src/table/utils/handle-scroll.js

@@ -26,27 +26,27 @@ export const handleHorizontalScroll = (evt, isRTL, memoedContainer) => {
   }
 };
 
-export const handleNavigateTop = ({ tableContainerRef, focusedCellCoord, rootElement }) => {
-  const MIN_ROW_COUNT = 2;
-
-  if (!tableContainerRef.current?.scrollTo) return;
+export const handleNavigateTop = (cellCoord, rootElement) => {
+  const tableContainer = rootElement.getElementsByClassName('sn-table-container')[0];
+  if (!tableContainer?.scrollTo) return;
 
-  if (focusedCellCoord[0] < MIN_ROW_COUNT) {
-    tableContainerRef.current.scrollTo({
+  const [x, y] = cellCoord;
+  const MIN_ROW_COUNT = 2;
+  if (x < MIN_ROW_COUNT) {
+    tableContainer.scrollTo({
       top: 0,
-      behavior: 'smooth',
+      behavior: 'instant',
     });
   } else {
-    const [x, y] = focusedCellCoord;
     const tableHead = rootElement.getElementsByClassName('sn-table-head-cell')[0];
     const rowElements = rootElement.getElementsByClassName('sn-table-row');
     const cell = rowElements[x]?.getElementsByClassName('sn-table-cell')[y];
 
-    if (cell.offsetTop - tableHead.offsetHeight - cell.offsetHeight <= tableContainerRef.current.scrollTop) {
-      const targetOffsetTop = tableContainerRef.current.scrollTop - cell.offsetHeight - tableHead.offsetHeight;
-      tableContainerRef.current.scrollTo({
+    if (cell.offsetTop - tableHead.offsetHeight < tableContainer.scrollTop) {
+      const targetOffsetTop = tableContainer.scrollTop - cell.offsetHeight - tableHead.offsetHeight;
+      tableContainer.scrollTo({
         top: Math.max(0, targetOffsetTop),
-        behavior: 'smooth',
+        behavior: 'instant',
       });
     }
   }