[EuiDataGrid] Cell actions cleanup + bug fixes (#5592)

* Rename CellButtons to CellActions - to be more specific and intentional about what we're referring to * Rename .euiDataGridRowCell__expandButton to .euiDataGridRowCell__expandActions * .euiDataGridRowCell__expandButtonIcon cleanup - `__expandButtonIcon` is redundant with `__actionButtonIcon` - DRY it out and have the expand popover action use `__actionButtonIcon` - isActive CSS is no longer used whatsoever - remove conditional CSS and need for `popoverIsOpen` prop - Rename animation / last cellButton reference to cellActions * [bugfix] Hide the expansion button if a cell has actions but is not expandable * [bugfix] Prevent cell action animation from restarting on targeted cell * Add changelog entry * [bugfix] Fix cell actions animation flickering issues - remove `:hover:not(:focus)` in favor of simply having `:focus` disable animations, which lets us DRY out CSS with `.euiDataGridRowCell--open` - add `:focus-within` catch that prevents flickering animation issue after cell popover expansion close - (lint) remove unnecessary brackets around `euiDataGridRowCell--open` class * Restore removed action button CSS affecting legacy theme `border` in particular was causing the cell slide in animation to look wonky - this is worth applying to all cell action button icons, not just the cell expansion popover, since it will also fix issues for custom actions that have `display="fill"` * Improve mobile UI of cell actions in expansion popover
elastic · Feb 8, 2022 · ae099d7 · ae099d7
1 parent 030a432
commit ae099d7
Show file tree

Hide file tree

Showing 8 changed files with 105 additions and 109 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,9 @@
 ## [`main`](https://github.com/elastic/eui/tree/main)
 
-No public interface changes since `47.0.0`.
+**Bug fixes**
+
+- Fixed `EuiDataGrid` to correctly remove the cell expansion action button when a column sets both `cellActions` and `isExpandable` to false ([#5592](https://github.com/elastic/eui/pull/5592))
+- Fixed `EuiDataGrid` re-playing the cell actions animation when hovering over an already-focused cell ([#5592](https://github.com/elastic/eui/pull/5592))
 
 ## [`47.0.0`](https://github.com/elastic/eui/tree/v47.0.0)
 

diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss
@@ -38,49 +38,30 @@
   // Only add the transition effect on hover, so that it is instantaneous on focus
   // Long delays on hover to mitigate the accordion effect
   &:hover {
-    .euiDataGridRowCell__expandButtonIcon {
-      animation-duration: $euiAnimSpeedExtraFast;
-      animation-name: euiDataGridCellButtonSlideIn;
-      animation-iteration-count: 1;
-      animation-delay: $euiAnimSpeedNormal;
-      animation-fill-mode: forwards;
-    }
-
     .euiDataGridRowCell__actionButtonIcon {
       animation-duration: $euiAnimSpeedExtraFast;
-      animation-name: euiDataGridCellButtonSlideIn;
+      animation-name: euiDataGridCellActionsSlideIn;
       animation-iteration-count: 1;
       animation-delay: $euiAnimSpeedNormal;
       animation-fill-mode: forwards;
     }
   }
 
-  &:not(:hover),
+  // On focus, directly show action buttons (without animation)
+  &:focus,
+  // Prevent the animation from flickering after cell popover close when focus is restored the expansion button
+  &:focus-within,
+  // Always make the cell actions visible when the cell popover is open
   &.euiDataGridRowCell--open {
-    .euiDataGridRowCell__expandButtonIcon {
-      animation: none;
-      margin-left: $euiDataGridCellPaddingM;
-      width: $euiSizeM;
-    }
-
     .euiDataGridRowCell__actionButtonIcon {
       animation: none;
       margin-left: $euiDataGridCellPaddingM;
       width: $euiSizeM;
     }
   }
 
-  // on focus, directly show action buttons (without animation), but still slide in the open popover button
-  &:focus {
-    .euiDataGridRowCell__actionButtonIcon {
-      margin-left: $euiDataGridCellPaddingM;
-      width: $euiSizeM;
-    }
-  }
-
   // 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) {
-    .euiDataGridRowCell__expandButtonIcon,
     .euiDataGridRowCell__actionButtonIcon {
       display: none;
     }
@@ -159,9 +140,7 @@
 }
 
 // Cell actions
-// Could probably be more precisely named than '__expandButton', since there can be multiple actions/buttons
-// TODO: Consider renaming this when working on https://github.com/elastic/eui/issues/5132
-.euiDataGridRowCell__expandButton {
+.euiDataGridRowCell__expandActions {
   display: flex;
 }
 @include euiDataGridRowCellActions($definedHeight: false) {
@@ -175,29 +154,17 @@
   padding: $euiDataGridCellPaddingM 0;
 }
 
-.euiDataGridRowCell__expandButtonIcon {
+.euiDataGridRowCell__actionButtonIcon {
   height: $euiSizeM;
   border-radius: $euiBorderRadius / 2;
   width: 0;
   overflow: hidden;
-  transition: none; // Have to take out the generic transition so it is instaneous on focus
+  // Have to take out the generic transition so it is instaneous on focus
+  transition: none;
+  // Disable filled button box-shadows on legacy theme - they're unnecessary when this small in a datagrid
   box-shadow: none !important; // sass-lint:disable-line no-important
-  // Remove default .euiButtonIcon--fill border
-  // This way we don't need to animate the border that is inexistent in Amsterdam and unnecessary for a fill button
+  // Remove filled button borders on legacy theme - this way we don't need to animate the border
   border: none;
-
-  &-isActive {
-    margin-left: $euiDataGridCellPaddingM;
-    width: $euiSizeM;
-  }
-}
-
-.euiDataGridRowCell__actionButtonIcon {
-  height: $euiSizeM;
-  border-radius: $euiBorderRadius / 2;
-  width: 0;
-  overflow: hidden;
-  transition: none; // Have to take out the generic transition so it is instaneous on focus
 }
 
 // Row highlights
@@ -279,7 +246,7 @@
   }
 }
 
-@keyframes euiDataGridCellButtonSlideIn {
+@keyframes euiDataGridCellActionsSlideIn {
   from {
     margin-left: 0;
     width: 0;

diff --git a/src/components/datagrid/_mixins.scss b/src/components/datagrid/_mixins.scss
@@ -87,12 +87,12 @@ $euiDataGridStyles: (
   @if $definedHeight {
     // Defined heights are cells with row heights of auto, lineCount, or a static height
     // that set the __contentByHeight class
-    .euiDataGridRowCell__contentByHeight + .euiDataGridRowCell__expandButton {
+    .euiDataGridRowCell__contentByHeight + .euiDataGridRowCell__expandActions {
       @content;
     }
   } @else {
     // Otherwise, an undefined height (single flex row) will set __expandContent
-    .euiDataGridRowCell__expandContent + .euiDataGridRowCell__expandButton {
+    .euiDataGridRowCell__expandContent + .euiDataGridRowCell__expandActions {
       @content;
     }
   }

diff --git a/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap
@@ -191,7 +191,7 @@ Array [
     class="euiPopoverFooter"
   >
     <div
-      class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--directionRow euiFlexGroup--responsive"
+      class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--directionRow euiFlexGroup--wrap"
     >
       <div
         class="euiFlexItem"

diff --git a/src/components/datagrid/body/data_grid_cell.test.tsx b/src/components/datagrid/body/data_grid_cell.test.tsx
@@ -64,21 +64,21 @@ describe('EuiDataGridCell', () => {
     );
     component.setState({ enableInteractions: true });
 
-    const getCellButtons = () => component.find('EuiDataGridCellButtons');
-    expect(getCellButtons()).toHaveLength(1);
+    const getCellActions = () => component.find('EuiDataGridCellActions');
+    expect(getCellActions()).toHaveLength(1);
 
     // Should handle opening the popover
-    (getCellButtons().prop('onExpandClick') as Function)();
+    (getCellActions().prop('onExpandClick') as Function)();
     expect(mockPopoverContext.openCellPopover).toHaveBeenCalled();
 
     // Should handle closing the popover
     component.setProps({
       isExpandable: true,
       popoverContext: { ...mockPopoverContext, popoverIsOpen: true },
     });
-    (getCellButtons().prop('onExpandClick') as Function)();
+    (getCellActions().prop('onExpandClick') as Function)();
     expect(mockPopoverContext.closeCellPopover).toHaveBeenCalledTimes(1);
-    (getCellButtons().prop('closePopover') as Function)();
+    (getCellActions().prop('closePopover') as Function)();
     expect(mockPopoverContext.closeCellPopover).toHaveBeenCalledTimes(2);
   });
 

diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx
@@ -33,9 +33,9 @@ import {
   EuiDataGridCellValueProps,
 } from '../data_grid_types';
 import {
-  EuiDataGridCellButtons,
-  EuiDataGridCellPopoverButtons,
-} from './data_grid_cell_buttons';
+  EuiDataGridCellActions,
+  EuiDataGridCellPopoverActions,
+} from './data_grid_cell_actions';
 import { IS_JEST_ENVIRONMENT } from '../../../test';
 
 const EuiDataGridCellContent: FunctionComponent<
@@ -465,7 +465,7 @@ export class EuiDataGridCell extends Component<
               isDetails={true}
             />
           </PopoverContent>
-          <EuiDataGridCellPopoverButtons
+          <EuiDataGridCellPopoverActions
             rowIndex={rowIndex}
             colIndex={colIndex}
             column={column}
@@ -495,8 +495,8 @@ export class EuiDataGridCell extends Component<
     const { rowIndex, visibleRowIndex, colIndex } = rest;
 
     const popoverIsOpen = this.isPopoverOpen();
-    const hasCellButtons = isExpandable || column?.cellActions;
-    const showCellButtons =
+    const hasCellActions = isExpandable || column?.cellActions;
+    const showCellActions =
       this.state.isFocused ||
       this.state.isEntered ||
       this.state.enableInteractions ||
@@ -506,7 +506,7 @@ export class EuiDataGridCell extends Component<
       'euiDataGridRowCell',
       {
         [`euiDataGridRowCell--${columnType}`]: columnType,
-        ['euiDataGridRowCell--open']: popoverIsOpen,
+        'euiDataGridRowCell--open': popoverIsOpen,
       },
       className
     );
@@ -626,18 +626,18 @@ export class EuiDataGridCell extends Component<
       </EuiFocusTrap>
     );
 
-    if (hasCellButtons) {
-      if (showCellButtons) {
+    if (hasCellActions) {
+      if (showCellActions) {
         innerContent = (
           <div className={anchorClass} ref={this.popoverAnchorRef}>
             <div className={expandClass}>
               <EuiDataGridCellContent {...cellContentProps} />
             </div>
-            <EuiDataGridCellButtons
+            <EuiDataGridCellActions
               rowIndex={rowIndex}
               colIndex={colIndex}
               column={column}
-              popoverIsOpen={popoverIsOpen}
+              isExpandable={isExpandable}
               closePopover={closeCellPopover}
               onExpandClick={() => {
                 if (popoverIsOpen) {

diff --git a/...grid/body/data_grid_cell_buttons.test.tsx → ...grid/body/data_grid_cell_actions.test.tsx b/...grid/body/data_grid_cell_buttons.test.tsx → ...grid/body/data_grid_cell_actions.test.tsx
@@ -10,30 +10,31 @@ import React from 'react';
 import { shallow } from 'enzyme';
 
 import {
-  EuiDataGridCellButtons,
-  EuiDataGridCellPopoverButtons,
-} from './data_grid_cell_buttons';
+  EuiDataGridCellActions,
+  EuiDataGridCellPopoverActions,
+} from './data_grid_cell_actions';
 
-describe('EuiDataGridCellButtons', () => {
+describe('EuiDataGridCellActions', () => {
   const requiredProps = {
-    popoverIsOpen: false,
     closePopover: jest.fn(),
     onExpandClick: jest.fn(),
     rowIndex: 0,
     colIndex: 0,
   };
 
   it('renders an expand button', () => {
-    const component = shallow(<EuiDataGridCellButtons {...requiredProps} />);
+    const component = shallow(
+      <EuiDataGridCellActions {...requiredProps} isExpandable={true} />
+    );
 
     expect(component).toMatchInlineSnapshot(`
       <div
-        className="euiDataGridRowCell__expandButton"
+        className="euiDataGridRowCell__expandActions"
       >
         <EuiI18n
           default="Click or hit enter to interact with cell content"
           key="expand"
-          token="euiDataGridCellButtons.expandButtonTitle"
+          token="euiDataGridCellActions.expandButtonTitle"
         >
           <Component />
         </EuiI18n>
@@ -44,7 +45,7 @@ describe('EuiDataGridCellButtons', () => {
     expect(button('expandButtonTitle')).toMatchInlineSnapshot(`
       <EuiButtonIcon
         aria-hidden={true}
-        className="euiDataGridRowCell__expandButtonIcon"
+        className="euiDataGridRowCell__actionButtonIcon"
         color="primary"
         display="fill"
         iconSize="s"
@@ -55,17 +56,18 @@ describe('EuiDataGridCellButtons', () => {
     `);
   });
 
-  it('renders column cell actions as `EuiButtonIcon`s', () => {
+  it('renders cell actions as `EuiButtonIcon`s', () => {
     const component = shallow(
-      <EuiDataGridCellButtons
+      <EuiDataGridCellActions
         {...requiredProps}
+        isExpandable={false}
         column={{ id: 'someId', cellActions: [() => <button />] }}
       />
     );
 
     expect(component).toMatchInlineSnapshot(`
       <div
-        className="euiDataGridRowCell__expandButton"
+        className="euiDataGridRowCell__expandActions"
       >
         <Component
           Component={[Function]}
@@ -76,13 +78,6 @@ describe('EuiDataGridCellButtons', () => {
           key="0"
           rowIndex={0}
         />
-        <EuiI18n
-          default="Click or hit enter to interact with cell content"
-          key="expand"
-          token="euiDataGridCellButtons.expandButtonTitle"
-        >
-          <Component />
-        </EuiI18n>
       </div>
     `);
 
@@ -96,12 +91,45 @@ describe('EuiDataGridCellButtons', () => {
       />
     `);
   });
+
+  it('renders both cell actions and expand button', () => {
+    const component = shallow(
+      <EuiDataGridCellActions
+        {...requiredProps}
+        isExpandable={true}
+        column={{ id: 'someId', cellActions: [() => <button />] }}
+      />
+    );
+
+    expect(component).toMatchInlineSnapshot(`
+      <div
+        className="euiDataGridRowCell__expandActions"
+      >
+        <Component
+          Component={[Function]}
+          closePopover={[MockFunction]}
+          colIndex={0}
+          columnId="someId"
+          isExpanded={false}
+          key="0"
+          rowIndex={0}
+        />
+        <EuiI18n
+          default="Click or hit enter to interact with cell content"
+          key="expand"
+          token="euiDataGridCellActions.expandButtonTitle"
+        >
+          <Component />
+        </EuiI18n>
+      </div>
+    `);
+  });
 });
 
-describe('EuiDataGridCellPopoverButtons', () => {
+describe('EuiDataGridCellPopoverActions', () => {
   it('renders column cell actions as `EuiButtonEmpty`s', () => {
     const component = shallow(
-      <EuiDataGridCellPopoverButtons
+      <EuiDataGridCellPopoverActions
         colIndex={0}
         rowIndex={0}
         column={{ id: 'someId', cellActions: [() => <button />] }}
@@ -112,6 +140,8 @@ describe('EuiDataGridCellPopoverButtons', () => {
       <EuiPopoverFooter>
         <EuiFlexGroup
           gutterSize="s"
+          responsive={false}
+          wrap={true}
         >
           <EuiFlexItem
             key="0"
@@ -143,7 +173,7 @@ describe('EuiDataGridCellPopoverButtons', () => {
 
   it('does not render anything if the column has no cell actions', () => {
     const component = shallow(
-      <EuiDataGridCellPopoverButtons
+      <EuiDataGridCellPopoverActions
         colIndex={0}
         rowIndex={0}
         column={{ id: 'noActions' }}