Skip to content

Commit

Permalink
[EuiDataGrid] Various footer fixes (#5720)
Browse files Browse the repository at this point in the history
* Fix focus fighting bug after moving a column and clicking its footer cell

Using a cell `key` with a column ID causes the cell node reference to move, instead of the cell/node reference staying in place and cell props changing (which is how react-window behaves)

* Footer docs example: remove expand button if footer cell is empty

* Add Cypress tests

* Add changelog entry

* Address PR feedback

* changelog

* PR feedback: currency formatting
  • Loading branch information
Constance authored Mar 18, 2022
1 parent b564bf1 commit 3b8aa95
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,11 @@ schemaDetectors={[
<EuiCode>EuiDataGridCellValueElementProps</EuiCode> and returning a
React node.
</p>
<p>
When rendering footer cell values, we encourage turning off cell
expansion on cells without content with{' '}
<EuiCode>setCellProps({'{ isExpandable: false }'})</EuiCode>.
</p>
</Fragment>
),
props: {
Expand Down
21 changes: 14 additions & 7 deletions src-docs/src/views/datagrid/schema_columns/footer_row.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,24 @@ const columns = [
];

const footerCellValues = {
amount: `Total: $${raw_data.reduce(
(acc, { amount }) => acc + Number(amount.split('$')[1]),
0
)}`,
amount: `Total: ${raw_data
.reduce((acc, { amount }) => acc + Number(amount.split('$')[1]), 0)
.toLocaleString('en-US', { style: 'currency', currency: 'USD' })}`,
version: `Latest: ${
raw_data.map(({ version }) => version).sort()[raw_data.length - 1]
}`,
};

const renderFooterCellValue = ({ columnId }) =>
footerCellValues[columnId] || null;
const RenderFooterCellValue = ({ columnId, setCellProps }) => {
const value = footerCellValues[columnId];

useEffect(() => {
// Turn off the cell expansion button if the footer cell is empty
if (!value) setCellProps({ isExpandable: false });
}, [value, setCellProps]);

return value || null;
};

export default () => {
// Pagination
Expand Down Expand Up @@ -121,7 +128,7 @@ export default () => {
rowCount={raw_data.length}
renderCellValue={RenderCellValue}
renderFooterCellValue={
showFooterRow ? renderFooterCellValue : undefined
showFooterRow ? RenderFooterCellValue : undefined
}
pagination={{
...pagination,
Expand Down
94 changes: 94 additions & 0 deletions src/components/datagrid/body/data_grid_footer_row.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

/// <reference types="../../../../cypress/support"/>

import React, { useState, useEffect } from 'react';
import { EuiDataGrid } from '../';

describe('EuiDataGridFooterRow', () => {
const mockData = [10, 15, 20];

const RenderCellValue = ({ rowIndex, columnId }) =>
columnId === 'b' ? mockData[rowIndex] : null;

const RenderFooterCellValue = ({ columnId, setCellProps }) => {
const value = columnId === 'b' ? mockData.reduce((a, b) => a + b, 0) : null;

useEffect(() => {
if (!value) setCellProps({ isExpandable: false });
}, [value, setCellProps]);

return value;
};

// We need to set up a test component here for column ordering to work
const GridTest = () => {
const [visibleColumns, setVisibleColumns] = useState(['a', 'b']);

return (
<EuiDataGrid
aria-label="Footer row tests"
columns={[{ id: 'a' }, { id: 'b', display: 'Total' }]}
columnVisibility={{ visibleColumns, setVisibleColumns }}
rowCount={3}
renderCellValue={RenderCellValue}
renderFooterCellValue={RenderFooterCellValue}
/>
);
};
GridTest.displayName = 'GridTest';

it('renders a footer of total values', () => {
cy.mount(<GridTest />);

cy.get(
'[data-gridcell-column-index="1"][data-gridcell-row-index="3"]'
).contains('45');
});

it('does not render an expansion button for the empty footer cell', () => {
cy.mount(<GridTest />);

cy.get(
'[data-gridcell-column-index="0"][data-gridcell-row-index="3"]'
).click();
cy.get('[data-test-subj="euiDataGridCellExpandButton"]').should(
'not.exist'
);
});

// Regression test for #5720
it('does not bug focus when moving a column and then clicking its footer cell', () => {
cy.mount(<GridTest />);

cy.get(
'[data-gridcell-column-index="1"][data-gridcell-row-index="-1"]'
).click();
cy.contains('Move left').click();

cy.get('[data-gridcell-column-index="0"][data-gridcell-row-index="3"]')
.click()
.contains('45')
.click();

// Note that the wait/timeout and multiple focused assertions are required for
// for this specific bug which bounces focus rapidly between cells, as otherwise
// Cypress re-tries until it happens to be on the cell with correct attributes
cy.wait(50);
const checkFocusedCell = () => {
cy.wait(1);
cy.focused({ timeout: 0 })
.should('have.attr', 'data-gridcell-column-index', '0')
.should('not.have.attr', 'data-gridcell-column-index', '1');
};
checkFocusedCell();
checkFocusedCell();
checkFocusedCell();
});
});
4 changes: 2 additions & 2 deletions src/components/datagrid/body/data_grid_footer_row.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('EuiDataGridFooterRow', () => {
columnType="string"
interactiveCellId="someId"
isExpandable={true}
key="someColumn-10"
key="0,10"
popoverContext={
Object {
"cellLocation": Object {
Expand All @@ -65,7 +65,7 @@ describe('EuiDataGridFooterRow', () => {
columnType={null}
interactiveCellId="someId"
isExpandable={true}
key="someColumnWithoutSchema-10"
key="1,10"
popoverContext={
Object {
"cellLocation": Object {
Expand Down
2 changes: 1 addition & 1 deletion src/components/datagrid/body/data_grid_footer_row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ const EuiDataGridFooterRow = memo(
return (
<EuiDataGridCell
{...sharedCellProps}
key={`${id}-${rowIndex}`}
key={`${columnPosition},${visibleRowIndex}`} // Note: this key should use cell position to match react-window/data cell behavior. See #5720
colIndex={columnPosition}
columnId={id}
columnType={columnType}
Expand Down
3 changes: 3 additions & 0 deletions upcoming_changelogs/5720.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed `EuiDataGrid` footer cell focus bugging out after moving its column

0 comments on commit 3b8aa95

Please sign in to comment.