From 5ab8005cc8bd7e5aee73aa80b71f2659907d6077 Mon Sep 17 00:00:00 2001 From: Cee Chen <549407+cee-chen@users.noreply.github.com> Date: Wed, 29 Nov 2023 08:18:27 -0800 Subject: [PATCH] [EuiDataGrid] Fix cell popovers not closing on outside grid click (#7387) --- .../datagrid/_data_grid_data_row.scss | 6 +- .../datagrid/body/data_grid_cell.tsx | 4 - .../body/data_grid_cell_popover.spec.tsx | 81 +++++++++++++++++-- .../body/data_grid_cell_popover.test.tsx | 1 + .../datagrid/body/data_grid_cell_popover.tsx | 20 +++-- 5 files changed, 92 insertions(+), 20 deletions(-) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index f3287fd86f5..93ea8d1bcb5 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -57,7 +57,9 @@ &:focus, &:focus-within, // Always make the cell actions visible when the cell popover is open - &.euiDataGridRowCell--open { + &.euiDataGridRowCell--open, + // Prevents the animation from replaying when keyboard focus is moved from the popover back to the cell + &[data-keyboard-closing] { .euiDataGridRowCell__actions { animation-duration: $euiAnimSpeedExtraFast; animation-name: euiDataGridCellActionsSlideIn; @@ -67,7 +69,7 @@ } // if a cell is not hovered nor focused nor open via popover, don't show buttons in general - &:not(:hover):not(:focus):not(.euiDataGridRowCell--open) { + &:not(:hover):not(:focus):not(.euiDataGridRowCell--open):not([data-keyboard-closing]) { .euiDataGridRowCell__actions { display: none; } diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index 0021a8cfc2f..61ad48987c0 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -457,10 +457,6 @@ export class EuiDataGridCell extends Component< this.setState({ disableCellTabIndex: true }); } }, 0); - // Close the cell popover if the popover was open and the user clicked the cell - if (this.props.popoverContext.popoverIsOpen) { - this.props.popoverContext.closeCellPopover(); - } } }; diff --git a/src/components/datagrid/body/data_grid_cell_popover.spec.tsx b/src/components/datagrid/body/data_grid_cell_popover.spec.tsx index 657e542661a..53e5e49982b 100644 --- a/src/components/datagrid/body/data_grid_cell_popover.spec.tsx +++ b/src/components/datagrid/body/data_grid_cell_popover.spec.tsx @@ -87,18 +87,83 @@ describe('EuiDataGridCellPopover', () => { }); }); - it('closes the cell popover when the originating cell is clicked', () => { - cy.realMount(); - cy.get( - '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' - ).realClick(); + describe('closes the cell popover', () => { + // Mount and open the first cell popover + beforeEach(() => { + cy.realMount(); + cy.get( + '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' + ).click(); + cy.realPress('Enter'); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist'); + }); + it('when the originating cell is clicked', () => { + cy.get( + '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' + ).realClick({ position: 'right' }); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should( + 'not.exist' + ); + }); + + it('when the cell expand action button is clicked', () => { + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should( + 'not.exist' + ); + }); + + it('when anywhere outside the grid is clicked', () => { + cy.get('body').realClick({ position: 'bottomRight' }); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should( + 'not.exist' + ); + }); + }); + + it('does not close the cell popover when other cell actions are clicked', () => { + const cellActions = [ + ({ Component }) => ( + + ), + ({ Component }) => ( + + ), + ]; + cy.realMount( + + ); + cy.get('[data-test-subj="dataGridRowCell"]').first().click(); + cy.realPress('Enter'); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist'); + + cy.get('[data-test-subj="cellActionA"]').first().realClick(); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist'); + + // Close and re-open the cell popover by clicking + 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'); - cy.get( - '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' - ).realClick({ position: 'right' }); + // Clicking the cell actions outside the popover should not have disabled the focus trap + cy.repeatRealPress('Tab', 3); + cy.focused().should( + 'have.attr', + 'data-test-subj', + 'euiDataGridExpansionPopover' + ); + cy.get('body').realClick({ position: 'bottomRight' }); cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should( 'not.exist' ); diff --git a/src/components/datagrid/body/data_grid_cell_popover.test.tsx b/src/components/datagrid/body/data_grid_cell_popover.test.tsx index 2db3d3970fa..360b5e86157 100644 --- a/src/components/datagrid/body/data_grid_cell_popover.test.tsx +++ b/src/components/datagrid/body/data_grid_cell_popover.test.tsx @@ -101,6 +101,7 @@ describe('useCellPopover', () => { display="block" focusTrapProps={ Object { + "clickOutsideDisables": false, "onClickOutside": [Function], } } diff --git a/src/components/datagrid/body/data_grid_cell_popover.tsx b/src/components/datagrid/body/data_grid_cell_popover.tsx index e7f2e4f0d0a..66df03f09d4 100644 --- a/src/components/datagrid/body/data_grid_cell_popover.tsx +++ b/src/components/datagrid/body/data_grid_cell_popover.tsx @@ -88,9 +88,9 @@ export const useCellPopover = (): { // clicking the expansion cell action triggers an outside click const onClickOutside = useCallback( (event: Event) => { - if (!popoverAnchor) return; - const cellActions = popoverAnchor.previousElementSibling; - if (cellActions?.contains(event.target as Node) === false) { + const cellActions = + popoverAnchor?.parentElement?.parentElement?.previousElementSibling; + if (!cellActions?.contains(event.target as Node)) { closeCellPopover(); } }, @@ -107,7 +107,7 @@ export const useCellPopover = (): { anchorPosition={popoverAnchorPosition} repositionToCrossAxis={false} {...cellPopoverProps} - focusTrapProps={{ onClickOutside }} + focusTrapProps={{ onClickOutside, clickOutsideDisables: false }} panelProps={{ 'data-test-subj': 'euiDataGridExpansionPopover', ...(cellPopoverProps.panelProps || {}), @@ -128,8 +128,16 @@ export const useCellPopover = (): { event.preventDefault(); event.stopPropagation(); closeCellPopover(); - // Ensure focus is returned to the parent cell - requestAnimationFrame(() => popoverAnchor.parentElement!.focus()); + const cell = + popoverAnchor.parentElement?.parentElement?.parentElement; + + // Prevent cell animation flash while focus is being shifted between popover and cell + cell?.setAttribute('data-keyboard-closing', 'true'); + // Ensure focus is returned to the parent cell, and remove animation stopgap + requestAnimationFrame(() => { + popoverAnchor.parentElement!.focus(); + cell?.removeAttribute('data-keyboard-closing'); + }); } }} button={popoverAnchor}