[EuiDataGrid] Fix keyboard navigation not fully scrolling cells into view

CHANGELOG.md

@@ -5,6 +5,15 @@
 `$euiFocusRingColor` instead of `currentColor` ([#5479](https://github.com/elastic/eui/pull/5479))
 - **[Beta]** Added `optimize` build as a lighter weight option more suited to prodcution environments ([#5527](https://github.com/elastic/eui/pull/5527))
 
+**Bug fixes**
+
+- Updated the outline color in `euiCustomControlFocused` mixin to use `$euiFocusRingColor` instead of `currentColor` ([#5479](https://github.com/elastic/eui/pull/5479))
+- Fixed keyboard navigation in `EuiDataGrid` not fully scrolling cells into view ([#5515](https://github.com/elastic/eui/pull/5515))
+
+**Deprecations**
+
+- Deprecated `data-gridcell-id` from `EuiDataGrid` in favor of 4 new and more flexible props - `data-gridcell-column-id`, `data-gridcell-column-index`, `data-gridcell-row-index`, and `data-gridcell-visible-row-index` ([#5515](https://github.com/elastic/eui/pull/5515))
+
 ## [`45.0.0`](https://github.com/elastic/eui/tree/v45.0.0)
 
 - Added virtulized rendering option to `EuiSelectableList` with `isVirtualized` ([#5521](https://github.com/elastic/eui/pull/5521))

src/components/datagrid/body/__snapshots__/data_grid_body.test.tsx.snap

@@ -15,6 +15,10 @@ exports[`EuiDataGridBody renders 1`] = `
     >
       <div
         class="euiDataGridHeaderCell euiDataGridHeaderCell--boolean"
+        data-gridcell-column-id="columnA"
+        data-gridcell-column-index="0"
+        data-gridcell-row-index="-1"
+        data-gridcell-visible-row-index="-1"
         data-test-subj="dataGridHeaderCell-columnA"
         role="columnheader"
         style="width:100px"
@@ -53,6 +57,10 @@ exports[`EuiDataGridBody renders 1`] = `
       </div>
       <div
         class="euiDataGridHeaderCell euiDataGridHeaderCell--string"
+        data-gridcell-column-id="columnB"
+        data-gridcell-column-index="1"
+        data-gridcell-row-index="-1"
+        data-gridcell-visible-row-index="-1"
         data-test-subj="dataGridHeaderCell-columnB"
         role="columnheader"
         style="width:100px"
@@ -92,7 +100,11 @@ exports[`EuiDataGridBody renders 1`] = `
     </div>
     <div
       class="euiDataGridRowCell euiDataGridRowCell--boolean euiDataGridRowCell--firstColumn"
+      data-gridcell-column-id="columnA"
+      data-gridcell-column-index="0"
       data-gridcell-id="0,0"
+      data-gridcell-row-index="0"
+      data-gridcell-visible-row-index="0"
       data-test-subj="dataGridRowCell"
       role="gridcell"
       style="position:absolute;left:0;top:0px;height:34px;width:100px"
@@ -122,7 +134,11 @@ exports[`EuiDataGridBody renders 1`] = `
     </div>
     <div
       class="euiDataGridRowCell euiDataGridRowCell--string euiDataGridRowCell--lastColumn"
-      data-gridcell-id="0,1"
+      data-gridcell-column-id="columnB"
+      data-gridcell-column-index="1"
+      data-gridcell-id="1,0"
+      data-gridcell-row-index="0"
+      data-gridcell-visible-row-index="0"
       data-test-subj="dataGridRowCell"
       role="gridcell"
       style="position:absolute;left:100px;top:0px;height:34px;width:100px"

src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap

@@ -43,7 +43,11 @@ exports[`EuiDataGridCell renders 1`] = `
 >
   <div
     className="euiDataGridRowCell"
+    data-gridcell-column-id="someColumn"
+    data-gridcell-column-index={0}
     data-gridcell-id="0,0"
+    data-gridcell-row-index={0}
+    data-gridcell-visible-row-index={0}
     data-test-subj="dataGridRowCell"
     onBlur={[Function]}
     onFocus={[Function]}

src/components/datagrid/body/data_grid_body.tsx

@@ -43,6 +43,7 @@ import {
 import { useDefaultColumnWidth, useColumnWidths } from '../utils/col_widths';
 import { useRowHeightUtils, useDefaultRowHeight } from '../utils/row_heights';
 import { useHeaderFocusWorkaround } from '../utils/focus';
+import { useScroll } from '../utils/scrolling';
 import { DataGridSortingContext } from '../utils/sorting';
 import { IS_JEST_ENVIRONMENT } from '../../../test';
 
@@ -359,6 +360,19 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
     visibleRowCount,
   ]);
 
+  /**
+   * Handle scrolling cells fully into view
+   */
+  useScroll({
+    gridRef,
+    outerGridRef,
+    innerGridRef,
+    headerRowHeight,
+    footerRowHeight,
+    visibleRowCount,
+    hasStickyFooter: !!(renderFooterCellValue && gridStyles.stickyFooter),
+  });
+
   /**
    * Row manager
    */

src/components/datagrid/body/data_grid_cell.tsx

@@ -616,7 +616,12 @@ export class EuiDataGridCell extends Component<
         ref={this.cellRef}
         {...cellProps}
         data-test-subj="dataGridRowCell"
-        data-gridcell-id={`${this.props.rowIndex},${this.props.colIndex}`}
+        // Data attributes to help target specific cells by either data or current cell location
+        data-gridcell-column-id={this.props.columnId} // Static column ID name, not affected by column order
+        data-gridcell-column-index={this.props.colIndex} // Affected by column reordering
+        data-gridcell-row-index={this.props.rowIndex} // Index from data, not affected by sorting or pagination
+        data-gridcell-visible-row-index={this.props.visibleRowIndex} // Affected by sorting & pagination
+        data-gridcell-id={`${this.props.colIndex},${this.props.rowIndex}`} // TODO: Deprecate in favor of the above 4 data attrs
         onKeyDown={handleCellKeyDown}
         onFocus={this.onFocus}
         onMouseEnter={() => {

src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx

@@ -49,6 +49,10 @@ describe('EuiDataGridHeaderCellWrapper', () => {
       >
         <div
           className="euiDataGridHeaderCell"
+          data-gridcell-column-id="someColumn"
+          data-gridcell-column-index={0}
+          data-gridcell-row-index="-1"
+          data-gridcell-visible-row-index="-1"
           data-test-subj="dataGridHeaderCell-someColumn"
           role="columnheader"
           style={Object {}}
@@ -71,6 +75,10 @@ describe('EuiDataGridHeaderCellWrapper', () => {
       <div
         aria-label="test"
         className="euiDataGridHeaderCell euiDataGridHeaderCell--test"
+        data-gridcell-column-id="someColumn"
+        data-gridcell-column-index={0}
+        data-gridcell-row-index="-1"
+        data-gridcell-visible-row-index="-1"
         data-test-subj="dataGridHeaderCell-someColumn"
         role="columnheader"
         style={

src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx

@@ -134,6 +134,10 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent<EuiDataGridHeaderCe
       tabIndex={isFocused && !isCellEntered ? 0 : -1}
       className={classes}
       data-test-subj={`dataGridHeaderCell-${id}`}
+      data-gridcell-column-id={id}
+      data-gridcell-column-index={index}
+      data-gridcell-row-index="-1"
+      data-gridcell-visible-row-index="-1"
       style={width != null ? { width: `${width}px` } : {}}
       {...rest}
     >

src/components/datagrid/controls/display_selector.test.tsx

@@ -9,6 +9,7 @@
 import React from 'react';
 import { act } from 'react-dom/test-utils';
 import { shallow, mount, ShallowWrapper, ReactWrapper } from 'enzyme';
+import { testCustomHook } from '../../../test';
 
 import {
   EuiDataGridToolBarVisibilityOptions,
@@ -419,27 +420,21 @@ describe('useDataGridDisplaySelector', () => {
   describe('gridStyles', () => {
     it('returns an object of grid styles with user overrides', () => {
       const initialStyles = { ...startingStyles, stripes: true };
-      const MockComponent = () => {
-        const [, { onChange, ...gridStyles }] = useDataGridDisplaySelector(
-          true,
-          initialStyles,
-          {}
-        );
-        return <table {...gridStyles} />;
-      };
-      const component = shallow(<MockComponent />);
+      const [, gridStyles] = testCustomHook(() =>
+        useDataGridDisplaySelector(true, initialStyles, {})
+      );
 
-      expect(component).toMatchInlineSnapshot(`
-        <table
-          border="all"
-          cellPadding="m"
-          fontSize="m"
-          footer="overline"
-          header="shade"
-          rowHover="highlight"
-          stickyFooter={true}
-          stripes={true}
-        />
+      expect(gridStyles).toMatchInlineSnapshot(`
+        Object {
+          "border": "all",
+          "cellPadding": "m",
+          "fontSize": "m",
+          "footer": "overline",
+          "header": "shade",
+          "rowHover": "highlight",
+          "stickyFooter": true,
+          "stripes": true,
+        }
       `);
     });
   });

src/components/datagrid/data_grid.spec.tsx

@@ -236,8 +236,12 @@ describe('EuiDataGrid', () => {
       // starts with body in focus
       cy.focused().should('not.exist');
 
-      cy.get('[data-gridcell-id="1,1"]').click();
-      cy.focused().should('have.attr', 'data-gridcell-id', '1,1');
+      cy.get(
+        '[data-gridcell-column-index="1"][data-gridcell-visible-row-index="1"]'
+      ).click();
+      cy.focused()
+        .should('have.attr', 'data-gridcell-column-index', '1')
+        .should('have.attr', 'data-gridcell-row-index', '1');
     });
 
     describe('cell keyboard interactions', () => {
@@ -269,7 +273,9 @@ describe('EuiDataGrid', () => {
 
         // tab into the grid, should focus first cell after a short delay
         cy.focused().tab();
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,0');
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '0')
+          .should('have.attr', 'data-gridcell-row-index', '0');
 
         cy.focused().tab().should('have.id', 'final-tabbable');
       });
@@ -282,27 +288,37 @@ describe('EuiDataGrid', () => {
         cy.get('[data-test-subj=euiDataGridBody]').focus();
 
         // first cell is non-interactive and non-expandable = focus cell
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,0');
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '0')
+          .should('have.attr', 'data-gridcell-row-index', '0');
 
         // already left-most, should be no-op
         cy.focused().type('{leftarrow}');
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,0');
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '0')
+          .should('have.attr', 'data-gridcell-row-index', '0');
 
         // arrow right, expandable cell with no interactive = focus cell
         cy.focused().type('{rightarrow}');
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,1');
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '1')
+          .should('have.attr', 'data-gridcell-row-index', '0');
 
         // arrow right, non-expandable cell with one interactive = focus interactive
         cy.focused().type('{rightarrow}');
         cy.focused().should('have.attr', 'data-test-subj', 'focusOnMe');
 
         // arrow right, non-expandable cell with two interactives = focus cell
         cy.focused().type('{rightarrow}');
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,3');
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '3')
+          .should('have.attr', 'data-gridcell-row-index', '0');
 
         // arrow right, expandable cell with two interactives = focus cell
         cy.focused().type('{rightarrow}');
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,4');
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '4')
+          .should('have.attr', 'data-gridcell-row-index', '0');
       });
 
       it('cell expansion/interaction', () => {
@@ -314,34 +330,50 @@ describe('EuiDataGrid', () => {
 
         // first cell is non-interactive and non-expandable, enter should have no effect
         cy.focused().type('{enter}');
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,0');
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '0')
+          .should('have.attr', 'data-gridcell-row-index', '0');
 
         // second cell is expandable
-        cy.get('[data-gridcell-id="0,1"]').click();
+        cy.get(
+          '[data-gridcell-column-index="1"][data-gridcell-row-index="0"]'
+        ).click();
         cy.focused().type('{enter}');
         cy.focused().should(
           'have.attr',
           'data-test-subj',
           'euiDataGridExpansionPopover'
         );
         cy.focused().type('{esc}');
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,1');
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '1')
+          .should('have.attr', 'data-gridcell-row-index', '0');
 
         // third cell is non-expandable & interactive, click should focus on the link
-        cy.get('[data-gridcell-id="0,2"]').click();
+        cy.get(
+          '[data-gridcell-column-index="2"][data-gridcell-row-index="0"]'
+        ).click();
         cy.focused().type('{enter}');
         cy.focused().should('have.attr', 'data-test-subj', 'focusOnMe');
 
         // fourth cell is non-expandable with multiple interactives, click should focus on the cell
-        cy.get('[data-gridcell-id="0,3"]').click();
+        cy.get(
+          '[data-gridcell-column-index="3"][data-gridcell-row-index="0"]'
+        ).click();
         cy.focused().type('{enter}');
         cy.focused().should('have.attr', 'data-test-subj', 'focusOnMe'); // focus trap focuses the link
         cy.focused().type('{esc}');
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,3');
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '3')
+          .should('have.attr', 'data-gridcell-row-index', '0');
 
         // fifth cell is non-expandable & no-actions with multiple interactives, click should focus cell
-        cy.get('[data-gridcell-id="0,4"]').click('topLeft'); // top left to avoid clicking a button
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,4');
+        cy.get(
+          '[data-gridcell-column-index="4"][data-gridcell-row-index="0"]'
+        ).click('topLeft'); // top left to avoid clicking a button
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '4')
+          .should('have.attr', 'data-gridcell-row-index', '0');
         // enable interactives & focus trap
         cy.focused().type('{enter}');
         cy.focused().should('have.attr', 'data-test-subj', 'btn-yes');
@@ -350,11 +382,17 @@ describe('EuiDataGrid', () => {
         cy.focused().tab();
         cy.focused().should('have.attr', 'data-test-subj', 'btn-yes');
         cy.focused().type('{esc}');
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,4');
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '4')
+          .should('have.attr', 'data-gridcell-row-index', '0');
 
         // sixth cell is expandable cell with two interactives, click should focus on the cell
-        cy.get('[data-gridcell-id="0,5"]').click('topLeft', { force: true }); // top left to avoid clicking a button
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,5');
+        cy.get(
+          '[data-gridcell-column-index="5"][data-gridcell-row-index="0"]'
+        ).click('topLeft', { force: true }); // top left to avoid clicking a button
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '5')
+          .should('have.attr', 'data-gridcell-row-index', '0');
         cy.focused().type('{enter}'); // trigger expansion popover
         cy.focused().should('have.attr', 'data-test-subj', 'btn-yes'); // focus trap should move focus to the first button
         cy.focused().parentsUntil(
@@ -369,15 +407,17 @@ describe('EuiDataGrid', () => {
           'euiDataGridExpansionPopover'
         );
         cy.focused().type('{esc}');
-        cy.focused().should('have.attr', 'data-gridcell-id', '0,5');
+        cy.focused()
+          .should('have.attr', 'data-gridcell-column-index', '5')
+          .should('have.attr', 'data-gridcell-row-index', '0');
       });
     });
   });
 });
 
 function getGridData() {
   // wait for the virtualized cells to render
-  cy.get('[data-gridcell-id="1,0"]');
+  cy.get('[data-gridcell-column-index="0"][data-gridcell-row-index="1"]');
   const rows = cy.get('[role=row]');
   return rows.then((rows) => {
     const headers: string[] = [];