Skip to content
This repository was archived by the owner on Aug 8, 2024. It is now read-only.

Commit

Permalink
fix: table moves up when doing selection
Browse files Browse the repository at this point in the history
  • Loading branch information
LiKang6688 committed Aug 10, 2022
1 parent 65329b7 commit affa9f8
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 35 deletions.
9 changes: 2 additions & 7 deletions src/table/components/TableWrapper.jsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand Down
37 changes: 21 additions & 16 deletions src/table/utils/__tests__/handle-scroll.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
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 }];
}
Expand All @@ -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' });
});
});
});
2 changes: 2 additions & 0 deletions src/table/utils/handle-key-press.js
Original file line number Diff line number Diff line change
@@ -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);

Expand Down Expand Up @@ -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);
Expand Down
24 changes: 12 additions & 12 deletions src/table/utils/handle-scroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
});
}
}
Expand Down

0 comments on commit affa9f8

Please sign in to comment.