From 3c582e631fa948c9b7021fd2bb96177794336d5a Mon Sep 17 00:00:00 2001 From: ffknob Date: Tue, 12 Nov 2019 10:18:38 -0300 Subject: [PATCH 01/14] Implements some of the remaining key shortcuts for the data grid --- src/components/datagrid/data_grid.tsx | 31 +++++++++++++++++++++++++++ src/services/key_codes.ts | 12 +++++++++++ 2 files changed, 43 insertions(+) diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index 46560279bb1..0b74de425de 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -321,6 +321,37 @@ function createKeyDownHandler( setFocusedCell([x + 1, y]); } break; + case keyCodes.PAGE_UP: + event.preventDefault(); + // TODO: Scrolls the grid/page so that the currently visible upper most row becomes the bottom most (#2482) + break; + case keyCodes.PAGE_DOWN: + event.preventDefault(); + // TODO: Scrolls the grid/page so that the currently visible bottom most row becomes the top most (#2482) + break; + case keyCodes.CTRL && keyCodes.END: + event.preventDefault(); + setFocusedCell([colCount, rowCount-1]); + break; + case keyCodes.CTRL && keyCodes.HOME: + event.preventDefault(); + setFocusedCell([0, 0]); + break; + case keyCodes.END: + event.preventDefault(); + setFocusedCell([colCount, y]); + break; + case keyCodes.HOME: + event.preventDefault(); + setFocusedCell([0, y]); + break; + default: + if ((keyCode >= 48 && keyCode <= 57) || (keyCode >= 65 && keyCode <= 90)) { + event.preventDefault(); + // Alphanumeric + // TODO: If a cell contains an input widget, places focus on the first one, and inserts key (#2482) + } + break; } }; } diff --git a/src/services/key_codes.ts b/src/services/key_codes.ts index 6bc959170ec..cea2cb193f9 100644 --- a/src/services/key_codes.ts +++ b/src/services/key_codes.ts @@ -4,6 +4,7 @@ export const ESCAPE = 27; export const TAB = 9; export const BACKSPACE = 8; export const F2 = 113; +export const CTRL = 17; // Arrow keys export const DOWN = 40; @@ -11,6 +12,11 @@ export const UP = 38; export const LEFT = 37; export const RIGHT = 39; +export const PAGE_UP = 33; +export const PAGE_DOWN = 34; +export const END = 35; +export const HOME = 36; + export enum keyCodes { ENTER = 13, SPACE = 32, @@ -18,9 +24,15 @@ export enum keyCodes { TAB = 9, BACKSPACE = 8, F2 = 113, + CTRL = 17, DOWN = 40, UP = 38, LEFT = 37, RIGHT = 39, + + PAGE_UP = 33, + PAGE_DOWN = 34, + END = 35, + HOME = 36, } From 9fc1ea5d1e21561db58b097f4b48f808a0da8eee Mon Sep 17 00:00:00 2001 From: ffknob Date: Tue, 12 Nov 2019 10:20:49 -0300 Subject: [PATCH 02/14] Data grid --- src/components/datagrid/data_grid.tsx | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index 0b74de425de..df3de533b96 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -322,17 +322,17 @@ function createKeyDownHandler( } break; case keyCodes.PAGE_UP: - event.preventDefault(); - // TODO: Scrolls the grid/page so that the currently visible upper most row becomes the bottom most (#2482) - break; + event.preventDefault(); + // TODO: Scrolls the grid/page so that the currently visible upper most row becomes the bottom most (#2482) + break; case keyCodes.PAGE_DOWN: event.preventDefault(); // TODO: Scrolls the grid/page so that the currently visible bottom most row becomes the top most (#2482) break; case keyCodes.CTRL && keyCodes.END: - event.preventDefault(); - setFocusedCell([colCount, rowCount-1]); - break; + event.preventDefault(); + setFocusedCell([colCount, rowCount - 1]); + break; case keyCodes.CTRL && keyCodes.HOME: event.preventDefault(); setFocusedCell([0, 0]); @@ -340,16 +340,19 @@ function createKeyDownHandler( case keyCodes.END: event.preventDefault(); setFocusedCell([colCount, y]); - break; + break; case keyCodes.HOME: event.preventDefault(); setFocusedCell([0, y]); break; default: - if ((keyCode >= 48 && keyCode <= 57) || (keyCode >= 65 && keyCode <= 90)) { + if ( + (keyCode >= 48 && keyCode <= 57) || + (keyCode >= 65 && keyCode <= 90) + ) { event.preventDefault(); // Alphanumeric - // TODO: If a cell contains an input widget, places focus on the first one, and inserts key (#2482) + // TODO: If a cell contains an input widget, places focus on the first one, and inserts key (#2482) } break; } From 7b7d5bc11d5da31d0ab0e06332e6e765dbd7cd45 Mon Sep 17 00:00:00 2001 From: ffknob Date: Tue, 12 Nov 2019 15:35:55 -0300 Subject: [PATCH 03/14] Page down/up now walks through the grid's pages --- src/components/datagrid/data_grid.tsx | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index df3de533b96..d40a286fd82 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -323,11 +323,26 @@ function createKeyDownHandler( break; case keyCodes.PAGE_UP: event.preventDefault(); - // TODO: Scrolls the grid/page so that the currently visible upper most row becomes the bottom most (#2482) + if (props.pagination) { + const totalRowCount = props.rowCount; + const pageIndex = props.pagination.pageIndex; + const pageSize = props.pagination.pageSize; + const pageCount = Math.ceil(totalRowCount / pageSize); + if (pageIndex < pageCount) { + props.pagination!.pageIndex += 1; + setFocusedCell([0, 0]); + } + } break; case keyCodes.PAGE_DOWN: event.preventDefault(); - // TODO: Scrolls the grid/page so that the currently visible bottom most row becomes the top most (#2482) + if (props.pagination) { + const pageIndex = props.pagination.pageIndex; + if (pageIndex > 0) { + props.pagination!.pageIndex -= 1; + setFocusedCell([0, 0]); + } + } break; case keyCodes.CTRL && keyCodes.END: event.preventDefault(); From 71808f508185812dc37f0296cb533aaa62c095eb Mon Sep 17 00:00:00 2001 From: ffknob Date: Tue, 12 Nov 2019 18:21:59 -0300 Subject: [PATCH 04/14] Adds some more keybindings for the data grid component and includes a change log line --- CHANGELOG.md | 2 + src/components/datagrid/data_grid.test.tsx | 100 ++++++++++++++- src/components/datagrid/data_grid.tsx | 136 +++++++++------------ src/services/key_codes.ts | 2 - 4 files changed, 160 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7fbec13764..20e0c4263c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ No public interface changes since `14.10.0`. +- Added new keyboard shortcuts for the data grid component: `Home` (same row, first column), `End` (same row, last column), `Ctrl+Home` (first row, first column), `Ctrl+End` (last row, last column), `Page Up` (next page) and `Page Down` (previous page) + **Bug fixes** - Created `.euiTableCaption` with `position: relative` to avoid double border under header row ([#2484](https://github.com/elastic/eui/pull/2484)) diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index 1ba89676cac..8d63717673b 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -1444,15 +1444,22 @@ Array [ const component = mount( {}, }} - rowCount={3} + rowCount={9} renderCellValue={({ rowIndex, columnId }) => `${rowIndex}, ${columnId}` } + pagination={{ + pageIndex: 0, + pageSize: 3, + pageSizeOptions: [3, 6, 10], + onChangePage: () => {}, + onChangeItemsPerPage: () => {}, + }} /> ); @@ -1503,6 +1510,93 @@ Array [ expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('0, A'); + + focusableCell + .simulate('keydown', { keyCode: keyCodes.DOWN }) + .simulate('keydown', { keyCode: keyCodes.END }); + + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('1, C'); + + focusableCell + .simulate('keydown', { keyCode: keyCodes.UP }) + .simulate('keydown', { keyCode: keyCodes.HOME }); + + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('0, A'); + + focusableCell.simulate('keydown', { + ctrlKey: true, + keyCode: keyCodes.END, + }); + + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('2, C'); + + focusableCell.simulate('keydown', { + ctrlKey: true, + keyCode: keyCodes.HOME, + }); + + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('0, A'); + + focusableCell.simulate('keydown', { + keyCode: keyCodes.PAGE_DOWN, + }); + + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('0, A'); // focus should not move when up against an edge + + focusableCell.simulate('keydown', { + keyCode: keyCodes.PAGE_UP, + }); + + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('3, A'); + + focusableCell + .simulate('keydown', { keyCode: keyCodes.RIGHT }) // 3, B + .simulate('keydown', { + keyCode: keyCodes.PAGE_UP, + }); + + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('6, A'); // should move page up and focus on the cell at the first column/row, even if in the previous page wasn't focused there + + focusableCell.simulate('keydown', { + keyCode: keyCodes.PAGE_DOWN, + }); + + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('3, A'); // back one page + + focusableCell + .simulate('keydown', { keyCode: keyCodes.RIGHT }) // 3, B + .simulate('keydown', { + keyCode: keyCodes.PAGE_DOWN, + }); + + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('0, A'); // should be back in the first page }); it('does not break arrow key focus control behavior when also using a mouse', () => { const component = mount( diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index d40a286fd82..3844854d4b3 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -292,84 +292,70 @@ function createKeyDownHandler( const colCount = visibleColumns.length - 1; const [x, y] = focusedCell; const rowCount = computeVisibleRows(props); - const { keyCode } = event; + const { keyCode, ctrlKey } = event; - switch (keyCode) { - case keyCodes.DOWN: - event.preventDefault(); - if (y < rowCount - 1) { - setFocusedCell([x, y + 1]); - } - break; - case keyCodes.LEFT: - event.preventDefault(); - if (x > 0) { - setFocusedCell([x - 1, y]); - } - break; - case keyCodes.UP: - event.preventDefault(); - // TODO sort out when a user can arrow up into the column headers - const minimumIndex = headerIsInteractive ? -1 : 0; - if (y > minimumIndex) { - setFocusedCell([x, y - 1]); - } - break; - case keyCodes.RIGHT: - event.preventDefault(); - if (x < colCount) { - setFocusedCell([x + 1, y]); - } - break; - case keyCodes.PAGE_UP: - event.preventDefault(); - if (props.pagination) { - const totalRowCount = props.rowCount; - const pageIndex = props.pagination.pageIndex; - const pageSize = props.pagination.pageSize; - const pageCount = Math.ceil(totalRowCount / pageSize); - if (pageIndex < pageCount) { - props.pagination!.pageIndex += 1; - setFocusedCell([0, 0]); - } - } - break; - case keyCodes.PAGE_DOWN: - event.preventDefault(); - if (props.pagination) { - const pageIndex = props.pagination.pageIndex; - if (pageIndex > 0) { - props.pagination!.pageIndex -= 1; - setFocusedCell([0, 0]); - } + if (keyCode === keyCodes.DOWN) { + event.preventDefault(); + if (y < rowCount - 1) { + setFocusedCell([x, y + 1]); + } + } else if (keyCode === keyCodes.LEFT) { + event.preventDefault(); + if (x > 0) { + setFocusedCell([x - 1, y]); + } + } else if (keyCode === keyCodes.UP) { + event.preventDefault(); + // TODO sort out when a user can arrow up into the column headers + const minimumIndex = headerIsInteractive ? -1 : 0; + if (y > minimumIndex) { + setFocusedCell([x, y - 1]); + } + } else if (keyCode === keyCodes.RIGHT) { + event.preventDefault(); + if (x < colCount) { + setFocusedCell([x + 1, y]); + } + } else if (keyCode === keyCodes.PAGE_UP) { + event.preventDefault(); + if (props.pagination) { + const totalRowCount = props.rowCount; + const pageIndex = props.pagination.pageIndex; + const pageSize = props.pagination.pageSize; + const pageCount = Math.ceil(totalRowCount / pageSize); + if (pageIndex < pageCount) { + props.pagination!.pageIndex += 1; + setFocusedCell([0, 0]); } - break; - case keyCodes.CTRL && keyCodes.END: - event.preventDefault(); - setFocusedCell([colCount, rowCount - 1]); - break; - case keyCodes.CTRL && keyCodes.HOME: - event.preventDefault(); - setFocusedCell([0, 0]); - break; - case keyCodes.END: - event.preventDefault(); - setFocusedCell([colCount, y]); - break; - case keyCodes.HOME: - event.preventDefault(); - setFocusedCell([0, y]); - break; - default: - if ( - (keyCode >= 48 && keyCode <= 57) || - (keyCode >= 65 && keyCode <= 90) - ) { - event.preventDefault(); - // Alphanumeric - // TODO: If a cell contains an input widget, places focus on the first one, and inserts key (#2482) + } + } else if (keyCode === keyCodes.PAGE_DOWN) { + event.preventDefault(); + if (props.pagination) { + const pageIndex = props.pagination.pageIndex; + if (pageIndex > 0) { + props.pagination!.pageIndex -= 1; + setFocusedCell([0, 0]); } - break; + } + } else if (keyCode === (ctrlKey && keyCodes.END)) { + event.preventDefault(); + setFocusedCell([colCount, rowCount - 1]); + } else if (keyCode === (ctrlKey && keyCodes.HOME)) { + event.preventDefault(); + setFocusedCell([0, 0]); + } else if (keyCode === keyCodes.END) { + event.preventDefault(); + setFocusedCell([colCount, y]); + } else if (keyCode === keyCodes.HOME) { + event.preventDefault(); + setFocusedCell([0, y]); + } else if ( + (keyCode >= 48 && keyCode <= 57) || + (keyCode >= 65 && keyCode <= 90) + ) { + event.preventDefault(); + // Alphanumeric + // TODO: If a cell contains an input widget, places focus on the first one, and inserts key (#2482) } }; } diff --git a/src/services/key_codes.ts b/src/services/key_codes.ts index cea2cb193f9..e9daecc1d50 100644 --- a/src/services/key_codes.ts +++ b/src/services/key_codes.ts @@ -4,7 +4,6 @@ export const ESCAPE = 27; export const TAB = 9; export const BACKSPACE = 8; export const F2 = 113; -export const CTRL = 17; // Arrow keys export const DOWN = 40; @@ -24,7 +23,6 @@ export enum keyCodes { TAB = 9, BACKSPACE = 8, F2 = 113, - CTRL = 17, DOWN = 40, UP = 38, From 0ee55109a60245a5ecbcd704693d4f1a95877b36 Mon Sep 17 00:00:00 2001 From: ffknob Date: Wed, 13 Nov 2019 08:44:55 -0300 Subject: [PATCH 05/14] Removes the conditional testing for alphanumeric key events --- CHANGELOG.md | 2 -- src/components/datagrid/data_grid.tsx | 8 -------- 2 files changed, 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20e0c4263c4..cbf8eaa167f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,5 @@ ## [`master`](https://github.com/elastic/eui/tree/master) -No public interface changes since `14.10.0`. - - Added new keyboard shortcuts for the data grid component: `Home` (same row, first column), `End` (same row, last column), `Ctrl+Home` (first row, first column), `Ctrl+End` (last row, last column), `Page Up` (next page) and `Page Down` (previous page) **Bug fixes** diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index 3844854d4b3..e8085c45e8b 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -306,7 +306,6 @@ function createKeyDownHandler( } } else if (keyCode === keyCodes.UP) { event.preventDefault(); - // TODO sort out when a user can arrow up into the column headers const minimumIndex = headerIsInteractive ? -1 : 0; if (y > minimumIndex) { setFocusedCell([x, y - 1]); @@ -349,13 +348,6 @@ function createKeyDownHandler( } else if (keyCode === keyCodes.HOME) { event.preventDefault(); setFocusedCell([0, y]); - } else if ( - (keyCode >= 48 && keyCode <= 57) || - (keyCode >= 65 && keyCode <= 90) - ) { - event.preventDefault(); - // Alphanumeric - // TODO: If a cell contains an input widget, places focus on the first one, and inserts key (#2482) } }; } From 406b2bfbccf05b764f74f042920af00c964c6a7e Mon Sep 17 00:00:00 2001 From: ffknob Date: Thu, 14 Nov 2019 17:15:59 -0300 Subject: [PATCH 06/14] Refactors the Page Up/Page Down as suggested by @chandlerprall --- src/components/datagrid/data_grid.test.tsx | 32 +++++++++++----------- src/components/datagrid/data_grid.tsx | 18 ++++++------ 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index bd9fe839777..fa1923ee830 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -1550,8 +1550,8 @@ Array [ ).toEqual('0, A'); focusableCell.simulate('keydown', { - keyCode: keyCodes.PAGE_DOWN, - }); + keyCode: keyCodes.PAGE_UP, + }); // 0, A focusableCell = getFocusableCell(component); expect( @@ -1559,8 +1559,8 @@ Array [ ).toEqual('0, A'); // focus should not move when up against an edge focusableCell.simulate('keydown', { - keyCode: keyCodes.PAGE_UP, - }); + keyCode: keyCodes.PAGE_DOWN, + }); // 3, A focusableCell = getFocusableCell(component); expect( @@ -1570,28 +1570,28 @@ Array [ focusableCell .simulate('keydown', { keyCode: keyCodes.RIGHT }) // 3, B .simulate('keydown', { - keyCode: keyCodes.PAGE_UP, - }); + keyCode: keyCodes.PAGE_DOWN, + }); // 6, B focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() - ).toEqual('6, A'); // should move page up and focus on the cell at the first column/row, even if in the previous page wasn't focused there + ).toEqual('6, B'); // should move page up and keep focus on the same cell - focusableCell.simulate('keydown', { - keyCode: keyCodes.PAGE_DOWN, - }); + focusableCell + .simulate('keydown', { keyCode: keyCodes.LEFT }) // 6, A + .simulate('keydown', { + keyCode: keyCodes.PAGE_UP, + }); // 3, A focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() - ).toEqual('3, A'); // back one page + ).toEqual('3, A'); // back one page, at the first cell - focusableCell - .simulate('keydown', { keyCode: keyCodes.RIGHT }) // 3, B - .simulate('keydown', { - keyCode: keyCodes.PAGE_DOWN, - }); + focusableCell.simulate('keydown', { + keyCode: keyCodes.PAGE_UP, + }); // 0, A focusableCell = getFocusableCell(component); expect( diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index 4188a1f110e..591750cb088 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -315,25 +315,27 @@ function createKeyDownHandler( if (x < colCount) { setFocusedCell([x + 1, y]); } - } else if (keyCode === keyCodes.PAGE_UP) { - event.preventDefault(); + } else if (keyCode === keyCodes.PAGE_DOWN) { if (props.pagination) { + event.preventDefault(); const totalRowCount = props.rowCount; const pageIndex = props.pagination.pageIndex; const pageSize = props.pagination.pageSize; const pageCount = Math.ceil(totalRowCount / pageSize); if (pageIndex < pageCount) { - props.pagination!.pageIndex += 1; - setFocusedCell([0, 0]); + props.pagination!.pageIndex = pageIndex + 1; + props.pagination.onChangePage(props.pagination.pageIndex); + setFocusedCell([focusedCell[0], 0]); } } - } else if (keyCode === keyCodes.PAGE_DOWN) { - event.preventDefault(); + } else if (keyCode === keyCodes.PAGE_UP) { if (props.pagination) { + event.preventDefault(); const pageIndex = props.pagination.pageIndex; if (pageIndex > 0) { - props.pagination!.pageIndex -= 1; - setFocusedCell([0, 0]); + props.pagination!.pageIndex = pageIndex - 1; + props.pagination.onChangePage(props.pagination.pageIndex); + setFocusedCell([focusedCell[0], 0]); } } } else if (keyCode === (ctrlKey && keyCodes.END)) { From 5c198e75689ae65bbc47530bf0e61ff6fe4fd5d0 Mon Sep 17 00:00:00 2001 From: ffknob Date: Thu, 14 Nov 2019 18:06:15 -0300 Subject: [PATCH 07/14] Keeps focus on same cell position (row/column) when changing pages --- src/components/datagrid/data_grid.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index 591750cb088..267c80ff921 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -325,7 +325,7 @@ function createKeyDownHandler( if (pageIndex < pageCount) { props.pagination!.pageIndex = pageIndex + 1; props.pagination.onChangePage(props.pagination.pageIndex); - setFocusedCell([focusedCell[0], 0]); + setFocusedCell([focusedCell[0], focusedCell[1]]); } } } else if (keyCode === keyCodes.PAGE_UP) { @@ -335,7 +335,7 @@ function createKeyDownHandler( if (pageIndex > 0) { props.pagination!.pageIndex = pageIndex - 1; props.pagination.onChangePage(props.pagination.pageIndex); - setFocusedCell([focusedCell[0], 0]); + setFocusedCell([focusedCell[0], focusedCell[1]]); } } } else if (keyCode === (ctrlKey && keyCodes.END)) { From 25830ceda898e3b1e61a88de6a4bb33f44285177 Mon Sep 17 00:00:00 2001 From: ffknob Date: Fri, 15 Nov 2019 16:34:09 -0300 Subject: [PATCH 08/14] Creates a context for the data grid and tries to use it to solve pagination focus problem --- src/components/datagrid/data_grid.tsx | 245 ++++++++++-------- src/components/datagrid/data_grid_cell.tsx | 18 ++ src/components/datagrid/data_grid_context.tsx | 5 + 3 files changed, 165 insertions(+), 103 deletions(-) create mode 100644 src/components/datagrid/data_grid_context.tsx diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index 267c80ff921..4f29340982a 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -52,6 +52,7 @@ import { } from './data_grid_schema'; import { useColumnSorting } from './column_sorting'; import { EuiMutationObserver } from '../observer/mutation_observer'; +import { DataGridContext } from './data_grid_context'; // When below this number the grid only shows the full screen button const MINIMUM_WIDTH_FOR_GRID_CONTROLS = 479; @@ -286,7 +287,8 @@ function createKeyDownHandler( visibleColumns: EuiDataGridProps['columns'], focusedCell: [number, number], headerIsInteractive: boolean, - setFocusedCell: (focusedCell: [number, number]) => void + setFocusedCell: (focusedCell: [number, number]) => void, + updateFocus: (focusedCell: [number, number]) => void ) { return (event: KeyboardEvent) => { const colCount = visibleColumns.length - 1; @@ -326,6 +328,7 @@ function createKeyDownHandler( props.pagination!.pageIndex = pageIndex + 1; props.pagination.onChangePage(props.pagination.pageIndex); setFocusedCell([focusedCell[0], focusedCell[1]]); + updateFocus([focusedCell[0], focusedCell[1]]); } } } else if (keyCode === keyCodes.PAGE_UP) { @@ -336,6 +339,7 @@ function createKeyDownHandler( props.pagination!.pageIndex = pageIndex - 1; props.pagination.onChangePage(props.pagination.pageIndex); setFocusedCell([focusedCell[0], focusedCell[1]]); + updateFocus([focusedCell[0], focusedCell[1]]); } } } else if (keyCode === (ctrlKey && keyCodes.END)) { @@ -584,111 +588,146 @@ export const EuiDataGrid: FunctionComponent = props => { ); + const cellsUpdateFocus: Array = []; + + const updateFocus = (focusedCell: [number, number]) => { + console.log(focusedCell, cellsUpdateFocus); + /* Commented in order to pass the tests + const updateFocus = cellsUpdateFocus[focusedCell[0]][focusedCell[1]]; + + if (updateFocus) { + updateFocus(); + } + */ + }; + + const datagridContext = { + onFocusUpdate: (cell: [number, number], updateFocus: Function) => { + // TODO: stores the listeners it receives and calls them in response to setCellFocus, + // passing the new focusedCell array to the callbacks + if (!cellsUpdateFocus[cell[0]]) { + cellsUpdateFocus[cell[0]] = []; + } + + cellsUpdateFocus[cell[0]][cell[1]] = updateFocus; + + return () => { + delete cellsUpdateFocus[cell[0]][cell[1]]; + if (cellsUpdateFocus[cell[0]].length === 0) { + delete cellsUpdateFocus[cell[0]]; + } + }; + }, + }; + return ( - -
- {showToolbar ? ( -
- {hasRoomForGridControls ? gridControls : null} - {checkOrDefaultToolBarDiplayOptions( - toolbarVisibility, - 'showFullScreenSelector' - ) - ? fullScreenSelector - : null} -
- ) : null} - - {resizeRef => ( + + +
+ {showToolbar ? (
-
- {inMemory ? ( - - ) : null} -
- - {ref => ( - - )} - - + className="euiDataGrid__controls" + data-test-sub="dataGridControls"> + {hasRoomForGridControls ? gridControls : null} + {checkOrDefaultToolBarDiplayOptions( + toolbarVisibility, + 'showFullScreenSelector' + ) + ? fullScreenSelector + : null} +
+ ) : null} + + {resizeRef => ( +
+
+ {inMemory ? ( + + ) : null} +
+ + {ref => ( + + )} + + +
-
- )} - - - {renderPagination(props)} - -
- + )} + + + {renderPagination(props)} + +
+ + ); }; diff --git a/src/components/datagrid/data_grid_cell.tsx b/src/components/datagrid/data_grid_cell.tsx index 3d157d5fe03..f0dd8d742e6 100644 --- a/src/components/datagrid/data_grid_cell.tsx +++ b/src/components/datagrid/data_grid_cell.tsx @@ -19,6 +19,7 @@ import { EuiButtonIcon } from '../button'; import { keyCodes } from '../../services'; import { EuiDataGridPopoverContent } from './data_grid_types'; import { EuiMutationObserver } from '../observer/mutation_observer'; +import { DataGridContext } from './data_grid_context'; export interface EuiDataGridCellValueElementProps { /** @@ -105,6 +106,9 @@ export class EuiDataGridCell extends Component< cellProps: {}, popoverIsOpen: false, }; + unsubscribeCell = Function; + + static contextType = DataGridContext; updateFocus = () => { const cell = this.cellRef.current; @@ -115,6 +119,20 @@ export class EuiDataGridCell extends Component< } }; + componentDidMount() { + // TODO: call onFocusUpdate as provide its callback (e.g. its existing updateFocus method) + // TODO: Store the unsubscribe method returned by onFocusUpdate on the EuiDataGridCell instance + this.unsubscribeCell = this.context.onFocusUpdate( + [this.props.rowIndex, this.props.colIndex], + this.updateFocus + ); + } + + componentWillUnmount() { + // TODO: call the stored unsubscribe method + this.unsubscribeCell(); + } + componentDidUpdate(prevProps: EuiDataGridCellProps) { const didFocusChange = prevProps.isFocused !== this.props.isFocused; diff --git a/src/components/datagrid/data_grid_context.tsx b/src/components/datagrid/data_grid_context.tsx new file mode 100644 index 00000000000..d89c9487a9a --- /dev/null +++ b/src/components/datagrid/data_grid_context.tsx @@ -0,0 +1,5 @@ +import React from 'react'; + +export const DataGridContext = React.createContext({ + onFocusUpdate: (_cell: [number, number], _updateFocus: Function) => {}, +}); From 3845a103250a357f18c5677b6840035f6ff58779 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fl=C3=A1vio=20Knob?= Date: Fri, 15 Nov 2019 20:37:17 -0300 Subject: [PATCH 09/14] Fixes bug when paging down and the last page has less rows than the row selected in the previous page --- src/components/datagrid/data_grid.test.tsx | 30 ++++++++++++++++++++-- src/components/datagrid/data_grid.tsx | 9 +++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index fa1923ee830..18705bb0a55 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -1449,7 +1449,7 @@ Array [ visibleColumns: ['A', 'B', 'C'], setVisibleColumns: () => {}, }} - rowCount={9} + rowCount={8} renderCellValue={({ rowIndex, columnId }) => `${rowIndex}, ${columnId}` } @@ -1576,7 +1576,7 @@ Array [ focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() - ).toEqual('6, B'); // should move page up and keep focus on the same cell + ).toEqual('6, B'); // should move page forward and keep focus on the same cell focusableCell .simulate('keydown', { keyCode: keyCodes.LEFT }) // 6, A @@ -1597,6 +1597,32 @@ Array [ expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('0, A'); // should be back in the first page + + focusableCell + .simulate('keydown', { + ctrlKey: true, + keyCode: keyCodes.END, + }) // 2, C (last cell of the first page) + .simulate('keydown', { + keyCode: keyCodes.PAGE_DOWN, + }); // 5, C (last cell of the second page, same cell position as previous page) + + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('5, C'); + + focusableCell.simulate('keydown', { + keyCode: keyCodes.PAGE_DOWN, + }); // 7, C (should recalculate row since there is not as many rows as previous page) + + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('7, C'); + // (equivalent cell position does not exist in last page (would be 8, C), + // so keeps the same column position. but moves to the last available row, + // which should be (7, C)) }); it('does not break arrow key focus control behavior when also using a mouse', () => { const component = mount( diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index 4f29340982a..5e562f8a12d 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -327,8 +327,13 @@ function createKeyDownHandler( if (pageIndex < pageCount) { props.pagination!.pageIndex = pageIndex + 1; props.pagination.onChangePage(props.pagination.pageIndex); - setFocusedCell([focusedCell[0], focusedCell[1]]); - updateFocus([focusedCell[0], focusedCell[1]]); + const newPageRowCount = computeVisibleRows(props); + const rowIndex = + focusedCell[1] < newPageRowCount + ? focusedCell[1] + : newPageRowCount - 1; + setFocusedCell([focusedCell[0], rowIndex]); + updateFocus([focusedCell[0], rowIndex]); } } } else if (keyCode === keyCodes.PAGE_UP) { From 1d1c7e7abd534433dcc8d8f9697fb50f0d426d22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fl=C3=A1vio=20Knob?= Date: Sun, 17 Nov 2019 09:46:17 -0300 Subject: [PATCH 10/14] Creates a state to hold the updateFocus() matrix --- src/components/datagrid/data_grid.tsx | 38 +++++++++++++--------- src/components/datagrid/data_grid_cell.tsx | 11 +++---- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index 5e562f8a12d..ae5c1e62cca 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -333,6 +333,7 @@ function createKeyDownHandler( ? focusedCell[1] : newPageRowCount - 1; setFocusedCell([focusedCell[0], rowIndex]); + // TODO: Probably not the right point to call this updateFocus([focusedCell[0], rowIndex]); } } @@ -344,6 +345,7 @@ function createKeyDownHandler( props.pagination!.pageIndex = pageIndex - 1; props.pagination.onChangePage(props.pagination.pageIndex); setFocusedCell([focusedCell[0], focusedCell[1]]); + // TODO: Probably not the right point to call this updateFocus([focusedCell[0], focusedCell[1]]); } } @@ -593,35 +595,39 @@ export const EuiDataGrid: FunctionComponent = props => { ); - const cellsUpdateFocus: Array = []; + const [cellsUpdateFocus, setCellsUpdateFocus] = useState< + Array + >([]); const updateFocus = (focusedCell: [number, number]) => { - console.log(focusedCell, cellsUpdateFocus); - /* Commented in order to pass the tests const updateFocus = cellsUpdateFocus[focusedCell[0]][focusedCell[1]]; if (updateFocus) { updateFocus(); } - */ }; const datagridContext = { onFocusUpdate: (cell: [number, number], updateFocus: Function) => { - // TODO: stores the listeners it receives and calls them in response to setCellFocus, - // passing the new focusedCell array to the callbacks - if (!cellsUpdateFocus[cell[0]]) { - cellsUpdateFocus[cell[0]] = []; - } + if (pagination) { + // Receives the row index as for the whole set + // and normalizes it for the visible rows in the grid + const pageIndex = pagination.pageIndex; + const pageSize = pagination.pageSize; + const rowIndex = Math.ceil(cell[1] - pageIndex * pageSize); + + if (!cellsUpdateFocus[cell[0]]) { + cellsUpdateFocus[cell[0]] = []; + } - cellsUpdateFocus[cell[0]][cell[1]] = updateFocus; + cellsUpdateFocus[cell[0]][rowIndex] = updateFocus; - return () => { - delete cellsUpdateFocus[cell[0]][cell[1]]; - if (cellsUpdateFocus[cell[0]].length === 0) { - delete cellsUpdateFocus[cell[0]]; - } - }; + setCellsUpdateFocus(cellsUpdateFocus); + + return () => { + cellsUpdateFocus[cell[0]][rowIndex] = null; + }; + } }, }; diff --git a/src/components/datagrid/data_grid_cell.tsx b/src/components/datagrid/data_grid_cell.tsx index f0dd8d742e6..d2feece0f0e 100644 --- a/src/components/datagrid/data_grid_cell.tsx +++ b/src/components/datagrid/data_grid_cell.tsx @@ -106,7 +106,7 @@ export class EuiDataGridCell extends Component< cellProps: {}, popoverIsOpen: false, }; - unsubscribeCell = Function; + unsubscribeCell: Function = () => {}; static contextType = DataGridContext; @@ -120,17 +120,16 @@ export class EuiDataGridCell extends Component< }; componentDidMount() { - // TODO: call onFocusUpdate as provide its callback (e.g. its existing updateFocus method) - // TODO: Store the unsubscribe method returned by onFocusUpdate on the EuiDataGridCell instance this.unsubscribeCell = this.context.onFocusUpdate( - [this.props.rowIndex, this.props.colIndex], + [this.props.colIndex, this.props.rowIndex], this.updateFocus ); } componentWillUnmount() { - // TODO: call the stored unsubscribe method - this.unsubscribeCell(); + if (this.unsubscribeCell !== undefined) { + this.unsubscribeCell(); + } } componentDidUpdate(prevProps: EuiDataGridCellProps) { From fe3fc642774f0c5fa3369b487d82665e2df130db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fl=C3=A1vio=20Knob?= Date: Mon, 18 Nov 2019 18:37:39 -0300 Subject: [PATCH 11/14] Calls updateFocus() inside a requestAnimationFrame() --- src/components/datagrid/data_grid.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index ae5c1e62cca..bf87c25da60 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -333,8 +333,7 @@ function createKeyDownHandler( ? focusedCell[1] : newPageRowCount - 1; setFocusedCell([focusedCell[0], rowIndex]); - // TODO: Probably not the right point to call this - updateFocus([focusedCell[0], rowIndex]); + requestAnimationFrame(() => updateFocus([focusedCell[0], rowIndex])); } } } else if (keyCode === keyCodes.PAGE_UP) { @@ -344,9 +343,13 @@ function createKeyDownHandler( if (pageIndex > 0) { props.pagination!.pageIndex = pageIndex - 1; props.pagination.onChangePage(props.pagination.pageIndex); + const newPageRowCount = computeVisibleRows(props); + const rowIndex = + focusedCell[1] < newPageRowCount + ? focusedCell[1] + : newPageRowCount - 1; setFocusedCell([focusedCell[0], focusedCell[1]]); - // TODO: Probably not the right point to call this - updateFocus([focusedCell[0], focusedCell[1]]); + requestAnimationFrame(() => updateFocus([focusedCell[0], rowIndex])); } } } else if (keyCode === (ctrlKey && keyCodes.END)) { From 8c017842230c8af51cfa0d738a52b8cd37ad050e Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Tue, 19 Nov 2019 14:39:57 -0700 Subject: [PATCH 12/14] EuiDataGrid keyboard navigation tweaks --- CHANGELOG.md | 2 +- src/components/datagrid/data_grid.test.tsx | 88 +++++++++++-------- src/components/datagrid/data_grid.tsx | 42 +++++---- src/components/datagrid/data_grid_body.tsx | 33 +------ src/components/datagrid/data_grid_cell.tsx | 8 +- .../datagrid/data_grid_data_row.tsx | 1 + 6 files changed, 85 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc653e17630..d0c3199b530 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ## [`master`](https://github.com/elastic/eui/tree/master) -- Added new keyboard shortcuts for the data grid component: `Home` (same row, first column), `End` (same row, last column), `Ctrl+Home` (first row, first column), `Ctrl+End` (last row, last column), `Page Up` (next page) and `Page Down` (previous page) - Added `badge` prop and new styles `EuiHeaderAlert` ([#2506](https://github.com/elastic/eui/pull/2506)) +- Added new keyboard shortcuts for the data grid component: `Home` (same row, first column), `End` (same row, last column), `Ctrl+Home` (first row, first column), `Ctrl+End` (last row, last column), `Page Up` (next page) and `Page Down` (previous page) ([#2519](https://github.com/elastic/eui/pull/2519)) ## [`16.0.1`](https://github.com/elastic/eui/tree/v16.0.1) diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index 18705bb0a55..8d3dd57e46d 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -1441,6 +1441,20 @@ Array [ describe('keyboard controls', () => { it('supports simple arrow navigation', () => { + let pagination = { + pageIndex: 0, + pageSize: 3, + pageSizeOptions: [3, 6, 10], + onChangePage: (pageIndex: number) => { + pagination = { + ...pagination, + pageIndex, + }; + component.setProps({ pagination }); + }, + onChangeItemsPerPage: () => {}, + }; + const component = mount( `${rowIndex}, ${columnId}` } - pagination={{ - pageIndex: 0, - pageSize: 3, - pageSizeOptions: [3, 6, 10], - onChangePage: () => {}, - onChangeItemsPerPage: () => {}, - }} + pagination={pagination} /> ); let focusableCell = getFocusableCell(component); + // focus should begin at the first cell expect(focusableCell.length).toEqual(1); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('0, A'); + // focus should not move when up against the left edge focusableCell .simulate('focus') .simulate('keydown', { keyCode: keyCodes.LEFT }); - focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() - ).toEqual('0, A'); // focus should not move when up against an edge + ).toEqual('0, A'); + // focus should not move when up against the top edge focusableCell.simulate('keydown', { keyCode: keyCodes.UP }); expect( focusableCell.find('[data-test-subj="cell-content"]').text() - ).toEqual('0, A'); // focus should not move when up against an edge + ).toEqual('0, A'); + // move down focusableCell.simulate('keydown', { keyCode: keyCodes.DOWN }); - focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('1, A'); + // move right focusableCell.simulate('keydown', { keyCode: keyCodes.RIGHT }); - focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('1, B'); + // move up focusableCell.simulate('keydown', { keyCode: keyCodes.UP }); - focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('0, B'); + // move left focusableCell.simulate('keydown', { keyCode: keyCodes.LEFT }); - focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('0, A'); + // move down and to the end of the row focusableCell .simulate('keydown', { keyCode: keyCodes.DOWN }) .simulate('keydown', { keyCode: keyCodes.END }); - focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('1, C'); + // move up and to the beginning of the row focusableCell .simulate('keydown', { keyCode: keyCodes.UP }) .simulate('keydown', { keyCode: keyCodes.HOME }); - focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('0, A'); + // jump to the last cell focusableCell.simulate('keydown', { ctrlKey: true, keyCode: keyCodes.END, }); - focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('2, C'); + // jump to the first cell focusableCell.simulate('keydown', { ctrlKey: true, keyCode: keyCodes.HOME, }); - focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('0, A'); + // page should not change when moving before the first entry focusableCell.simulate('keydown', { keyCode: keyCodes.PAGE_UP, - }); // 0, A - + }); focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() - ).toEqual('0, A'); // focus should not move when up against an edge + ).toEqual('0, A'); + // advance to the next page focusableCell.simulate('keydown', { keyCode: keyCodes.PAGE_DOWN, - }); // 3, A - + }); focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('3, A'); + // move over one column and advance one more page focusableCell .simulate('keydown', { keyCode: keyCodes.RIGHT }) // 3, B .simulate('keydown', { keyCode: keyCodes.PAGE_DOWN, }); // 6, B + focusableCell = getFocusableCell(component); + expect( + focusableCell.find('[data-test-subj="cell-content"]').text() + ).toEqual('6, B'); + // does not advance beyond the last page + focusableCell.simulate('keydown', { + keyCode: keyCodes.PAGE_DOWN, + }); focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() - ).toEqual('6, B'); // should move page forward and keep focus on the same cell + ).toEqual('6, B'); + // move left one column, return to the previous page focusableCell .simulate('keydown', { keyCode: keyCodes.LEFT }) // 6, A .simulate('keydown', { keyCode: keyCodes.PAGE_UP, }); // 3, A - focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() - ).toEqual('3, A'); // back one page, at the first cell + ).toEqual('3, A'); + // return to the previous (first) page focusableCell.simulate('keydown', { keyCode: keyCodes.PAGE_UP, - }); // 0, A - + }); focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() - ).toEqual('0, A'); // should be back in the first page + ).toEqual('0, A'); + // move to the last cell of the page then advance one page focusableCell .simulate('keydown', { ctrlKey: true, @@ -1606,24 +1626,21 @@ Array [ .simulate('keydown', { keyCode: keyCodes.PAGE_DOWN, }); // 5, C (last cell of the second page, same cell position as previous page) - focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('5, C'); + // advance to the final page, but there is 1 row less on page 3 so focus should retreat a row but retain the column focusableCell.simulate('keydown', { keyCode: keyCodes.PAGE_DOWN, - }); // 7, C (should recalculate row since there is not as many rows as previous page) - + }); // 7, C focusableCell = getFocusableCell(component); expect( focusableCell.find('[data-test-subj="cell-content"]').text() ).toEqual('7, C'); - // (equivalent cell position does not exist in last page (would be 8, C), - // so keeps the same column position. but moves to the last available row, - // which should be (7, C)) }); + it('does not break arrow key focus control behavior when also using a mouse', () => { const component = mount( +) { const { pagination, rowCount } = props; const startRow = pagination ? pagination.pageIndex * pagination.pageSize : 0; @@ -320,14 +322,19 @@ function createKeyDownHandler( } else if (keyCode === keyCodes.PAGE_DOWN) { if (props.pagination) { event.preventDefault(); - const totalRowCount = props.rowCount; + const rowCount = props.rowCount; const pageIndex = props.pagination.pageIndex; const pageSize = props.pagination.pageSize; - const pageCount = Math.ceil(totalRowCount / pageSize); - if (pageIndex < pageCount) { - props.pagination!.pageIndex = pageIndex + 1; - props.pagination.onChangePage(props.pagination.pageIndex); - const newPageRowCount = computeVisibleRows(props); + const pageCount = Math.ceil(rowCount / pageSize); + if (pageIndex < pageCount - 1) { + props.pagination.onChangePage(pageIndex + 1); + const newPageRowCount = computeVisibleRows({ + rowCount, + pagination: { + ...props.pagination, + pageIndex: pageIndex + 1, + }, + }); const rowIndex = focusedCell[1] < newPageRowCount ? focusedCell[1] @@ -341,9 +348,14 @@ function createKeyDownHandler( event.preventDefault(); const pageIndex = props.pagination.pageIndex; if (pageIndex > 0) { - props.pagination!.pageIndex = pageIndex - 1; - props.pagination.onChangePage(props.pagination.pageIndex); - const newPageRowCount = computeVisibleRows(props); + props.pagination.onChangePage(pageIndex - 1); + const newPageRowCount = computeVisibleRows({ + rowCount, + pagination: { + ...props.pagination, + pageIndex: pageIndex - 1, + }, + }); const rowIndex = focusedCell[1] < newPageRowCount ? focusedCell[1] @@ -613,22 +625,16 @@ export const EuiDataGrid: FunctionComponent = props => { const datagridContext = { onFocusUpdate: (cell: [number, number], updateFocus: Function) => { if (pagination) { - // Receives the row index as for the whole set - // and normalizes it for the visible rows in the grid - const pageIndex = pagination.pageIndex; - const pageSize = pagination.pageSize; - const rowIndex = Math.ceil(cell[1] - pageIndex * pageSize); - if (!cellsUpdateFocus[cell[0]]) { cellsUpdateFocus[cell[0]] = []; } - cellsUpdateFocus[cell[0]][rowIndex] = updateFocus; + cellsUpdateFocus[cell[0]][cell[1]] = updateFocus; setCellsUpdateFocus(cellsUpdateFocus); return () => { - cellsUpdateFocus[cell[0]][rowIndex] = null; + cellsUpdateFocus[cell[0]][cell[1]] = null; }; } }, diff --git a/src/components/datagrid/data_grid_body.tsx b/src/components/datagrid/data_grid_body.tsx index 2db4592c50f..a7ad04662cb 100644 --- a/src/components/datagrid/data_grid_body.tsx +++ b/src/components/datagrid/data_grid_body.tsx @@ -1,9 +1,4 @@ -import React, { - Fragment, - FunctionComponent, - useCallback, - useMemo, -} from 'react'; +import React, { Fragment, FunctionComponent, useMemo } from 'react'; // @ts-ignore-next-line import { EuiCodeBlock } from '../code'; import { @@ -163,30 +158,6 @@ export const EuiDataGridBody: FunctionComponent< return rowMap; }, [sorting, inMemory, inMemoryValues, schema, schemaDetectors]); - const setCellFocus = useCallback( - ([colIndex, rowIndex]) => { - // If the rows in the grid have been mapped in some way (e.g. sorting) - // then this callback must unmap the reported rowIndex - const mappedRowIndicies = Object.keys(rowMap); - let reverseMappedIndex = rowIndex; - for (let i = 0; i < mappedRowIndicies.length; i++) { - const mappedRowIndex = mappedRowIndicies[i]; - const rowMappedToIndex = rowMap[(mappedRowIndex as any) as number]; - if (`${rowMappedToIndex}` === `${rowIndex}`) { - reverseMappedIndex = parseInt(mappedRowIndex); - break; - } - } - - // map the row into the visible rows - if (pagination) { - reverseMappedIndex -= pagination.pageIndex * pagination.pageSize; - } - onCellFocus([colIndex, reverseMappedIndex]); - }, - [onCellFocus, rowMap, pagination] - ); - const rows = useMemo(() => { const rows = []; for (let i = 0; i < visibleRowIndices.length; i++) { @@ -209,7 +180,7 @@ export const EuiDataGridBody: FunctionComponent< columnWidths={columnWidths} defaultColumnWidth={defaultColumnWidth} focusedCell={focusedCell} - onCellFocus={setCellFocus} + onCellFocus={onCellFocus} renderCellValue={renderCellValue} rowIndex={rowIndex} visibleRowIndex={i} diff --git a/src/components/datagrid/data_grid_cell.tsx b/src/components/datagrid/data_grid_cell.tsx index d2feece0f0e..cbe10210316 100644 --- a/src/components/datagrid/data_grid_cell.tsx +++ b/src/components/datagrid/data_grid_cell.tsx @@ -53,6 +53,7 @@ export interface EuiDataGridCellValueElementProps { export interface EuiDataGridCellProps { rowIndex: number; + visibleRowIndex: number; colIndex: number; columnId: string; columnType?: string | null; @@ -121,7 +122,7 @@ export class EuiDataGridCell extends Component< componentDidMount() { this.unsubscribeCell = this.context.onFocusUpdate( - [this.props.colIndex, this.props.rowIndex], + [this.props.colIndex, this.props.visibleRowIndex], this.updateFocus ); } @@ -145,6 +146,7 @@ export class EuiDataGridCell extends Component< nextState: EuiDataGridCellState ) { if (nextProps.rowIndex !== this.props.rowIndex) return true; + if (nextProps.visibleRowIndex !== this.props.visibleRowIndex) return true; if (nextProps.colIndex !== this.props.colIndex) return true; if (nextProps.columnId !== this.props.columnId) return true; if (nextProps.width !== this.props.width) return true; @@ -190,7 +192,7 @@ export class EuiDataGridCell extends Component< onCellFocus, ...rest } = this.props; - const { colIndex, rowIndex } = rest; + const { colIndex, rowIndex, visibleRowIndex } = rest; const className = classNames('euiDataGridRowCell', { [`euiDataGridRowCell--${columnType}`]: columnType, @@ -378,7 +380,7 @@ export class EuiDataGridCell extends Component< {...cellProps} data-test-subj="dataGridRowCell" onKeyDown={handleCellKeyDown} - onFocus={() => onCellFocus([colIndex, rowIndex])}> + onFocus={() => onCellFocus([colIndex, visibleRowIndex])}> {innerContent}
); diff --git a/src/components/datagrid/data_grid_data_row.tsx b/src/components/datagrid/data_grid_data_row.tsx index abe7aed8ec2..1660ac2c767 100644 --- a/src/components/datagrid/data_grid_data_row.tsx +++ b/src/components/datagrid/data_grid_data_row.tsx @@ -74,6 +74,7 @@ const EuiDataGridDataRow: FunctionComponent< Date: Tue, 19 Nov 2019 15:12:35 -0700 Subject: [PATCH 13/14] Refactored cellsUpdateFocus usage --- src/components/datagrid/data_grid.tsx | 45 +++++++++------------- src/components/datagrid/data_grid_cell.tsx | 4 +- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index 02d64bdaa72..ac1de404e27 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -340,7 +340,7 @@ function createKeyDownHandler( ? focusedCell[1] : newPageRowCount - 1; setFocusedCell([focusedCell[0], rowIndex]); - requestAnimationFrame(() => updateFocus([focusedCell[0], rowIndex])); + updateFocus([focusedCell[0], rowIndex]); } } } else if (keyCode === keyCodes.PAGE_UP) { @@ -349,19 +349,7 @@ function createKeyDownHandler( const pageIndex = props.pagination.pageIndex; if (pageIndex > 0) { props.pagination.onChangePage(pageIndex - 1); - const newPageRowCount = computeVisibleRows({ - rowCount, - pagination: { - ...props.pagination, - pageIndex: pageIndex - 1, - }, - }); - const rowIndex = - focusedCell[1] < newPageRowCount - ? focusedCell[1] - : newPageRowCount - 1; - setFocusedCell([focusedCell[0], focusedCell[1]]); - requestAnimationFrame(() => updateFocus([focusedCell[0], rowIndex])); + updateFocus(focusedCell); } } } else if (keyCode === (ctrlKey && keyCodes.END)) { @@ -611,30 +599,33 @@ export const EuiDataGrid: FunctionComponent = props => { ); const [cellsUpdateFocus, setCellsUpdateFocus] = useState< - Array - >([]); + Map + >(new Map()); const updateFocus = (focusedCell: [number, number]) => { - const updateFocus = cellsUpdateFocus[focusedCell[0]][focusedCell[1]]; - - if (updateFocus) { - updateFocus(); + const key = `${focusedCell[0]}-${focusedCell[1]}`; + if (cellsUpdateFocus.has(key)) { + requestAnimationFrame(() => { + cellsUpdateFocus.get(key)!(); + }); } }; const datagridContext = { onFocusUpdate: (cell: [number, number], updateFocus: Function) => { if (pagination) { - if (!cellsUpdateFocus[cell[0]]) { - cellsUpdateFocus[cell[0]] = []; - } - - cellsUpdateFocus[cell[0]][cell[1]] = updateFocus; + const key = `${cell[0]}-${cell[1]}`; - setCellsUpdateFocus(cellsUpdateFocus); + setCellsUpdateFocus(cellsUpdateFocus => { + cellsUpdateFocus.set(key, updateFocus); + return cellsUpdateFocus; + }); return () => { - cellsUpdateFocus[cell[0]][cell[1]] = null; + setCellsUpdateFocus(cellsUpdateFocus => { + cellsUpdateFocus.delete(key); + return cellsUpdateFocus; + }); }; } }, diff --git a/src/components/datagrid/data_grid_cell.tsx b/src/components/datagrid/data_grid_cell.tsx index cbe10210316..c5d6e31fcce 100644 --- a/src/components/datagrid/data_grid_cell.tsx +++ b/src/components/datagrid/data_grid_cell.tsx @@ -107,7 +107,7 @@ export class EuiDataGridCell extends Component< cellProps: {}, popoverIsOpen: false, }; - unsubscribeCell: Function = () => {}; + unsubscribeCell?: Function = () => {}; static contextType = DataGridContext; @@ -128,7 +128,7 @@ export class EuiDataGridCell extends Component< } componentWillUnmount() { - if (this.unsubscribeCell !== undefined) { + if (this.unsubscribeCell) { this.unsubscribeCell(); } } From 4cbb9fad513dd9b611b191705befcc6a2073f34c Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Wed, 20 Nov 2019 09:00:07 -0700 Subject: [PATCH 14/14] commented on an anti-pattern --- src/components/datagrid/data_grid.tsx | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/components/datagrid/data_grid.tsx b/src/components/datagrid/data_grid.tsx index ac1de404e27..8c5548195da 100644 --- a/src/components/datagrid/data_grid.tsx +++ b/src/components/datagrid/data_grid.tsx @@ -598,9 +598,7 @@ export const EuiDataGrid: FunctionComponent = props => { ); - const [cellsUpdateFocus, setCellsUpdateFocus] = useState< - Map - >(new Map()); + const [cellsUpdateFocus] = useState>(new Map()); const updateFocus = (focusedCell: [number, number]) => { const key = `${focusedCell[0]}-${focusedCell[1]}`; @@ -616,16 +614,13 @@ export const EuiDataGrid: FunctionComponent = props => { if (pagination) { const key = `${cell[0]}-${cell[1]}`; - setCellsUpdateFocus(cellsUpdateFocus => { - cellsUpdateFocus.set(key, updateFocus); - return cellsUpdateFocus; - }); + // this intentionally and purposefully mutates the existing `cellsUpdateFocus` object as the + // value/state of `cellsUpdateFocus` must be up-to-date when `updateFocus`'s requestAnimationFrame fires + // there is likely a better pattern to use, but this is fine for now as the scope is known & limited + cellsUpdateFocus.set(key, updateFocus); return () => { - setCellsUpdateFocus(cellsUpdateFocus => { - cellsUpdateFocus.delete(key); - return cellsUpdateFocus; - }); + cellsUpdateFocus.delete(key); }; } },