From affa9f8a71eae0bb0696ca4b88c9f1db1a0fcd2d Mon Sep 17 00:00:00 2001 From: Li Kang Date: Wed, 10 Aug 2022 15:53:17 +0200 Subject: [PATCH] fix: table moves up when doing selection --- src/table/components/TableWrapper.jsx | 9 +---- .../utils/__tests__/handle-scroll.spec.js | 37 +++++++++++-------- src/table/utils/handle-key-press.js | 2 + src/table/utils/handle-scroll.js | 24 ++++++------ 4 files changed, 37 insertions(+), 35 deletions(-) diff --git a/src/table/components/TableWrapper.jsx b/src/table/components/TableWrapper.jsx index 50297fcbc..cff167b83 100644 --- a/src/table/components/TableWrapper.jsx +++ b/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) { { 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 }); + 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 ', () => { 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 might be 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' }); }); }); }); diff --git a/src/table/utils/handle-key-press.js b/src/table/utils/handle-key-press.js index 7e3cc61ef..561e0cb4d 100644 --- a/src/table/utils/handle-key-press.js +++ b/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); diff --git a/src/table/utils/handle-scroll.js b/src/table/utils/handle-scroll.js index ed216e0bb..e316dc852 100644 --- a/src/table/utils/handle-scroll.js +++ b/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 - cell.offsetHeight <= tableContainer.scrollTop) { + const targetOffsetTop = tableContainer.scrollTop - cell.offsetHeight - tableHead.offsetHeight; + tableContainer.scrollTo({ top: Math.max(0, targetOffsetTop), - behavior: 'smooth', + behavior: 'instant', }); } }