From 55a4ba6b513dc651ba8817880fee69a149de8771 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 1 Feb 2022 12:20:08 -0800 Subject: [PATCH] [PR feedback] Rename fns to indicate multiple concerns - having a side effect in a getter feels bad, so change that to a `find` - rename use hook to indicate sorting and pagination concerns --- src/components/datagrid/utils/ref.test.ts | 40 +++++++++++------------ src/components/datagrid/utils/ref.ts | 16 ++++----- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/components/datagrid/utils/ref.test.ts b/src/components/datagrid/utils/ref.test.ts index abb7957297a..bf49b5fcd87 100644 --- a/src/components/datagrid/utils/ref.test.ts +++ b/src/components/datagrid/utils/ref.test.ts @@ -7,7 +7,7 @@ */ import { testCustomHook } from '../../../test/test_custom_hook.test_helper'; -import { useCellLocationCheck, useVisibleRowIndex } from './ref'; +import { useCellLocationCheck, useSortPageCheck } from './ref'; // see ref.spec.tsx for E2E useImperativeGridRef tests @@ -39,17 +39,17 @@ describe('useCellLocationCheck', () => { }); }); -describe('useVisibleRowIndex', () => { +describe('useSortPageCheck', () => { describe('if the grid is not sorted or paginated', () => { const pagination = undefined; const sortedRowMap: number[] = []; it('returns the passed rowIndex as-is', () => { const { - return: { getVisibleRowIndex }, - } = testCustomHook(() => useVisibleRowIndex(pagination, sortedRowMap)); + return: { findVisibleRowIndex }, + } = testCustomHook(() => useSortPageCheck(pagination, sortedRowMap)); - expect(getVisibleRowIndex(5)).toEqual(5); + expect(findVisibleRowIndex(5)).toEqual(5); }); }); @@ -59,14 +59,14 @@ describe('useVisibleRowIndex', () => { it('returns the visibleRowIndex of the passed rowIndex (which is the index of the sortedRowMap)', () => { const { - return: { getVisibleRowIndex }, - } = testCustomHook(() => useVisibleRowIndex(pagination, sortedRowMap)); - - expect(getVisibleRowIndex(0)).toEqual(4); - expect(getVisibleRowIndex(1)).toEqual(2); - expect(getVisibleRowIndex(2)).toEqual(3); - expect(getVisibleRowIndex(3)).toEqual(0); - expect(getVisibleRowIndex(4)).toEqual(1); + return: { findVisibleRowIndex }, + } = testCustomHook(() => useSortPageCheck(pagination, sortedRowMap)); + + expect(findVisibleRowIndex(0)).toEqual(4); + expect(findVisibleRowIndex(1)).toEqual(2); + expect(findVisibleRowIndex(2)).toEqual(3); + expect(findVisibleRowIndex(3)).toEqual(0); + expect(findVisibleRowIndex(4)).toEqual(1); }); }); @@ -83,22 +83,22 @@ describe('useVisibleRowIndex', () => { it('calculates what page the row should be on, paginates to that page, and returns the index of the row on that page', () => { const { - return: { getVisibleRowIndex }, - } = testCustomHook(() => useVisibleRowIndex(pagination, sortedRowMap)); + return: { findVisibleRowIndex }, + } = testCustomHook(() => useSortPageCheck(pagination, sortedRowMap)); - expect(getVisibleRowIndex(20)).toEqual(0); // First item on 2nd page + expect(findVisibleRowIndex(20)).toEqual(0); // First item on 2nd page expect(pagination.onChangePage).toHaveBeenLastCalledWith(1); - expect(getVisibleRowIndex(75)).toEqual(15); // 16th item on 4th page + expect(findVisibleRowIndex(75)).toEqual(15); // 16th item on 4th page expect(pagination.onChangePage).toHaveBeenLastCalledWith(3); }); it('does not paginate if the user is already on the correct page', () => { const { - return: { getVisibleRowIndex }, - } = testCustomHook(() => useVisibleRowIndex(pagination, sortedRowMap)); + return: { findVisibleRowIndex }, + } = testCustomHook(() => useSortPageCheck(pagination, sortedRowMap)); - expect(getVisibleRowIndex(5)).toEqual(5); + expect(findVisibleRowIndex(5)).toEqual(5); expect(pagination.onChangePage).not.toHaveBeenCalled(); }); }); diff --git a/src/components/datagrid/utils/ref.ts b/src/components/datagrid/utils/ref.ts index afc488292ae..88aebb8eb8d 100644 --- a/src/components/datagrid/utils/ref.ts +++ b/src/components/datagrid/utils/ref.ts @@ -38,7 +38,7 @@ export const useImperativeGridRef = ({ }: Dependencies) => { // Cell location helpers const { checkCellExists } = useCellLocationCheck(rowCount, visibleColCount); - const { getVisibleRowIndex } = useVisibleRowIndex(pagination, sortedRowMap); + const { findVisibleRowIndex } = useSortPageCheck(pagination, sortedRowMap); // Focus APIs const { setFocusedCell: _setFocusedCell } = focusContext; // eslint complains about the dependency array otherwise @@ -50,10 +50,10 @@ export const useImperativeGridRef = ({ const setFocusedCell = useCallback( ({ rowIndex, colIndex }) => { checkCellExists({ rowIndex, colIndex }); - const visibleRowIndex = getVisibleRowIndex(rowIndex); + const visibleRowIndex = findVisibleRowIndex(rowIndex); _setFocusedCell([colIndex, visibleRowIndex]); // Transmog args from obj to array }, - [_setFocusedCell, checkCellExists, getVisibleRowIndex] + [_setFocusedCell, checkCellExists, findVisibleRowIndex] ); // Popover APIs @@ -69,10 +69,10 @@ export const useImperativeGridRef = ({ const openCellPopover = useCallback( ({ rowIndex, colIndex }) => { checkCellExists({ rowIndex, colIndex }); - const visibleRowIndex = getVisibleRowIndex(rowIndex); + const visibleRowIndex = findVisibleRowIndex(rowIndex); _openCellPopover({ rowIndex: visibleRowIndex, colIndex }); }, - [_openCellPopover, checkCellExists, getVisibleRowIndex] + [_openCellPopover, checkCellExists, findVisibleRowIndex] ); // Set the ref APIs @@ -123,11 +123,11 @@ export const useCellLocationCheck = (rowCount: number, colCount: number) => { * the row is not on the current page, the grid should automatically handle * paginating to that row. */ -export const useVisibleRowIndex = ( +export const useSortPageCheck = ( pagination: EuiDataGridProps['pagination'], sortedRowMap: DataGridSortingContextShape['sortedRowMap'] ) => { - const getVisibleRowIndex = useCallback( + const findVisibleRowIndex = useCallback( (rowIndex: number): number => { // Account for sorting const visibleRowIndex = sortedRowMap.length @@ -150,5 +150,5 @@ export const useVisibleRowIndex = ( [pagination, sortedRowMap] ); - return { getVisibleRowIndex }; + return { findVisibleRowIndex }; };