[PR feedback] Fix focus fighting bug + add E2E regression test for be…

…havior
elastic · Jan 5, 2024 · d6c70f6 · d6c70f6
1 parent 41ed92e
commit d6c70f6
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 6 deletions.
diff --git a/src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx b/src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx
@@ -52,6 +52,7 @@ describe('EuiDataGridHeaderCellWrapper', () => {
           data-gridcell-row-index="-1"
           data-gridcell-visible-row-index="-1"
           data-test-subj="dataGridHeaderCell-someColumn"
+          onFocus={[Function]}
           role="columnheader"
           style={Object {}}
           tabIndex={0}
@@ -100,6 +101,7 @@ describe('EuiDataGridHeaderCellWrapper', () => {
         data-gridcell-row-index="-1"
         data-gridcell-visible-row-index="-1"
         data-test-subj="dataGridHeaderCell-someColumn"
+        onFocus={[Function]}
         role="columnheader"
         style={
           Object {

diff --git a/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx b/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx
@@ -9,6 +9,7 @@
 import classnames from 'classnames';
 import React, {
   FunctionComponent,
+  FocusEventHandler,
   useContext,
   useEffect,
   useRef,
@@ -64,12 +65,23 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent<
     }
   }, [isFocused]);
 
+  // For cell headers with actions, auto-focus into the button instead of the cell wrapper div
+  // The button text is significantly more useful to screen readers (e.g. contains sort order & hints)
+  const onFocus: FocusEventHandler = useCallback(
+    (e) => {
+      if (hasActionsPopover && e.target === headerRef.current) {
+        focusActionsButton?.();
+      }
+    },
+    [hasActionsPopover, focusActionsButton]
+  );
+
   return (
     <div
       role="columnheader"
       ref={headerRef}
       tabIndex={isFocused && !isActionsButtonFocused ? 0 : -1}
-      onFocus={hasActionsPopover ? focusActionsButton : undefined}
+      onFocus={onFocus}
       className={classes}
       data-test-subj={`dataGridHeaderCell-${id}`}
       data-gridcell-column-id={id}

diff --git a/src/components/datagrid/data_grid.spec.tsx b/src/components/datagrid/data_grid.spec.tsx
@@ -139,29 +139,24 @@ describe('EuiDataGrid', () => {
       {
         id: 'no_interactive_expandable',
         display: '0 interactive',
-        actions: false,
       },
       {
         id: 'one_interactive',
         display: '1 interactive',
         isExpandable: false,
-        actions: false,
       },
       {
         id: 'one_interactive_expandable',
         display: '1 interactive',
-        actions: false,
       },
       {
         id: 'two_interactives',
         display: '2 interactives',
         isExpandable: false,
-        actions: false,
       },
       {
         id: 'two_interactives_expandable',
         display: '2 interactives',
-        actions: false,
       },
     ];
     const columnVisibility = {
@@ -436,6 +431,32 @@ describe('EuiDataGrid', () => {
           .should('have.attr', 'data-gridcell-column-index', '5')
           .should('have.attr', 'data-gridcell-row-index', '0');
       });
+
+      it('column header cells', () => {
+        cy.realMount(<EuiDataGrid {...focusManagementBaseProps} />);
+        cy.repeatRealPress('Tab', 5);
+        cy.realPress('{rightarrow}');
+
+        // Should auto-focus the actions button (over the cell itself)
+        cy.focused()
+          .parent()
+          .should('have.attr', 'data-gridcell-column-index', '1')
+          .should('have.attr', 'data-gridcell-row-index', '-1');
+
+        // Pressing enter should toggle the actions popover
+        cy.realPress('Enter');
+        cy.get(
+          '[data-test-subj="dataGridHeaderCellActionGroup-no_interactive_expandable"]'
+        ).should('be.visible');
+
+        // The actions popover should be fully tabbable/focus trapped with no regressions
+        cy.realPress('Tab');
+        cy.focused().should('have.text', 'Hide column');
+        cy.realPress('Tab');
+        cy.focused().should('have.text', 'Move left');
+        cy.realPress('Tab');
+        cy.focused().should('have.text', 'Move right');
+      });
     });
   });