[EuiDataGrid] Fix cell popovers not closing on outside grid click (#7387

)
elastic · Nov 29, 2023 · 5ab8005 · 5ab8005
1 parent 88cb79c
commit 5ab8005
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 20 deletions.
diff --git 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
@@ -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
@@ -87,18 +87,83 @@ describe('EuiDataGridCellPopover', () => {
     });
   });
 
-  it('closes the cell popover when the originating cell is clicked', () => {
-    cy.realMount(<EuiDataGrid {...baseProps} />);
-    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(<EuiDataGrid {...baseProps} />);
+      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
+          iconType="plusInCircle"
+          aria-label="A"
+          data-test-subj="cellActionA"
+        />
+      ),
+      ({ Component }) => (
+        <Component
+          iconType="minusInCircle"
+          aria-label="B"
+          data-test-subj="cellActionB"
+        />
+      ),
+    ];
+    cy.realMount(
+      <EuiDataGrid {...baseProps} columns={[{ id: 'A', cellActions }]} />
+    );
+    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
@@ -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
@@ -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}