From ee6507288a7be576e26d075bf95216616c3ca03f Mon Sep 17 00:00:00 2001
From: Constance Chen <constance.chen.3@gmail.com>
Date: Tue, 1 Mar 2022 13:16:55 -0800
Subject: [PATCH 1/4] [misc setup] refactor cell actions unit tests to use mock
 component

- this allows us to `component.find()` by the 'MockAction' name, which future unit tests will be using to determine the # of actions rendered
---
 .../body/data_grid_cell_actions.test.tsx      | 24 ++++++++++---------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/components/datagrid/body/data_grid_cell_actions.test.tsx b/src/components/datagrid/body/data_grid_cell_actions.test.tsx
index eb4056ffe02..2039e3eb448 100644
--- a/src/components/datagrid/body/data_grid_cell_actions.test.tsx
+++ b/src/components/datagrid/body/data_grid_cell_actions.test.tsx
@@ -9,11 +9,16 @@
 import React from 'react';
 import { shallow } from 'enzyme';
 
+import { EuiDataGridColumnCellAction } from '../data_grid_types';
 import {
   EuiDataGridCellActions,
   EuiDataGridCellPopoverActions,
 } from './data_grid_cell_actions';
 
+const MockAction: EuiDataGridColumnCellAction = ({ Component }) => (
+  <Component iconType="starEmpty" data-test-subj="mockCellAction" />
+);
+
 describe('EuiDataGridCellActions', () => {
   const requiredProps = {
     closePopover: jest.fn(),
@@ -62,7 +67,7 @@ describe('EuiDataGridCellActions', () => {
       <EuiDataGridCellActions
         {...requiredProps}
         isExpandable={false}
-        column={{ id: 'someId', cellActions: [() => <button />] }}
+        column={{ id: 'someId', cellActions: [MockAction] }}
       />
     );
 
@@ -70,7 +75,7 @@ describe('EuiDataGridCellActions', () => {
       <div
         className="euiDataGridRowCell__expandActions"
       >
-        <Component
+        <MockAction
           Component={[Function]}
           closePopover={[MockFunction]}
           colIndex={0}
@@ -98,7 +103,7 @@ describe('EuiDataGridCellActions', () => {
       <EuiDataGridCellActions
         {...requiredProps}
         isExpandable={true}
-        column={{ id: 'someId', cellActions: [() => <button />] }}
+        column={{ id: 'someId', cellActions: [MockAction] }}
       />
     );
 
@@ -106,7 +111,7 @@ describe('EuiDataGridCellActions', () => {
       <div
         className="euiDataGridRowCell__expandActions"
       >
-        <Component
+        <MockAction
           Component={[Function]}
           closePopover={[MockFunction]}
           colIndex={0}
@@ -133,7 +138,7 @@ describe('EuiDataGridCellPopoverActions', () => {
       <EuiDataGridCellPopoverActions
         colIndex={0}
         rowIndex={0}
-        column={{ id: 'someId', cellActions: [() => <button />] }}
+        column={{ id: 'someId', cellActions: [MockAction] }}
       />
     );
 
@@ -147,7 +152,7 @@ describe('EuiDataGridCellPopoverActions', () => {
           <EuiFlexItem
             key="0"
           >
-            <Component
+            <MockAction
               Component={[Function]}
               colIndex={0}
               columnId="someId"
@@ -159,11 +164,8 @@ describe('EuiDataGridCellPopoverActions', () => {
       </EuiPopoverFooter>
     `);
 
-    const button = component
-      .childAt(0)
-      .childAt(0)
-      .childAt(0) // .find('Component') doesn't work, for whatever reason
-      .renderProp('Component');
+    const action = component.find('MockAction') as any;
+    const button = action.renderProp('Component');
     expect(button({ iconType: 'function' })).toMatchInlineSnapshot(`
       <EuiButtonEmpty
         iconType="function"

From da8eb04d541d6aef616dc8b92d70fcfa31ae55bd Mon Sep 17 00:00:00 2001
From: Constance Chen <constance.chen.3@gmail.com>
Date: Tue, 1 Mar 2022 14:17:38 -0800
Subject: [PATCH 2/4] Misc docs cleanup

- Add intro paragraph to cell popovers page, explaining the UX purpose of the expansion popover

- Fix incorrect cellActions example in main datagrid doc

- Fix variable casing on source code
---
 src-docs/src/views/datagrid/_snippets.tsx     | 14 ++++-------
 .../datagrid_cell_popover_example.js          |  5 ++--
 .../cells_popovers/datagrid_cells_example.js  | 24 +++++++++++++++++++
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/src-docs/src/views/datagrid/_snippets.tsx b/src-docs/src/views/datagrid/_snippets.tsx
index 428426876f8..fb0d3384e6c 100644
--- a/src-docs/src/views/datagrid/_snippets.tsx
+++ b/src-docs/src/views/datagrid/_snippets.tsx
@@ -15,16 +15,12 @@ inMemory={{ level: 'sorting' }}`,
     actions: { showMoveLeft: false, showMoveRight: false }, // doesn't show move actions in column header
     schema: 'franchise', // custom schema later defined under schemaDetectors
     cellActions: [ // provides one additional cell action that triggers an alert once clicked
-      {
-        label: 'test',
-        iconType: 'heart',
-        callback: ()=> alert('test')
-      }
-    ]
-  }
+      ({ Component }) => <Component iconType="heart" onClick={() => alert('test')}>Custom action</Component>,
+    ],
+  },
 ]}`,
   columnVisibility: `columnVisibility={{
-  visibleColumns: ['A', 'C'],
+  visibleColumns: ['A'],
   setVisibleColumns: () => {},
 }}`,
   leadingControlColumns: `leadingControlColumns={[
@@ -61,7 +57,7 @@ inMemory={{ level: 'sorting' }}`,
   onChangeItemsPerPage: () => {},
 }}`,
   sorting: `sorting={{
-  columns: [{ id: 'C', direction: 'asc' }],
+  columns: [{ id: 'A', direction: 'asc' }],
   onSort: () => {},
 }}`,
   toolbarVisibility: `toolbarVisibility={{
diff --git a/src-docs/src/views/datagrid/cells_popovers/datagrid_cell_popover_example.js b/src-docs/src/views/datagrid/cells_popovers/datagrid_cell_popover_example.js
index 9d6e6c323ab..dc3a264359c 100644
--- a/src-docs/src/views/datagrid/cells_popovers/datagrid_cell_popover_example.js
+++ b/src-docs/src/views/datagrid/cells_popovers/datagrid_cell_popover_example.js
@@ -4,7 +4,7 @@ import { GuideSectionTypes } from '../../../components';
 import { EuiCode, EuiCallOut, EuiSpacer } from '../../../../../src';
 
 import IsDetailsPopover from './cell_popover_is_details';
-const IsDetailsPopoverSource = require('!!raw-loader!./cell_popover_is_details');
+const isDetailsPopoverSource = require('!!raw-loader!./cell_popover_is_details');
 
 import RenderCellPopover from './cell_popover_rendercellpopover';
 const renderCellPopoverSource = require('!!raw-loader!./cell_popover_rendercellpopover');
@@ -19,7 +19,6 @@ import {
 } from '!!prop-loader!../../../../../src/components/datagrid/data_grid_types';
 
 export const DataGridCellPopoverExample = {
-  title: 'Data grid cell popovers',
   sections: [
     {
       title: 'Conditionally customizing cell popover content',
@@ -48,7 +47,7 @@ export const DataGridCellPopoverExample = {
       source: [
         {
           type: GuideSectionTypes.TSX,
-          code: IsDetailsPopoverSource,
+          code: isDetailsPopoverSource,
         },
       ],
       props: {
diff --git a/src-docs/src/views/datagrid/cells_popovers/datagrid_cells_example.js b/src-docs/src/views/datagrid/cells_popovers/datagrid_cells_example.js
index b0e2b83a0eb..c5e7bff6f9f 100644
--- a/src-docs/src/views/datagrid/cells_popovers/datagrid_cells_example.js
+++ b/src-docs/src/views/datagrid/cells_popovers/datagrid_cells_example.js
@@ -1,4 +1,5 @@
 import React, { Fragment } from 'react';
+import { Link } from 'react-router-dom';
 
 import { GuideSectionTypes } from '../../../components';
 import {
@@ -7,6 +8,7 @@ import {
   EuiCallOut,
   EuiBasicTable,
   EuiSpacer,
+  EuiText,
   EuiListGroupItem,
 } from '../../../../../src';
 
@@ -27,6 +29,28 @@ import {
 
 export const DataGridCellsExample = {
   title: 'Data grid cells & popovers',
+  intro: (
+    <EuiText>
+      <p>
+        Data grid cells are rendered using the{' '}
+        <EuiCode>renderCellValue</EuiCode> prop. This prop accepts a function
+        which is treated as a React component behind the scenes. This means the
+        data grid passes cell-specific props (e.g. row index, column/schema
+        info, etc.) to your render function, which can ouput anything from a
+        string to any JSX element.
+      </p>
+      <p>
+        Each data grid cell will automatically render an expand action button
+        and an expansion popover (
+        <Link to="#disabling-cell-expansion-popovers">
+          which can be disabled
+        </Link>
+        ). For cells with overflowing or truncated content, this expansion
+        popover helps display the full amount of content, or can be customized
+        to show extra details.
+      </p>
+    </EuiText>
+  ),
   sections: [
     {
       source: [

From 0ca49a8347a3713d4dff24aa85b6d8600adc2e83 Mon Sep 17 00:00:00 2001
From: Constance Chen <constance.chen.3@gmail.com>
Date: Tue, 1 Mar 2022 14:14:14 -0800
Subject: [PATCH 3/4] Force cell isExpandable to evaluate as true if
 cellActions exist

+ add more comments/documentation to note this behavior since I missed it the first time

+ simplify hasCellActions to just use isExpandable, now that isExpandable accounts for cellActions
---
 .../cell_popover_is_expandable.tsx            |  2 +-
 .../datagrid_cell_popover_example.js          | 19 ++++++++++++----
 .../datagrid/body/data_grid_cell.test.tsx     | 12 ++++++++++
 .../datagrid/body/data_grid_cell.tsx          |  8 ++++---
 .../body/data_grid_cell_actions.test.tsx      | 22 +------------------
 .../datagrid/body/data_grid_cell_actions.tsx  | 10 +++++----
 6 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/src-docs/src/views/datagrid/cells_popovers/cell_popover_is_expandable.tsx b/src-docs/src/views/datagrid/cells_popovers/cell_popover_is_expandable.tsx
index 499b457423f..994ed4148f5 100644
--- a/src-docs/src/views/datagrid/cells_popovers/cell_popover_is_expandable.tsx
+++ b/src-docs/src/views/datagrid/cells_popovers/cell_popover_is_expandable.tsx
@@ -28,7 +28,7 @@ const columns: EuiDataGridColumn[] = [
   },
   {
     id: 'lastName',
-    isExpandable: false,
+    isExpandable: false, // Overridden by the fact that cellActions is set
     cellActions,
   },
   {
diff --git a/src-docs/src/views/datagrid/cells_popovers/datagrid_cell_popover_example.js b/src-docs/src/views/datagrid/cells_popovers/datagrid_cell_popover_example.js
index dc3a264359c..65bf4acf642 100644
--- a/src-docs/src/views/datagrid/cells_popovers/datagrid_cell_popover_example.js
+++ b/src-docs/src/views/datagrid/cells_popovers/datagrid_cell_popover_example.js
@@ -1,4 +1,5 @@
 import React from 'react';
+import { Link } from 'react-router-dom';
 
 import { GuideSectionTypes } from '../../../components';
 import { EuiCode, EuiCallOut, EuiSpacer } from '../../../../../src';
@@ -136,10 +137,10 @@ export const DataGridCellPopoverExample = {
       text: (
         <>
           <p>
-            Popovers can sometimes be unnecessary for short form content. In the
-            example below we&apos;ve turned them off by setting{' '}
-            <EuiCode>isExpandable=false</EuiCode> on specific{' '}
-            <EuiCode>columns</EuiCode>.
+            Popovers can sometimes be unnecessary for short form content, and
+            can be disabled by setting <EuiCode>columns.isExpandable</EuiCode>{' '}
+            to <EuiCode>false</EuiCode>. In the example below, we&apos;ve turned
+            off expansion on the suffix column.
           </p>
           <p>
             To set <EuiCode>isExpandable</EuiCode> at a per-cell level instead
@@ -148,6 +149,16 @@ export const DataGridCellPopoverExample = {
             example conditionally disables the expansion popover for boolean
             cells that are &apos;false&apos;.
           </p>
+          <EuiCallOut
+            color="warning"
+            iconType="alert"
+            title="Cells with actions are always expandable"
+          >
+            If <EuiCode>columns.cellActions</EuiCode> is defined,{' '}
+            <EuiCode>isExpandable</EuiCode> will always be forced to true. This
+            ensures that keyboard and screen reader users have access to all
+            cell actions.
+          </EuiCallOut>
         </>
       ),
       demo: <IsExpandablePopover />,
diff --git a/src/components/datagrid/body/data_grid_cell.test.tsx b/src/components/datagrid/body/data_grid_cell.test.tsx
index 4355fd8dffc..31ca2e5e20e 100644
--- a/src/components/datagrid/body/data_grid_cell.test.tsx
+++ b/src/components/datagrid/body/data_grid_cell.test.tsx
@@ -391,6 +391,18 @@ describe('EuiDataGridCell', () => {
   });
 
   describe('isExpandable', () => {
+    it('always returns true if column.cellActions exists', () => {
+      const component = mount(
+        <EuiDataGridCell
+          {...requiredProps}
+          column={{ id: 'someId', cellActions: [() => <button />] }}
+          isExpandable={false}
+        />
+      );
+
+      expect(component.find('renderCellValue').prop('isExpandable')).toBe(true);
+    });
+
     it('falls back to props.isExpandable which is derived from the column config', () => {
       const component = mount(
         <EuiDataGridCell {...requiredProps} isExpandable={true} />
diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx
index a207b63e379..78c6ced31c3 100644
--- a/src/components/datagrid/body/data_grid_cell.tsx
+++ b/src/components/datagrid/body/data_grid_cell.tsx
@@ -429,6 +429,10 @@ export class EuiDataGridCell extends Component<
   };
 
   isExpandable = () => {
+    // A cell must always show an expansion popover if it has cell actions,
+    // otherwise keyboard and screen reader users have no way of accessing them
+    if (this.props.column?.cellActions?.length) return true;
+
     // props.isExpandable inherits from column.isExpandable
     // state.cellProps allows consuming applications to override isExpandable on a per-cell basis
     return this.state.cellProps.isExpandable ?? this.props.isExpandable;
@@ -516,7 +520,6 @@ export class EuiDataGridCell extends Component<
 
     const isExpandable = this.isExpandable();
     const popoverIsOpen = this.isPopoverOpen();
-    const hasCellActions = isExpandable || column?.cellActions;
     const showCellActions =
       this.state.isFocused ||
       this.state.isEntered ||
@@ -652,7 +655,7 @@ export class EuiDataGridCell extends Component<
       </EuiFocusTrap>
     );
 
-    if (hasCellActions) {
+    if (isExpandable) {
       innerContent = (
         <div className={anchorClass} ref={this.popoverAnchorRef}>
           <div className={expandClass}>
@@ -663,7 +666,6 @@ export class EuiDataGridCell extends Component<
               rowIndex={rowIndex}
               colIndex={colIndex}
               column={column}
-              isExpandable={isExpandable}
               closePopover={closeCellPopover}
               onExpandClick={() => {
                 if (popoverIsOpen) {
diff --git a/src/components/datagrid/body/data_grid_cell_actions.test.tsx b/src/components/datagrid/body/data_grid_cell_actions.test.tsx
index 2039e3eb448..c1e77e8a3a4 100644
--- a/src/components/datagrid/body/data_grid_cell_actions.test.tsx
+++ b/src/components/datagrid/body/data_grid_cell_actions.test.tsx
@@ -28,9 +28,7 @@ describe('EuiDataGridCellActions', () => {
   };
 
   it('renders an expand button', () => {
-    const component = shallow(
-      <EuiDataGridCellActions {...requiredProps} isExpandable={true} />
-    );
+    const component = shallow(<EuiDataGridCellActions {...requiredProps} />);
 
     expect(component).toMatchInlineSnapshot(`
       <div
@@ -66,27 +64,10 @@ describe('EuiDataGridCellActions', () => {
     const component = shallow(
       <EuiDataGridCellActions
         {...requiredProps}
-        isExpandable={false}
         column={{ id: 'someId', cellActions: [MockAction] }}
       />
     );
 
-    expect(component).toMatchInlineSnapshot(`
-      <div
-        className="euiDataGridRowCell__expandActions"
-      >
-        <MockAction
-          Component={[Function]}
-          closePopover={[MockFunction]}
-          colIndex={0}
-          columnId="someId"
-          isExpanded={false}
-          key="0"
-          rowIndex={0}
-        />
-      </div>
-    `);
-
     const button = component.childAt(0).renderProp('Component');
     expect(button({ iconType: 'eye' })).toMatchInlineSnapshot(`
       <EuiButtonIcon
@@ -102,7 +83,6 @@ describe('EuiDataGridCellActions', () => {
     const component = shallow(
       <EuiDataGridCellActions
         {...requiredProps}
-        isExpandable={true}
         column={{ id: 'someId', cellActions: [MockAction] }}
       />
     );
diff --git a/src/components/datagrid/body/data_grid_cell_actions.tsx b/src/components/datagrid/body/data_grid_cell_actions.tsx
index ff63b402876..cf2649601c4 100644
--- a/src/components/datagrid/body/data_grid_cell_actions.tsx
+++ b/src/components/datagrid/body/data_grid_cell_actions.tsx
@@ -20,21 +20,23 @@ import { EuiFlexGroup, EuiFlexItem } from '../../flex';
 import { EuiPopoverFooter } from '../../popover';
 
 export const EuiDataGridCellActions = ({
-  isExpandable,
   closePopover,
   onExpandClick,
   column,
   rowIndex,
   colIndex,
 }: {
-  isExpandable: boolean;
   closePopover: () => void;
   onExpandClick: () => void;
   column?: EuiDataGridColumn;
   rowIndex: number;
   colIndex: number;
 }) => {
-  const expandButton = isExpandable ? (
+  // Note: The cell expand button/expansion popover is *always* rendered if
+  // column.cellActions is present (regardless of column.isExpandable).
+  // This is because cell actions are not otherwise accessible to keyboard
+  // or screen reader users
+  const expandButton = (
     <EuiI18n
       key={'expand'}
       token="euiDataGridCellActions.expandButtonTitle"
@@ -54,7 +56,7 @@ export const EuiDataGridCellActions = ({
         />
       )}
     </EuiI18n>
-  ) : null;
+  );
 
   const additionalButtons = useMemo(() => {
     const ButtonComponent = (props: EuiButtonIconProps) => (

From 63cd8d23ff77e78282cd5b67743711bc68f007cc Mon Sep 17 00:00:00 2001
From: Constance Chen <constance.chen.3@gmail.com>
Date: Thu, 10 Mar 2022 10:40:34 -0800
Subject: [PATCH 4/4] Add changelog entry

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 25667bf8d58..4ebb63b0d54 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 - Updated `testenv` mock for `EuiIcon` to render `aria-label` as text ([#5709](https://github.com/elastic/eui/pull/5709))
 - Added `editorChecklist` glyph to `EuiIcon` ([#5705](https://github.com/elastic/eui/pull/5705))
+- `EuiDataGrid` now forces `isExpandable` to be true if any `cellActions` are passed, as keyboard users are otherwise unable to access cell actions without the expansion popover ([#5710](https://github.com/elastic/eui/pull/5710))
 
 **Breaking changes**