[EuiBasicTable] Fix regressed disabled actions UX when selecting rows (…

…#7428)

Co-authored-by: Trevor Pierce <[email protected]>

changelogs/upcoming/7428.md

@@ -0,0 +1,3 @@
+**Bug fixes**
+
+- Fixed `EuiBasicTable`/`EuiInMemoryTable` actions to correctly show as disabled when rows are being selected

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

@@ -362,7 +362,10 @@ export default () => {
 
   return (
     <>
-      <EuiFlexGroup alignItems="center">
+      <EuiFlexGroup
+        alignItems="center"
+        css={({ euiTheme }) => ({ minHeight: euiTheme.size.xxl })}
+      >
         <EuiFlexItem grow={false}>
           <EuiSwitch
             label="Multiple Actions"

src-docs/src/views/tables/actions/actions_section.js

@@ -41,6 +41,11 @@ export const section = {
           When more than 2 actions are supplied, only the ellipses icon button
           stays visible at all times.
         </li>
+        <li>
+          When one or more table row(s) are selected, all item actions are
+          disabled. Users should be expected to use some bulk action outside the
+          individual table rows instead.
+        </li>
       </ul>
     </>
   ),

src/components/basic_table/basic_table.test.tsx

@@ -20,7 +20,10 @@ import {
 } from './basic_table';
 
 import { SortDirection } from '../../services';
-import { EuiTableFieldDataColumnType } from './table_types';
+import {
+  EuiTableFieldDataColumnType,
+  EuiTableActionsColumnType,
+} from './table_types';
 
 describe('getItemId', () => {
   it('returns undefined if no itemId prop is given', () => {
@@ -635,27 +638,65 @@ describe('EuiBasicTable', () => {
   });
 
   describe('actions', () => {
+    const actions: EuiTableActionsColumnType<{ id: string }>['actions'] = [
+      {
+        type: 'icon',
+        name: 'Edit',
+        isPrimary: true,
+        icon: 'pencil',
+        available: ({ id }) => !(Number(id) % 2),
+        description: 'edit',
+        'data-test-subj': 'editAction',
+        onClick: () => {},
+      },
+      {
+        type: 'icon',
+        name: 'Share',
+        icon: 'share',
+        isPrimary: true,
+        available: ({ id }) => id !== '3',
+        description: 'share',
+        onClick: () => {},
+      },
+      // Below actions are not primary and should be hidden behind collapse button
+      {
+        type: 'icon',
+        name: 'Copy',
+        icon: 'copy',
+        description: 'copy',
+        onClick: () => {},
+      },
+      {
+        type: 'icon',
+        name: 'Delete',
+        icon: 'trash',
+        description: 'delete',
+        'data-test-subj': ({ id }) => `deleteAction-${id}`,
+        onClick: () => {},
+      },
+      {
+        type: 'icon',
+        name: 'elastic.co',
+        icon: 'link',
+        description: 'Go to link',
+        onClick: () => {},
+      },
+    ];
+
     test('single action', () => {
       const props: EuiBasicTableProps<BasicItem> = {
         items: basicItems,
         columns: [
           ...basicColumns,
           {
             name: 'Actions',
-            actions: [
-              {
-                type: 'button',
-                name: 'Edit',
-                description: 'edit',
-                onClick: () => {},
-              },
-            ],
+            actions: [actions[3]],
           },
         ],
       };
       const { getAllByText } = render(<EuiBasicTable {...props} />);
 
-      expect(getAllByText('Edit')).toHaveLength(basicItems.length);
+      expect(getAllByText('Delete')).toHaveLength(basicItems.length);
     });
 
     test('multiple actions with custom availability', () => {
@@ -665,48 +706,7 @@ describe('EuiBasicTable', () => {
           ...basicColumns,
           {
             name: 'Actions',
-            actions: [
-              {
-                type: 'icon',
-                name: 'Edit',
-                isPrimary: true,
-                icon: 'pencil',
-                available: ({ id }) => !(Number(id) % 2),
-                description: 'edit',
-                onClick: () => {},
-              },
-              {
-                type: 'icon',
-                name: 'Share',
-                icon: 'share',
-                isPrimary: true,
-                available: ({ id }) => id !== '3',
-                description: 'share',
-                onClick: () => {},
-              },
-              // Below actions are not primary and should be hidden behind collapse button
-              {
-                type: 'icon',
-                name: 'Copy',
-                icon: 'copy',
-                description: 'copy',
-                onClick: () => {},
-              },
-              {
-                type: 'icon',
-                name: 'Delete',
-                icon: 'trash',
-                description: 'delete',
-                onClick: () => {},
-              },
-              {
-                type: 'icon',
-                name: 'elastic.co',
-                icon: 'link',
-                description: 'Go to link',
-                onClick: () => {},
-              },
-            ],
+            actions: actions,
           },
         ],
       };
@@ -720,6 +720,54 @@ describe('EuiBasicTable', () => {
         4
       );
     });
+
+    describe('are disabled on selection', () => {
+      test('single action', () => {
+        const props: EuiBasicTableProps<BasicItem> = {
+          items: basicItems,
+          selection: {
+            onSelectionChange: () => {},
+            selected: [basicItems[0]],
+          },
+          columns: [
+            ...basicColumns,
+            {
+              name: 'Actions',
+              actions: [actions[3]],
+            },
+          ],
+        };
+        const { getByTestSubject } = render(<EuiBasicTable {...props} />);
+
+        expect(getByTestSubject('deleteAction-1')).toBeDisabled();
+        expect(getByTestSubject('deleteAction-2')).toBeDisabled();
+        expect(getByTestSubject('deleteAction-3')).toBeDisabled();
+      });
+
+      test('multiple actions', () => {
+        const props: EuiBasicTableProps<BasicItem> = {
+          items: basicItems,
+          selection: {
+            onSelectionChange: () => {},
+            selected: [basicItems[0]],
+          },
+          columns: [
+            ...basicColumns,
+            {
+              name: 'Actions',
+              actions: actions,
+            },
+          ],
+        };
+        const { getAllByTestSubject } = render(<EuiBasicTable {...props} />);
+
+        getAllByTestSubject('euiCollapsedItemActionsButton').forEach(
+          (button) => {
+            expect(button).toBeDisabled();
+          }
+        );
+      });
+    });
   });
 
   it('renders (kitchen sink) with pagination, selection, sorting, actions, and footer', () => {

src/components/basic_table/basic_table.tsx

@@ -1153,17 +1153,21 @@ export class EuiBasicTable<T = any> extends Component<
     column: EuiTableActionsColumnType<T>,
     columnIndex: number
   ) {
-    const actionEnabled = (action: Action<T>) =>
-      this.state.selection.length === 0 &&
-      (!action.enabled || action.enabled(item));
+    // Disable all actions if any row(s) are selected
+    const allDisabled = this.state.selection.length > 0;
 
     let actualActions = column.actions.filter(
       (action: Action<T>) => !action.available || action.available(item)
     );
     if (actualActions.length > 2) {
-      // 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 (allDisabled) {
+        // 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 we have more than 1 action, we don't show them all in the cell, instead we
       // put them all in a popover tool. This effectively means we can only have a maximum
@@ -1177,9 +1181,9 @@ export class EuiBasicTable<T = any> extends Component<
           return (
             <CollapsedItemActions
               actions={column.actions}
+              actionsDisabled={allDisabled}
               itemId={itemId}
               item={item}
-              actionEnabled={actionEnabled}
             />
           );
         },
@@ -1189,9 +1193,9 @@ export class EuiBasicTable<T = any> extends Component<
     const tools = (
       <ExpandedItemActions
         actions={actualActions}
+        actionsDisabled={allDisabled}
         itemId={itemId}
         item={item}
-        actionEnabled={actionEnabled}
       />
     );
 

src/components/basic_table/collapsed_item_actions.test.tsx

@@ -36,7 +36,7 @@ describe('CollapsedItemActions', () => {
       ],
       itemId: 'id',
       item: { id: '1' },
-      actionEnabled: () => true,
+      actionsDisabled: false,
     };
 
     const { container } = render(<CollapsedItemActions {...props} />);
@@ -63,7 +63,7 @@ describe('CollapsedItemActions', () => {
       ],
       itemId: 'id',
       item: { id: 'xyz' },
-      actionEnabled: () => true,
+      actionsDisabled: false,
     };
 
     const { getByTestSubject, getByText, baseElement } = render(
@@ -92,7 +92,7 @@ describe('CollapsedItemActions', () => {
       ],
       itemId: 'id',
       item: { id: 'xyz' },
-      actionEnabled: () => true,
+      actionsDisabled: false,
     };
 
     const { getByTestSubject, baseElement } = render(

src/components/basic_table/collapsed_item_actions.tsx

@@ -33,19 +33,18 @@ export interface CollapsedItemActionsProps<T extends {}> {
   actions: Array<Action<T>>;
   item: T;
   itemId: ItemIdResolved;
-  actionEnabled: (action: Action<T>) => boolean;
+  actionsDisabled: boolean;
   className?: string;
 }
 
 export const CollapsedItemActions = <T extends {}>({
   actions,
   itemId,
   item,
-  actionEnabled,
+  actionsDisabled,
   className,
 }: CollapsedItemActionsProps<T>) => {
   const [popoverOpen, setPopoverOpen] = useState(false);
-  const [allDisabled, setAllDisabled] = useState(true);
 
   const onClickItem = useCallback((onClickAction?: () => void) => {
     setPopoverOpen(false);
@@ -57,8 +56,7 @@ export const CollapsedItemActions = <T extends {}>({
       const available = action.available?.(item) ?? true;
       if (!available) return controls;
 
-      const enabled = actionEnabled(action);
-      if (enabled) setAllDisabled(false);
+      const enabled = action.enabled == null || action.enabled(item);
 
       if (isCustomItemAction<T>(action)) {
         const customAction = action as CustomItemAction<T>;
@@ -94,7 +92,7 @@ export const CollapsedItemActions = <T extends {}>({
           <EuiContextMenuItem
             key={index}
             className="euiBasicTable__collapsedAction"
-            disabled={!enabled}
+            disabled={!enabled && !actionsDisabled}
             href={href}
             target={target}
             icon={icon}
@@ -111,25 +109,35 @@ export const CollapsedItemActions = <T extends {}>({
       }
       return controls;
     }, []);
-  }, [actions, actionEnabled, item, onClickItem]);
+  }, [actions, actionsDisabled, item, onClickItem]);
 
   const popoverButton = (
-    <EuiI18n token="euiCollapsedItemActions.allActions" default="All actions">
-      {(allActions: string) => (
+    <EuiI18n
+      tokens={[
+        'euiCollapsedItemActions.allActions',
+        'euiCollapsedItemActions.allActionsDisabled',
+      ]}
+      defaults={[
+        'All actions',
+        'Individual item actions are disabled when rows are being selected.',
+      ]}
+    >
+      {([allActions, allActionsDisabled]: string[]) => (
         <EuiButtonIcon
           className={className}
-          aria-label={allActions}
+          aria-label={actionsDisabled ? allActionsDisabled : allActions}
+          title={actionsDisabled ? allActionsDisabled : undefined}
           iconType="boxesHorizontal"
           color="text"
-          isDisabled={allDisabled}
+          isDisabled={actionsDisabled}
           onClick={() => setPopoverOpen((isOpen) => !isOpen)}
           data-test-subj="euiCollapsedItemActionsButton"
         />
       )}
     </EuiI18n>
   );
 
-  const withTooltip = !allDisabled && (
+  const withTooltip = !actionsDisabled && (
     <EuiI18n token="euiCollapsedItemActions.allActions" default="All actions">
       {(allActions: ReactNode) => (
         <EuiToolTip content={allActions} delay="long">

src/components/basic_table/expanded_item_actions.test.tsx

@@ -30,7 +30,7 @@ describe('ExpandedItemActions', () => {
       ],
       itemId: 'xyz',
       item: { id: 'xyz' },
-      actionEnabled: () => true,
+      actionsDisabled: false,
     };
 
     const component = shallow(<ExpandedItemActions {...props} />);