Skip to content

Commit

Permalink
♻️ Move cell showOnHover API to basic table actions API
Browse files Browse the repository at this point in the history
- The goal of this is to limit usage to being controlled by EUI, and reduce potentially hidden information from users

- this allows us to replace `showOnHover` props on basic table columns (of which there is one usage in Kibana)

+ remove length logic from `ExpandedItemActions`, and let the correct hover className be determined via p
rop instead

+ update docs example to show custom actions working with `isPrimary` and `showOnHover` logic (previously could not be easily customized with these options)
  • Loading branch information
cee-chen committed Apr 1, 2024
1 parent 14b17f7 commit 4a6a8e8
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 63 deletions.
4 changes: 4 additions & 0 deletions changelogs/upcoming/7640.md
Original file line number Diff line number Diff line change
@@ -1,3 +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.
6 changes: 5 additions & 1 deletion src-docs/src/views/tables/actions/actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ export default () => {
];
if (multiAction) {
actions = [
{
...actions[0],
isPrimary: true,
showOnHover: true,
},
{
render: (user: User) => {
return (
Expand All @@ -176,7 +181,6 @@ export default () => {
return <EuiLink onClick={() => {}}>Edit</EuiLink>;
},
},
...actions,
];
}
return actions;
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
`;
20 changes: 16 additions & 4 deletions src/components/basic_table/action_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
*/
Expand All @@ -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>
Expand Down
13 changes: 11 additions & 2 deletions src/components/basic_table/basic_table.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -680,7 +683,7 @@ describe('EuiBasicTable', () => {
},
],
};
const { getAllByText, getAllByTestSubject } = render(
const { getAllByText, getAllByTestSubject, container } = render(
<EuiBasicTable {...props} />
);

Expand All @@ -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', () => {
Expand Down
31 changes: 18 additions & 13 deletions src/components/basic_table/basic_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1139,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
Expand All @@ -1152,16 +1158,15 @@ 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}
/>
),
});
}

Expand Down
16 changes: 11 additions & 5 deletions src/components/basic_table/expanded_item_actions.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
{
Expand All @@ -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();
});
});
4 changes: 1 addition & 3 deletions src/components/basic_table/expanded_item_actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ export const ExpandedItemActions = <T extends {}>({
actionsDisabled,
className,
}: ExpandedItemActionsProps<T>): ReactElement => {
const moreThanThree = actions.length > 2;

return (
<>
{actions.reduce<ReactNode[]>((tools, action, index) => {
Expand All @@ -48,7 +46,7 @@ export const ExpandedItemActions = <T extends {}>({
const key = `item_action_${itemId}_${index}`;

const classes = classNames(className, {
'euiBasicTableAction-showOnHover': moreThanThree && index < 2,
'euiBasicTableAction-showOnHover': action.showOnHover,
});

if (isCustomItemAction<T>(action)) {
Expand Down

0 comments on commit 4a6a8e8

Please sign in to comment.