Convert table actions hover UX/behavior (#7640)

changelogs/upcoming/7640.md

@@ -0,0 +1,7 @@
+**Bug fixes**
+
+- `EuiBasicTable` & `EuiInMemoryTable` `isPrimary` actions are now correctly shown on mobile views
+
+**Breaking changes**
+
+- Removed the `showOnHover` prop from `EuiTableRowCell` / `EuiBasicTable`/`EuiInMemoryTable`'s `columns` API. Use the new actions `columns[].actions[].showOnHover` API instead.

src-docs/src/views/tables/actions/actions.tsx

@@ -162,6 +162,11 @@ export default () => {
       ];
       if (multiAction) {
         actions = [
+          {
+            ...actions[0],
+            isPrimary: true,
+            showOnHover: true,
+          },
           {
             render: (user: User) => {
               return (
@@ -176,7 +181,6 @@ export default () => {
               return <EuiLink onClick={() => {}}>Edit</EuiLink>;
             },
           },
-          ...actions,
         ];
       }
       return actions;

src/components/basic_table/__snapshots__/basic_table.test.tsx.snap

@@ -327,16 +327,16 @@ exports[`EuiBasicTable renders (kitchen sink) with pagination, selection, sortin
           </div>
         </td>
         <td
-          class="euiTableRowCell euiTableRowCell--hasActions emotion-euiTableRowCell-middle-desktop-euiBasicTableActionsWrapper"
+          class="euiTableRowCell euiTableRowCell--hasActions emotion-euiTableRowCell-hasActions-middle-desktop-actions"
         >
           <div
-            class="euiTableCellContent euiTableCellContent--alignRight euiTableCellContent--showOnHover euiTableCellContent--overflowingContent"
+            class="euiTableCellContent euiTableCellContent--alignRight euiTableCellContent--overflowingContent"
           >
             <span
               class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock"
             >
               <button
-                class="euiButtonEmpty euiTableCellContent__hoverItem emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
+                class="euiButtonEmpty emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
                 type="button"
               >
                 <span
@@ -354,7 +354,7 @@ exports[`EuiBasicTable renders (kitchen sink) with pagination, selection, sortin
               class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock"
             >
               <button
-                class="euiButtonEmpty euiTableCellContent__hoverItem emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
+                class="euiButtonEmpty emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
                 type="button"
               >
                 <span
@@ -448,16 +448,16 @@ exports[`EuiBasicTable renders (kitchen sink) with pagination, selection, sortin
           </div>
         </td>
         <td
-          class="euiTableRowCell euiTableRowCell--hasActions emotion-euiTableRowCell-middle-desktop-euiBasicTableActionsWrapper"
+          class="euiTableRowCell euiTableRowCell--hasActions emotion-euiTableRowCell-hasActions-middle-desktop-actions"
         >
           <div
-            class="euiTableCellContent euiTableCellContent--alignRight euiTableCellContent--showOnHover euiTableCellContent--overflowingContent"
+            class="euiTableCellContent euiTableCellContent--alignRight euiTableCellContent--overflowingContent"
           >
             <span
               class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock"
             >
               <button
-                class="euiButtonEmpty euiTableCellContent__hoverItem emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
+                class="euiButtonEmpty emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
                 type="button"
               >
                 <span
@@ -475,7 +475,7 @@ exports[`EuiBasicTable renders (kitchen sink) with pagination, selection, sortin
               class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock"
             >
               <button
-                class="euiButtonEmpty euiTableCellContent__hoverItem emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
+                class="euiButtonEmpty emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
                 type="button"
               >
                 <span
@@ -569,16 +569,16 @@ exports[`EuiBasicTable renders (kitchen sink) with pagination, selection, sortin
           </div>
         </td>
         <td
-          class="euiTableRowCell euiTableRowCell--hasActions emotion-euiTableRowCell-middle-desktop-euiBasicTableActionsWrapper"
+          class="euiTableRowCell euiTableRowCell--hasActions emotion-euiTableRowCell-hasActions-middle-desktop-actions"
         >
           <div
-            class="euiTableCellContent euiTableCellContent--alignRight euiTableCellContent--showOnHover euiTableCellContent--overflowingContent"
+            class="euiTableCellContent euiTableCellContent--alignRight euiTableCellContent--overflowingContent"
           >
             <span
               class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock"
             >
               <button
-                class="euiButtonEmpty euiTableCellContent__hoverItem emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
+                class="euiButtonEmpty emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
                 type="button"
               >
                 <span
@@ -596,7 +596,7 @@ exports[`EuiBasicTable renders (kitchen sink) with pagination, selection, sortin
               class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock"
             >
               <button
-                class="euiButtonEmpty euiTableCellContent__hoverItem emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
+                class="euiButtonEmpty emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
                 type="button"
               >
                 <span

src/components/basic_table/__snapshots__/expanded_item_actions.test.tsx.snap

@@ -1,39 +1,46 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`ExpandedItemActions render 1`] = `
-<Fragment>
-  <DefaultItemAction
-    action={
-      Object {
-        "description": "default 1",
-        "name": "default1",
-        "onClick": [Function],
-      }
-    }
-    enabled={true}
-    item={
-      Object {
-        "id": "xyz",
-      }
-    }
-    key="item_action_xyz_0"
+exports[`ExpandedItemActions renders 1`] = `
+<div>
+  <span
+    class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock"
+  >
+    <button
+      class="euiButtonEmpty emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
+      type="button"
+    >
+      <span
+        class="euiButtonEmpty__content emotion-euiButtonDisplayContent"
+      >
+        <span
+          class="eui-textTruncate euiButtonEmpty__text"
+        >
+          default1
+        </span>
+      </span>
+    </button>
+  </span>
+  <div
+    class=""
   />
-  <CustomItemAction
-    action={
-      Object {
-        "description": "custom 1",
-        "name": "custom1",
-        "render": [Function],
-      }
-    }
-    enabled={true}
-    index={1}
-    item={
-      Object {
-        "id": "xyz",
-      }
-    }
-    key="item_action_xyz_1"
-  />
-</Fragment>
+  <span
+    class="euiToolTipAnchor emotion-euiToolTipAnchor-inlineBlock"
+  >
+    <a
+      class="euiButtonEmpty euiBasicTableAction-showOnHover emotion-euiButtonDisplay-euiButtonEmpty-s-empty-primary-flush-right"
+      href="#"
+      rel="noreferrer"
+    >
+      <span
+        class="euiButtonEmpty__content emotion-euiButtonDisplayContent"
+      >
+        <span
+          class="eui-textTruncate euiButtonEmpty__text"
+        >
+          showOnHover
+        </span>
+      </span>
+    </a>
+  </span>
+</div>
 `;

src/components/basic_table/action_types.ts

@@ -39,8 +39,21 @@ export interface DefaultItemActionBase<T extends object> {
    * A callback function that determines whether the action is enabled
    */
   enabled?: (item: T) => boolean;
-  isPrimary?: boolean;
   'data-test-subj'?: string | ((item: T) => string);
+  /**
+   * If more than 3 actions are passed, 2 primary actions will show (on hover)
+   * next to an expansion menu of all actions.
+   *
+   * On mobile, primary actions will be tucked away in the expansion menu for space.
+   */
+  isPrimary?: boolean;
+  /**
+   * Allows only showing the action on mouse hover or keyboard focus.
+   * If more than 3 actions are passed, this will always be true for `isPrimary` actions.
+   *
+   * Has no effect on mobile, or if `hasActions` is not set.
+   */
+  showOnHover?: boolean;
 }
 
 export interface DefaultItemEmptyButtonAction<T extends object>
@@ -70,7 +83,7 @@ export type DefaultItemAction<T extends object> = ExclusiveUnion<
   DefaultItemIconButtonAction<T>
 >;
 
-export interface CustomItemAction<T> {
+export type CustomItemAction<T> = {
   /**
    * Allows rendering a totally custom action
    */
@@ -83,8 +96,7 @@ export interface CustomItemAction<T> {
    * A callback that defines whether the action is enabled
    */
   enabled?: (item: T) => boolean;
-  isPrimary?: boolean;
-}
+} & Pick<DefaultItemActionBase<{}>, 'isPrimary' | 'showOnHover'>;
 
 export type Action<T extends object> =
   | DefaultItemAction<T>

src/components/basic_table/basic_table.styles.ts

@@ -54,12 +54,6 @@ export const euiBasicTableBodyLoading = ({ euiTheme }: UseEuiTheme) => css`
 // Fix to make the loading indicator position correctly in Safari
 // For whatever annoying reason, Safari doesn't respect `position: relative;`
 // on `tbody` without `position: relative` on the parent `table`
-export const safariLoadingWorkaround = () => css`
+export const safariLoadingWorkaround = css`
   position: relative;
 `;
-
-// Unsets the extra height caused by tooltip/popover wrappers around table action buttons
-// Without this, the row height jumps whenever actions are disabled
-export const euiBasicTableActionsWrapper = css`
-  line-height: 1;
-`;

src/components/basic_table/basic_table.test.tsx

@@ -664,9 +664,12 @@ describe('EuiBasicTable', () => {
           },
         ],
       };
-      const { getAllByText } = render(<EuiBasicTable {...props} />);
+      const { getAllByText, container } = render(<EuiBasicTable {...props} />);
 
       expect(getAllByText('Delete')).toHaveLength(basicItems.length);
+      expect(
+        container.querySelector('.euiBasicTableAction-showOnHover')
+      ).not.toBeInTheDocument();
     });
 
     test('multiple actions with custom availability', () => {
@@ -680,7 +683,7 @@ describe('EuiBasicTable', () => {
           },
         ],
       };
-      const { getAllByText, getAllByTestSubject } = render(
+      const { getAllByText, getAllByTestSubject, container } = render(
         <EuiBasicTable {...props} />
       );
 
@@ -689,6 +692,12 @@ describe('EuiBasicTable', () => {
       expect(getAllByTestSubject('euiCollapsedItemActionsButton')).toHaveLength(
         4
       );
+      expect(
+        container.querySelector('.euiBasicTable__collapsedActions')
+      ).toBeInTheDocument();
+      expect(
+        container.querySelector('.euiBasicTableAction-showOnHover')
+      ).toBeInTheDocument();
     });
 
     test('custom item actions', () => {

src/components/basic_table/basic_table.tsx

@@ -78,7 +78,6 @@ import { EuiTableSortMobileProps } from '../table/mobile/table_sort_mobile';
 import {
   euiBasicTableBodyLoading,
   safariLoadingWorkaround,
-  euiBasicTableActionsWrapper,
 } from './basic_table.styles';
 
 type DataTypeProfiles = Record<
@@ -1140,9 +1139,15 @@ export class EuiBasicTable<T extends object = any> extends Component<
         // If all actions are disabled, do not show any actions but the popover toggle
         actualActions = [];
       } else {
-        // if any of the actions `isPrimary`, add them inline as well, but only the first 2
-        const primaryActions = actualActions.filter((o) => o.isPrimary);
-        actualActions = primaryActions.slice(0, 2);
+        // if any of the actions `isPrimary`, add them inline as well, but only the first 2,
+        // which we'll force to only show on hover for desktop views
+        const primaryActions = actualActions.filter(
+          (action) => action.isPrimary
+        );
+        actualActions = primaryActions.slice(0, 2).map((action) => ({
+          ...action,
+          showOnHover: true,
+        }));
       }
 
       // if we have more than 1 action, we don't show them all in the cell, instead we
@@ -1153,28 +1158,25 @@ export class EuiBasicTable<T extends object = any> extends Component<
 
       actualActions.push({
         name: 'All actions',
-        render: (item: T) => {
-          return (
-            <CollapsedItemActions
-              actions={column.actions}
-              actionsDisabled={allDisabled}
-              itemId={itemId}
-              item={item}
-            />
-          );
-        },
+        render: (item: T) => (
+          <CollapsedItemActions
+            className="euiBasicTable__collapsedActions"
+            actions={column.actions}
+            actionsDisabled={allDisabled}
+            itemId={itemId}
+            item={item}
+          />
+        ),
       });
     }
 
     const key = `record_actions_${itemId}_${columnIndex}`;
     return (
       <EuiTableRowCell
-        showOnHover={true}
         key={key}
         align="right"
         textOnly={false}
         hasActions={hasCustomActions ? 'custom' : true}
-        css={euiBasicTableActionsWrapper}
       >
         <ExpandedItemActions
           actions={actualActions}

src/components/basic_table/expanded_item_actions.test.tsx

@@ -7,14 +7,15 @@
  */
 
 import React from 'react';
-import { shallow } from 'enzyme';
+import { render } from '../../test/rtl';
+
 import {
   ExpandedItemActions,
   ExpandedItemActionsProps,
 } from './expanded_item_actions';
 
 describe('ExpandedItemActions', () => {
-  test('render', () => {
+  it('renders', () => {
     const props: ExpandedItemActionsProps<{ id: string }> = {
       actions: [
         {
@@ -27,14 +28,19 @@ describe('ExpandedItemActions', () => {
           description: 'custom 1',
           render: (_item) => <></>,
         },
+        {
+          name: 'showOnHover',
+          description: 'show on hover',
+          href: '#',
+          showOnHover: true,
+        },
       ],
       itemId: 'xyz',
       item: { id: 'xyz' },
       actionsDisabled: false,
     };
+    const { container } = render(<ExpandedItemActions {...props} />);
 
-    const component = shallow(<ExpandedItemActions {...props} />);
-
-    expect(component).toMatchSnapshot();
+    expect(container).toMatchSnapshot();
   });
 });