From df318b311710f8f682dfc116093cbd22cc1bad54 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 14 Mar 2022 14:44:24 -0700 Subject: [PATCH 1/7] 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) --- src/components/datagrid/body/data_grid_footer_row.test.tsx | 4 ++-- src/components/datagrid/body/data_grid_footer_row.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/datagrid/body/data_grid_footer_row.test.tsx b/src/components/datagrid/body/data_grid_footer_row.test.tsx index 33af4964473..eca2c81be9c 100644 --- a/src/components/datagrid/body/data_grid_footer_row.test.tsx +++ b/src/components/datagrid/body/data_grid_footer_row.test.tsx @@ -39,7 +39,7 @@ describe('EuiDataGridFooterRow', () => { columnType="string" interactiveCellId="someId" isExpandable={true} - key="someColumn-10" + key="0,10" popoverContext={ Object { "cellLocation": Object { @@ -65,7 +65,7 @@ describe('EuiDataGridFooterRow', () => { columnType={null} interactiveCellId="someId" isExpandable={true} - key="someColumnWithoutSchema-10" + key="1,10" popoverContext={ Object { "cellLocation": Object { diff --git a/src/components/datagrid/body/data_grid_footer_row.tsx b/src/components/datagrid/body/data_grid_footer_row.tsx index fea8bdc8ea7..ca78dacc3cd 100644 --- a/src/components/datagrid/body/data_grid_footer_row.tsx +++ b/src/components/datagrid/body/data_grid_footer_row.tsx @@ -81,7 +81,7 @@ const EuiDataGridFooterRow = memo( return ( Date: Mon, 14 Mar 2022 14:53:38 -0700 Subject: [PATCH 2/7] Footer docs example: remove expand button if footer cell is empty --- .../views/datagrid/schema_columns/footer_row.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src-docs/src/views/datagrid/schema_columns/footer_row.js b/src-docs/src/views/datagrid/schema_columns/footer_row.js index 4f5cdc4ebc8..7a52ef909c4 100644 --- a/src-docs/src/views/datagrid/schema_columns/footer_row.js +++ b/src-docs/src/views/datagrid/schema_columns/footer_row.js @@ -75,8 +75,16 @@ const footerCellValues = { }`, }; -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 @@ -121,7 +129,7 @@ export default () => { rowCount={raw_data.length} renderCellValue={RenderCellValue} renderFooterCellValue={ - showFooterRow ? renderFooterCellValue : undefined + showFooterRow ? RenderFooterCellValue : undefined } pagination={{ ...pagination, From 730b1731d3c69da19d2842ccd1d1260df5fb7b86 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 14 Mar 2022 16:20:21 -0700 Subject: [PATCH 3/7] Add Cypress tests --- .../body/data_grid_footer_row.spec.tsx | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 src/components/datagrid/body/data_grid_footer_row.spec.tsx diff --git a/src/components/datagrid/body/data_grid_footer_row.spec.tsx b/src/components/datagrid/body/data_grid_footer_row.spec.tsx new file mode 100644 index 00000000000..bae4600c7ae --- /dev/null +++ b/src/components/datagrid/body/data_grid_footer_row.spec.tsx @@ -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. + */ + +/// + +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 ( + + ); + }; + GridTest.displayName = 'GridTest'; + + it('renders a footer of total values', () => { + cy.mount(); + + 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(); + + 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(); + + 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(); + }); +}); From e72ef0ab97783d3101cf50e650a76c2e80b32dae Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 14 Mar 2022 16:22:18 -0700 Subject: [PATCH 4/7] Add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25667bf8d58..de2b2d3bee8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ - Updated `testenv` mock for `EuiIcon` to render `aria-label` as text ([#5709](https://github.com/elastic/eui/pull/5709)) - Added `editorChecklist` glyph to `EuiIcon` ([#5705](https://github.com/elastic/eui/pull/5705)) +**Bug fixes** + +- Fixed `EuiDataGrid` footer cell focus bugging out after moving its column ([#5720](https://github.com/elastic/eui/pull/5720)) + **Breaking changes** - Removed Legacy theme including compiled CSS ([#5688](https://github.com/elastic/eui/pull/5688)) From d3db2fc8ef93d8701636ad74a2cac482447a8b41 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 15 Mar 2022 13:43:15 -0700 Subject: [PATCH 5/7] Address PR feedback --- .../datagrid/schema_columns/datagrid_columns_example.js | 5 +++++ src-docs/src/views/datagrid/schema_columns/footer_row.js | 7 +++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src-docs/src/views/datagrid/schema_columns/datagrid_columns_example.js b/src-docs/src/views/datagrid/schema_columns/datagrid_columns_example.js index e553d1dccaf..cc5f073c1a9 100644 --- a/src-docs/src/views/datagrid/schema_columns/datagrid_columns_example.js +++ b/src-docs/src/views/datagrid/schema_columns/datagrid_columns_example.js @@ -298,6 +298,11 @@ schemaDetectors={[ EuiDataGridCellValueElementProps and returning a React node.

+

+ When rendering footer cell values, we encourage turning off cell + expansion on cells without content with{' '} + setCellProps({'{ isExpandable: false }'}). +

), props: { diff --git a/src-docs/src/views/datagrid/schema_columns/footer_row.js b/src-docs/src/views/datagrid/schema_columns/footer_row.js index 7a52ef909c4..b862cac6517 100644 --- a/src-docs/src/views/datagrid/schema_columns/footer_row.js +++ b/src-docs/src/views/datagrid/schema_columns/footer_row.js @@ -66,10 +66,9 @@ 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) + .toFixed(2)}`, version: `Latest: ${ raw_data.map(({ version }) => version).sort()[raw_data.length - 1] }`, From 48cfc2826918c3e45ad7ac4978b0924b41d20157 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 17 Mar 2022 13:14:55 -0700 Subject: [PATCH 6/7] changelog --- CHANGELOG.md | 4 ---- upcoming_changelogs/5720.md | 3 +++ 2 files changed, 3 insertions(+), 4 deletions(-) create mode 100644 upcoming_changelogs/5720.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 13a098a3fa1..48313c4b28b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,6 @@ - Added `compressed` prop to `EuiFilterGroup` and reduced the size of the `EuiFilterButton` notification badge ([#5717](https://github.com/elastic/eui/pull/5717)) - Increased contrast of `EuiSelectableTemplateSitewide` input text when in dark header ([#5724](https://github.com/elastic/eui/pull/5724)) -**Bug fixes** - -- Fixed `EuiDataGrid` footer cell focus bugging out after moving its column ([#5720](https://github.com/elastic/eui/pull/5720)) - **Breaking changes** - Removed Legacy theme including compiled CSS ([#5688](https://github.com/elastic/eui/pull/5688)) diff --git a/upcoming_changelogs/5720.md b/upcoming_changelogs/5720.md new file mode 100644 index 00000000000..77401a5208e --- /dev/null +++ b/upcoming_changelogs/5720.md @@ -0,0 +1,3 @@ +**Bug fixes** + +- Fixed `EuiDataGrid` footer cell focus bugging out after moving its column From 1cd438bebe8ea14925fd3d635a31e565a1357bb4 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 17 Mar 2022 13:18:12 -0700 Subject: [PATCH 7/7] PR feedback: currency formatting --- src-docs/src/views/datagrid/schema_columns/footer_row.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src-docs/src/views/datagrid/schema_columns/footer_row.js b/src-docs/src/views/datagrid/schema_columns/footer_row.js index b862cac6517..4222e2fa4a5 100644 --- a/src-docs/src/views/datagrid/schema_columns/footer_row.js +++ b/src-docs/src/views/datagrid/schema_columns/footer_row.js @@ -66,9 +66,9 @@ const columns = [ ]; const footerCellValues = { - amount: `Total: $${raw_data + amount: `Total: ${raw_data .reduce((acc, { amount }) => acc + Number(amount.split('$')[1]), 0) - .toFixed(2)}`, + .toLocaleString('en-US', { style: 'currency', currency: 'USD' })}`, version: `Latest: ${ raw_data.map(({ version }) => version).sort()[raw_data.length - 1] }`,