Fix cell popovers not closing on outside grid click

- I had the DOM markup/hierarchy wrong, so the cell popover was never closing + add bonus regresison test for ensuring that clicking other cell actions does not close the popover
elastic · Nov 23, 2023 · a86a523 · a86a523
1 parent 9ae02c4
commit a86a523
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 15 deletions.
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,21 +87,74 @@ 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');
+    });
 
-    cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click();
-    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'
+      );
+    });
 
-    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');
   });
 
   it('allows consumers to use setCellPopoverProps, passed from renderCellPopover, to customize popover props', () => {

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();
       }
     },