From 41dec7866297dd7776439fbd2f8902bedb882098 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 16 Jan 2024 19:23:22 -0800 Subject: [PATCH 1/5] [perf] Reduce EuiDataGridCell rerenders - memoize all fns to class methods - do not define jsx as const, this causes react to redefine a new component each rerender --- .../datagrid/body/cell/data_grid_cell.tsx | 118 ++++++++++-------- 1 file changed, 63 insertions(+), 55 deletions(-) diff --git a/src/components/datagrid/body/cell/data_grid_cell.tsx b/src/components/datagrid/body/cell/data_grid_cell.tsx index af9041848df..1cc80e45a04 100644 --- a/src/components/datagrid/body/cell/data_grid_cell.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell.tsx @@ -501,10 +501,43 @@ export class EuiDataGridCell extends Component< } }; + handleCellKeyDown = (event: KeyboardEvent) => { + if (this.isExpandable()) { + if (this.isPopoverOpen()) return; + const { + popoverContext: { openCellPopover }, + visibleRowIndex, + colIndex, + } = this.props; + + switch (event.key) { + case keys.ENTER: + case keys.F2: + event.preventDefault(); + openCellPopover({ rowIndex: visibleRowIndex, colIndex }); + break; + } + } + }; + + handleCellExpansionClick = () => { + const { + popoverContext: { openCellPopover, closeCellPopover }, + visibleRowIndex, + colIndex, + } = this.props; + + if (this.isPopoverOpen()) { + closeCellPopover(); + } else { + openCellPopover({ rowIndex: visibleRowIndex, colIndex }); + } + }; + render() { const { width, - popoverContext: { closeCellPopover, openCellPopover }, + popoverContext, interactiveCellId, columnType, className, @@ -557,21 +590,6 @@ export class EuiDataGridCell extends Component< ...cellPropsStyle, // apply anything from setCellProps({ style }) }; - const handleCellKeyDown = (event: KeyboardEvent) => { - if (isExpandable) { - if (popoverIsOpen) { - return; - } - switch (event.key) { - case keys.ENTER: - case keys.F2: - event.preventDefault(); - openCellPopover({ rowIndex: visibleRowIndex, colIndex }); - break; - } - } - }; - const rowHeight = rowHeightUtils?.getRowHeightOption( rowIndex, rowHeightsOptions @@ -595,43 +613,6 @@ export class EuiDataGridCell extends Component< ariaRowIndex, }; - const cellActions = isExpandable ? ( - <> - { - if (popoverIsOpen) { - closeCellPopover(); - } else { - openCellPopover({ rowIndex: visibleRowIndex, colIndex }); - } - }} - /> - {/* Give the cell expansion popover a separate div/ref - otherwise the - extra popover wrappers mess up the absolute positioning and cause - animation stuttering */} -
- - ) : undefined; - - const cellContent = ( - - - - ); - const cell = (
- {cellContent} + + + + {/* Give the cell expansion popover a separate div/ref - otherwise the + extra popover wrappers mess up the absolute positioning and cause + animation stuttering */} +
+ + ) + } + /> +
); From b45ed1a7ed48e9159c4c0d03e700c5cf7230b2db Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 16 Jan 2024 19:37:15 -0800 Subject: [PATCH 2/5] Go back to previous approach of only rendering cell actions for hovered or focus cells - appears to be significantly more performant --- .../datagrid/body/cell/data_grid_cell.test.tsx | 2 +- src/components/datagrid/body/cell/data_grid_cell.tsx | 12 +++++++++++- src/components/datagrid/data_grid_types.ts | 1 + 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/components/datagrid/body/cell/data_grid_cell.test.tsx b/src/components/datagrid/body/cell/data_grid_cell.test.tsx index 1e3db47cb5b..2e02393b49c 100644 --- a/src/components/datagrid/body/cell/data_grid_cell.test.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell.test.tsx @@ -83,7 +83,7 @@ describe('EuiDataGridCell', () => { /> ); act(() => { - component.setState({ enableInteractions: true }); + component.setState({ isHovered: true }); }); const getCellActions = () => component.find('EuiDataGridCellActions'); diff --git a/src/components/datagrid/body/cell/data_grid_cell.tsx b/src/components/datagrid/body/cell/data_grid_cell.tsx index 1cc80e45a04..58e59bcd437 100644 --- a/src/components/datagrid/body/cell/data_grid_cell.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell.tsx @@ -159,6 +159,7 @@ export class EuiDataGridCell extends Component< state: EuiDataGridCellState = { cellProps: {}, isFocused: false, + isHovered: false, cellTextAlign: 'Left', }; unsubscribeCell?: Function; @@ -378,6 +379,7 @@ export class EuiDataGridCell extends Component< if (nextState.cellProps !== this.state.cellProps) return true; if (nextState.isFocused !== this.state.isFocused) return true; + if (nextState.isHovered !== this.state.isHovered) return true; return false; } @@ -534,6 +536,9 @@ export class EuiDataGridCell extends Component< } }; + onMouseEnter = () => this.setState({ isHovered: true }); + onMouseLeave = () => this.setState({ isHovered: false }); + render() { const { width, @@ -553,6 +558,9 @@ export class EuiDataGridCell extends Component< const isExpandable = this.isExpandable(); const popoverIsOpen = this.isPopoverOpen(); + const showCellActions = + isExpandable && + (popoverIsOpen || this.state.isFocused || this.state.isHovered); const cellClasses = classNames( 'euiDataGridRowCell', @@ -627,6 +635,8 @@ export class EuiDataGridCell extends Component< 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 onKeyDown={this.handleCellKeyDown} + onMouseEnter={this.onMouseEnter} + onMouseLeave={this.onMouseLeave} > Date: Tue, 16 Jan 2024 19:37:59 -0800 Subject: [PATCH 3/5] Revert previous test changes - see https://github.com/elastic/eui/pull/7448/commits/21ae96c8743e1784998761f8d44781bab16806ca --- .../body/cell/data_grid_cell_popover.spec.tsx | 24 +++++-------------- .../body/footer/data_grid_footer_row.spec.tsx | 6 ++--- src/components/datagrid/data_grid.test.tsx | 24 +++++++++++-------- 3 files changed, 23 insertions(+), 31 deletions(-) diff --git a/src/components/datagrid/body/cell/data_grid_cell_popover.spec.tsx b/src/components/datagrid/body/cell/data_grid_cell_popover.spec.tsx index a1bd45cdd36..cf02b7a1eee 100644 --- a/src/components/datagrid/body/cell/data_grid_cell_popover.spec.tsx +++ b/src/components/datagrid/body/cell/data_grid_cell_popover.spec.tsx @@ -53,9 +53,7 @@ describe('EuiDataGridCellPopover', () => { '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]') - .filter(':visible') - .click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); cy.focused().should( 'have.attr', 'data-test-subj', @@ -75,9 +73,7 @@ describe('EuiDataGridCellPopover', () => { '[data-gridcell-row-index="1"][data-gridcell-column-index="1"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]') - .filter(':visible') - .click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); cy.focused().should( 'have.attr', 'data-test-subj', @@ -112,9 +108,7 @@ describe('EuiDataGridCellPopover', () => { }); it('when the cell expand action button is clicked', () => { - cy.get('[data-test-subj="euiDataGridCellExpandButton"]') - .filter(':visible') - .click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should( 'not.exist' ); @@ -156,12 +150,8 @@ describe('EuiDataGridCellPopover', () => { cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist'); // Close and re-open the cell popover by clicking - cy.get('[data-test-subj="euiDataGridCellExpandButton"]') - .filter(':visible') - .click(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]') - .filter(':visible') - .click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); cy.get('[data-test-subj="cellActionB"]').first().realClick(); cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist'); @@ -201,9 +191,7 @@ describe('EuiDataGridCellPopover', () => { cy.get( '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]') - .filter(':visible') - .click(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); cy.get('.euiDataGridRowCell__popover.hello.world').should('exist'); }); diff --git a/src/components/datagrid/body/footer/data_grid_footer_row.spec.tsx b/src/components/datagrid/body/footer/data_grid_footer_row.spec.tsx index dddd82bb296..5dc1a7b7f3f 100644 --- a/src/components/datagrid/body/footer/data_grid_footer_row.spec.tsx +++ b/src/components/datagrid/body/footer/data_grid_footer_row.spec.tsx @@ -65,9 +65,9 @@ describe('EuiDataGridFooterRow', () => { cy.get( '[data-gridcell-column-index="0"][data-gridcell-row-index="3"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]') - .filter(':visible') - .should('not.exist'); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').should( + 'not.exist' + ); }); // Regression test for #5720 diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index 49120c60c4d..2498ddae5b5 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -1316,7 +1316,7 @@ describe('EuiDataGrid', () => { const component = mount( {}, @@ -2164,17 +2164,21 @@ describe('EuiDataGrid', () => { /> ); - // cell buttons should be `display: none` for unfocused, unhovered cell - expect( - findTestSubject(component, 'alertAction').last().getDOMNode() - ).not.toBeVisible(); - expect( - findTestSubject(component, 'happyAction').last().getDOMNode() - ).not.toBeVisible(); + // cell buttons should not get rendered for unfocused, unhovered cell + expect(findTestSubject(component, 'alertAction').exists()).toBe(false); + expect(findTestSubject(component, 'happyAction').exists()).toBe(false); + + act(() => { + findTestSubject(component, 'dataGridRowCell') + .at(1) + .prop('onMouseEnter')!({} as React.MouseEvent); + }); + + component.update(); - findTestSubject(component, 'alertAction').at(1).simulate('click'); + findTestSubject(component, 'alertAction').at(0).simulate('click'); expect(alertFn).toHaveBeenCalledWith(1, 'A'); - findTestSubject(component, 'happyAction').at(1).simulate('click'); + findTestSubject(component, 'happyAction').at(0).simulate('click'); expect(happyFn).toHaveBeenCalledWith(1, 'A'); alertFn.mockReset(); happyFn.mockReset(); From c1a01393311c33d4e8480ff5afe0a356d779fe6e Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 16 Jan 2024 19:38:06 -0800 Subject: [PATCH 4/5] Update snapshots --- .../__snapshots__/data_grid.test.tsx.snap | 576 ------------------ .../data_grid_body_custom.test.tsx.snap | 96 --- .../data_grid_body_virtualized.test.tsx.snap | 48 -- .../data_grid_cell.test.tsx.snap | 24 - src/components/datagrid/data_grid.test.tsx | 8 + 5 files changed, 8 insertions(+), 744 deletions(-) diff --git a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap index cfbf6e00f0d..e50e7a79f1a 100644 --- a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap +++ b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap @@ -725,31 +725,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` > - A, column 1, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 2, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
- A, column 1, row 2 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 2, row 2 - . - Press the Enter key to expand this cell.

-
- -
-
- A, column 1, row 3 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 2, row 3 - . - Press the Enter key to expand this cell.

-
- -
-
@@ -1360,31 +1216,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` > - A, column 2, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 3, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
- A, column 2, row 2 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 3, row 2 - . - Press the Enter key to expand this cell.

-
- -
-
- A, column 2, row 3 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 3, row 3 - . - Press the Enter key to expand this cell.

-
- -
-
- A, column 1, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 2, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
- A, column 1, row 2 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 2, row 2 - . - Press the Enter key to expand this cell.

-
- -
-
- A, column 1, row 3 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 2, row 3 - . - Press the Enter key to expand this cell.

-
- -
-
@@ -2628,31 +2196,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` > - A, column 1, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 2, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
- A, column 1, row 2 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 2, row 2 - . - Press the Enter key to expand this cell.

-
- -
-
- A, column 1, row 3 - . - Press the Enter key to expand this cell.

-
- -
-
- B, column 2, row 3 - . - Press the Enter key to expand this cell.

-
- -
-
diff --git a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap index 73d35ea0a01..f5c3ea2d80a 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap @@ -143,31 +143,7 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render > - columnA, column 1, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
- columnB, column 2, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
- columnA, column 1, row 2 - . - Press the Enter key to expand this cell.

-
- -
-
- columnB, column 2, row 2 - . - Press the Enter key to expand this cell.

-
- -
-
diff --git a/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap index 776c1e879b4..3a2f6df3422 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap @@ -146,31 +146,7 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = ` > - columnA, column 1, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
- columnB, column 2, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
diff --git a/src/components/datagrid/body/cell/__snapshots__/data_grid_cell.test.tsx.snap b/src/components/datagrid/body/cell/__snapshots__/data_grid_cell.test.tsx.snap index 075243d94b9..b21135c6442 100644 --- a/src/components/datagrid/body/cell/__snapshots__/data_grid_cell.test.tsx.snap +++ b/src/components/datagrid/body/cell/__snapshots__/data_grid_cell.test.tsx.snap @@ -71,30 +71,6 @@ exports[`EuiDataGridCell renders 1`] = ` > - someColumn, column 1, row 1 - . - Press the Enter key to expand this cell.

-
- -
-
`; diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index 2498ddae5b5..c0fd34833b7 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -547,6 +547,8 @@ describe('EuiDataGrid', () => { "data-gridcell-visible-row-index": 0, "data-test-subj": "dataGridRowCell", "onKeyDown": [Function], + "onMouseEnter": [Function], + "onMouseLeave": [Function], "role": "gridcell", "style": Object { "color": "red", @@ -569,6 +571,8 @@ describe('EuiDataGrid', () => { "data-gridcell-visible-row-index": 0, "data-test-subj": "dataGridRowCell", "onKeyDown": [Function], + "onMouseEnter": [Function], + "onMouseLeave": [Function], "role": "gridcell", "style": Object { "color": "blue", @@ -591,6 +595,8 @@ describe('EuiDataGrid', () => { "data-gridcell-visible-row-index": 1, "data-test-subj": "dataGridRowCell", "onKeyDown": [Function], + "onMouseEnter": [Function], + "onMouseLeave": [Function], "role": "gridcell", "style": Object { "color": "red", @@ -613,6 +619,8 @@ describe('EuiDataGrid', () => { "data-gridcell-visible-row-index": 1, "data-test-subj": "dataGridRowCell", "onKeyDown": [Function], + "onMouseEnter": [Function], + "onMouseLeave": [Function], "role": "gridcell", "style": Object { "color": "blue", From 6e1f26825506103afafc66df844e88877a97e302 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 16 Jan 2024 19:43:22 -0800 Subject: [PATCH 5/5] changelog --- changelogs/upcoming/7465.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/upcoming/7465.md diff --git a/changelogs/upcoming/7465.md b/changelogs/upcoming/7465.md new file mode 100644 index 00000000000..1c5d6166615 --- /dev/null +++ b/changelogs/upcoming/7465.md @@ -0,0 +1,3 @@ +**Bug fixes** + +- Minor `EuiDataGrid` cell performance fixes