diff --git a/changelogs/upcoming/7373.md b/changelogs/upcoming/7373.md new file mode 100644 index 00000000000..3a2283e7b76 --- /dev/null +++ b/changelogs/upcoming/7373.md @@ -0,0 +1,17 @@ +- Updated the actions column in `EuiBasicTable` and `EuiInMemoryTable`s. Alongside `name`, the `description`, `href`, and `data-test-subj` properties now also accept an optional callback that the current `item` will be passed to +- Updated `EuiContextMenuItem` with a new `toolTipProps` prop + +**Bug fixes** + +- Fixed `EuiBasicTable` and `EuiInMemoryTable` actions not showing tooltip descriptions when rendered in the all actions popover menu +- Fixed missing underlines on `EuiContextMenu` link hover + +**Deprecations** + +- Deprecated `EuiContextMenuItem`'s `toolTipTitle` prop. Use `toolTipProps.title` instead +- Deprecated `EuiContextMenuItem`'s `toolTipPosition` prop. Use `toolTipProps.position` instead + +**Accessibility** + +- Fixed `EuiBasicTable` and `EuiInMemoryTable` actions not correctly reading out action descriptions to screen readers +- Fixed `EuiBasicTable` and `EuiInMemoryTable` primary actions not visibly appearing on keyboard focus diff --git a/scripts/jest/setup/throw_on_console_error.js b/scripts/jest/setup/throw_on_console_error.js index a0829e622c3..b0c208cd5b8 100644 --- a/scripts/jest/setup/throw_on_console_error.js +++ b/scripts/jest/setup/throw_on_console_error.js @@ -26,6 +26,17 @@ console.error = (message, ...rest) => { return; } + // Silence RTL act() errors, that appear to primarily come from the fact + // that we have multiple versions of `@testing-library/dom` installed + if ( + typeof message === 'string' && + message.startsWith( + 'Warning: The current testing environment is not configured to support act(...)' + ) + ) { + return; + } + // Print React validateDOMNesting warning as a console.warn instead // of throwing an error. // TODO: Remove when edge-case DOM nesting is fixed in all components diff --git a/src-docs/src/views/tables/actions/actions.tsx b/src-docs/src/views/tables/actions/actions.tsx index a233c872ac1..77a9faefbe1 100644 --- a/src-docs/src/views/tables/actions/actions.tsx +++ b/src-docs/src/views/tables/actions/actions.tsx @@ -183,13 +183,15 @@ export default () => { } else { let actions: Array> = [ { - name: 'Elastic.co', - description: 'Go to elastic.co', + name: 'User profile', + description: ({ firstName, lastName }) => + `Visit ${firstName} ${lastName}'s profile`, icon: 'editorLink', color: 'primary', type: 'icon', - href: 'https://elastic.co', - target: '_blank', + enabled: ({ online }) => !!online, + href: ({ id }) => `${window.location.href}?id=${id}`, + target: '_self', 'data-test-subj': 'action-outboundlink', }, ]; @@ -205,18 +207,20 @@ export default () => { }, { name: (user: User) => (user.id ? 'Delete' : 'Remove'), - description: 'Delete this user', + description: ({ firstName, lastName }) => + `Delete ${firstName} ${lastName}`, icon: 'trash', color: 'danger', type: 'icon', onClick: deleteUser, isPrimary: true, - 'data-test-subj': 'action-delete', + 'data-test-subj': ({ id }) => `action-delete-${id}`, }, { name: 'Edit', isPrimary: true, - available: ({ online }: { online: boolean }) => !online, + available: ({ online }) => !online, + enabled: ({ online }) => !!online, description: 'Edit this user', icon: 'pencil', type: 'icon', diff --git a/src-docs/src/views/tables/actions/actions_section.js b/src-docs/src/views/tables/actions/actions_section.js index 0f35200ab81..e9854f5b3e2 100644 --- a/src-docs/src/views/tables/actions/actions_section.js +++ b/src-docs/src/views/tables/actions/actions_section.js @@ -1,9 +1,14 @@ import React from 'react'; -import { EuiBasicTable } from '../../../../../src/components'; + +import { EuiBasicTable, EuiCode } from '../../../../../src/components'; + import { GuideSectionTypes } from '../../../components'; +import { EuiTableActionsColumnType } from '!!prop-loader!../../../../../src/components/basic_table/table_types'; +import { CustomItemAction } from '!!prop-loader!../../../../../src/components/basic_table/action_types'; +import { DefaultItemActionProps as DefaultItemAction } from '../props/props'; + import Table from './actions'; -import { EuiCode } from '../../../../../src/components/code'; const source = require('!!raw-loader!./actions'); export const section = { @@ -40,5 +45,6 @@ export const section = { ), components: { EuiBasicTable }, + props: { EuiTableActionsColumnType, DefaultItemAction, CustomItemAction }, demo: , }; diff --git a/src/components/basic_table/__snapshots__/collapsed_item_actions.test.tsx.snap b/src/components/basic_table/__snapshots__/collapsed_item_actions.test.tsx.snap index 7d351bcfc26..5b6e87d2620 100644 --- a/src/components/basic_table/__snapshots__/collapsed_item_actions.test.tsx.snap +++ b/src/components/basic_table/__snapshots__/collapsed_item_actions.test.tsx.snap @@ -1,7 +1,9 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`CollapsedItemActions custom actions 1`] = ` - +
- - + default1 + + + + - - default2 - - + + name xyz + + +
diff --git a/src/components/basic_table/__snapshots__/default_item_action.test.tsx.snap b/src/components/basic_table/__snapshots__/default_item_action.test.tsx.snap index 99ecdfd1bef..4feeceba827 100644 --- a/src/components/basic_table/__snapshots__/default_item_action.test.tsx.snap +++ b/src/components/basic_table/__snapshots__/default_item_action.test.tsx.snap @@ -1,86 +1,51 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`DefaultItemAction render - button 1`] = ` - - - action1 - - -`; - -exports[`DefaultItemAction render - default button 1`] = ` - - - action1 - - -`; - -exports[`DefaultItemAction render - icon 1`] = ` - - - - + action1 - - + + `; -exports[`DefaultItemAction render - name 1`] = ` - - + + + + - xyz + action1 - - + + `; diff --git a/src/components/basic_table/action_types.ts b/src/components/basic_table/action_types.ts index 09fb84d0109..6121c708f5f 100644 --- a/src/components/basic_table/action_types.ts +++ b/src/components/basic_table/action_types.ts @@ -18,18 +18,18 @@ type EuiButtonIconColorFunction = (item: T) => ButtonColor; export interface DefaultItemActionBase { /** - * The display name of the action (will be the button caption) + * The display name of the action (will render as visible text if rendered within a collapsed menu) */ name: ReactNode | ((item: T) => ReactNode); /** - * Describes the action (will be the button title) + * Describes the action (will render as tooltip content) */ - description: string; + description: string | ((item: T) => string); /** * A handler function to execute the action */ onClick?: (item: T) => void; - href?: string; + href?: string | ((item: T) => string); target?: string; /** * A callback function that determines whether the action is available @@ -40,7 +40,7 @@ export interface DefaultItemActionBase { */ enabled?: (item: T) => boolean; isPrimary?: boolean; - 'data-test-subj'?: string; + 'data-test-subj'?: string | ((item: T) => string); } export interface DefaultItemEmptyButtonAction @@ -88,8 +88,13 @@ export interface CustomItemAction { export type Action = DefaultItemAction | CustomItemAction; -export const isCustomItemAction = ( - action: DefaultItemAction | CustomItemAction -): action is CustomItemAction => { +export const isCustomItemAction = ( + action: DefaultItemAction | CustomItemAction +): action is CustomItemAction => { return action.hasOwnProperty('render'); }; + +export const callWithItemIfFunction = + (item: T) => + (prop: U | ((item: T) => U)): U => + typeof prop === 'function' ? (prop as Function)(item) : prop; diff --git a/src/components/basic_table/collapsed_item_actions.test.tsx b/src/components/basic_table/collapsed_item_actions.test.tsx index f369e459f9f..8d76caf5871 100644 --- a/src/components/basic_table/collapsed_item_actions.test.tsx +++ b/src/components/basic_table/collapsed_item_actions.test.tsx @@ -12,16 +12,19 @@ import { render, waitForEuiPopoverOpen, waitForEuiPopoverClose, + waitForEuiToolTipVisible, } from '../../test/rtl'; import { CollapsedItemActions } from './collapsed_item_actions'; +type Item = { id: string }; + describe('CollapsedItemActions', () => { it('renders', () => { const props = { actions: [ { - name: (item: { id: string }) => `default${item.id}`, + name: (item: Item) => `default${item.id}`, description: 'default 1', onClick: () => {}, }, @@ -51,10 +54,11 @@ describe('CollapsedItemActions', () => { 'data-test-subj': 'defaultAction', }, { - name: 'default2', - description: 'default 2', - href: 'https://www.elastic.co/', + name: ({ id }: Item) => `name ${id}`, + description: ({ id }: Item) => `description ${id}`, + href: ({ id }: Item) => `#/${id}`, target: '_blank', + 'data-test-subj': ({ id }: Item) => `${id}-link`, }, ], itemId: 'id', @@ -62,7 +66,7 @@ describe('CollapsedItemActions', () => { actionEnabled: () => true, }; - const { getByTestSubject, baseElement } = render( + const { getByTestSubject, getByText, baseElement } = render( ); fireEvent.click(getByTestSubject('euiCollapsedItemActionsButton')); @@ -70,6 +74,12 @@ describe('CollapsedItemActions', () => { expect(baseElement).toMatchSnapshot(); + expect(getByTestSubject('xyz-link')).toHaveAttribute('href', '#/xyz'); + expect(getByTestSubject('xyz-link')).toHaveTextContent('name xyz'); + fireEvent.mouseEnter(getByTestSubject('xyz-link')); + await waitForEuiToolTipVisible(); + expect(getByText('description xyz')).toBeInTheDocument(); + fireEvent.click(getByTestSubject('defaultAction')); await waitForEuiPopoverClose(); }); diff --git a/src/components/basic_table/collapsed_item_actions.tsx b/src/components/basic_table/collapsed_item_actions.tsx index bc93d2ce296..580e21d6a65 100644 --- a/src/components/basic_table/collapsed_item_actions.tsx +++ b/src/components/basic_table/collapsed_item_actions.tsx @@ -21,7 +21,12 @@ import { EuiButtonIcon } from '../button'; import { EuiToolTip } from '../tool_tip'; import { EuiI18n } from '../i18n'; -import { Action, CustomItemAction } from './action_types'; +import { + Action, + CustomItemAction, + isCustomItemAction, + callWithItemIfFunction, +} from './action_types'; import { ItemIdResolved } from './table_types'; export interface CollapsedItemActionsProps { @@ -32,10 +37,6 @@ export interface CollapsedItemActionsProps { className?: string; } -const actionIsCustomItemAction = ( - action: Action -): action is CustomItemAction => action.hasOwnProperty('render'); - export const CollapsedItemActions = ({ actions, itemId, @@ -59,7 +60,7 @@ export const CollapsedItemActions = ({ const enabled = actionEnabled(action); if (enabled) setAllDisabled(false); - if (actionIsCustomItemAction(action)) { + if (isCustomItemAction(action)) { const customAction = action as CustomItemAction; const actionControl = customAction.render(item, enabled); controls.push( @@ -74,20 +75,20 @@ export const CollapsedItemActions = ({ ); } else { - const { - onClick, - name, - href, - target, - 'data-test-subj': dataTestSubj, - } = action; - const buttonIcon = action.icon; let icon; if (buttonIcon) { icon = isString(buttonIcon) ? buttonIcon : buttonIcon(item); } - const buttonContent = typeof name === 'function' ? name(item) : name; + + const buttonContent = callWithItemIfFunction(item)(action.name); + const toolTipContent = callWithItemIfFunction(item)(action.description); + const href = callWithItemIfFunction(item)(action.href); + const dataTestSubj = callWithItemIfFunction(item)( + action['data-test-subj'] + ); + + const { onClick, target } = action; controls.push( ({ onClick={() => onClickItem(onClick ? () => onClick(item) : undefined) } + toolTipContent={toolTipContent} + toolTipProps={{ delay: 'long' }} > {buttonContent} diff --git a/src/components/basic_table/default_item_action.test.tsx b/src/components/basic_table/default_item_action.test.tsx index 607c1ab44e6..bafd8918835 100644 --- a/src/components/basic_table/default_item_action.test.tsx +++ b/src/components/basic_table/default_item_action.test.tsx @@ -7,7 +7,13 @@ */ import React from 'react'; -import { shallow } from 'enzyme'; +import { fireEvent } from '@testing-library/react'; +import { + render, + waitForEuiToolTipVisible, + waitForEuiToolTipHidden, +} from '../../test/rtl'; + import { DefaultItemAction } from './default_item_action'; import { DefaultItemEmptyButtonAction as EmptyButtonAction, @@ -19,10 +25,11 @@ interface Item { } describe('DefaultItemAction', () => { - test('render - default button', () => { + it('renders an EuiButtonEmpty when `type="button"', () => { const action: EmptyButtonAction = { name: 'action1', description: 'action 1', + type: 'button', onClick: () => {}, }; const props = { @@ -31,16 +38,17 @@ describe('DefaultItemAction', () => { item: { id: 'xyz' }, }; - const component = shallow(); + const { container } = render(); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); - test('render - button', () => { - const action: EmptyButtonAction = { - name: 'action1', + it('renders an EuiButtonIcon with screen reader text when `type="icon"`', () => { + const action: IconButtonAction = { + name: action1, description: 'action 1', - type: 'button', + type: 'icon', + icon: 'trash', onClick: () => {}, }; const props = { @@ -49,16 +57,15 @@ describe('DefaultItemAction', () => { item: { id: 'xyz' }, }; - const component = shallow(); + const { container } = render(); - expect(component).toMatchSnapshot(); + expect(container).toMatchSnapshot(); }); - test('render - name', () => { + it('renders an EuiButtonEmpty if no type is specified', () => { const action: EmptyButtonAction = { - name: (item) => {item.id}, + name: 'action1', description: 'action 1', - type: 'button', onClick: () => {}, }; const props = { @@ -67,27 +74,52 @@ describe('DefaultItemAction', () => { item: { id: 'xyz' }, }; - const component = shallow(); + const { container } = render(); - expect(component).toMatchSnapshot(); + expect(container.querySelector('.euiButtonEmpty')).toBeInTheDocument(); }); - test('render - icon', () => { - const action: IconButtonAction = { - name: action1, - description: 'action 1', - type: 'icon', - icon: 'trash', - onClick: () => {}, - }; - const props = { - action, - enabled: true, - item: { id: 'xyz' }, + test('props that can be functions', async () => { + const action: EmptyButtonAction = { + name: ({ id }) => + id === 'hello' ? Hello : world, + description: ({ id }) => + id === 'hello' ? 'hello tooltip' : 'goodbye tooltip', + href: ({ id }) => `#/${id}`, + 'data-test-subj': ({ id }) => `action-${id}`, }; - const component = shallow(); + const { getByTestSubject, getByText } = render( + <> + + + + ); + + const firstAction = getByTestSubject('action-hello'); + expect(firstAction).toHaveTextContent('Hello'); + expect(firstAction).toHaveAttribute('href', '#/hello'); + + const secondAction = getByTestSubject('action-world'); + expect(secondAction).toHaveTextContent('world'); + expect(secondAction).toHaveAttribute('href', '#/world'); + + fireEvent.mouseOver(firstAction); + await waitForEuiToolTipVisible(); + expect(getByText('hello tooltip')).toBeInTheDocument(); + fireEvent.mouseOut(firstAction); + await waitForEuiToolTipHidden(); - expect(component).toMatchSnapshot(); + fireEvent.mouseOver(secondAction); + await waitForEuiToolTipVisible(); + expect(getByText('goodbye tooltip')).toBeInTheDocument(); }); }); diff --git a/src/components/basic_table/default_item_action.tsx b/src/components/basic_table/default_item_action.tsx index 3cac7ddc1c0..7b71419502a 100644 --- a/src/components/basic_table/default_item_action.tsx +++ b/src/components/basic_table/default_item_action.tsx @@ -6,7 +6,8 @@ * Side Public License, v 1. */ -import React, { ReactElement } from 'react'; +import React, { ReactElement, ReactNode } from 'react'; + import { isString } from '../../services/predicate'; import { EuiButtonEmpty, @@ -15,10 +16,14 @@ import { EuiButtonIconProps, } from '../button'; import { EuiToolTip } from '../tool_tip'; -import { DefaultItemAction as Action } from './action_types'; import { useGeneratedHtmlId } from '../../services/accessibility'; import { EuiScreenReaderOnly } from '../accessibility'; +import { + DefaultItemAction as Action, + callWithItemIfFunction, +} from './action_types'; + export interface DefaultItemActionProps { action: Action; enabled: boolean; @@ -26,10 +31,7 @@ export interface DefaultItemActionProps { className?: string; } -// In order to use generics with an arrow function inside a .tsx file, it's necessary to use -// this `extends` hack and declare the types as shown, instead of declaring the const as a -// FunctionComponent -export const DefaultItemAction = ({ +export const DefaultItemAction = ({ action, enabled, item, @@ -55,32 +57,41 @@ export const DefaultItemAction = ({ } let button; - const actionContent = - typeof action.name === 'function' ? action.name(item) : action.name; + const actionContent = callWithItemIfFunction(item)(action.name); + const tooltipContent = callWithItemIfFunction(item)(action.description); + const href = callWithItemIfFunction(item)(action.href); + const dataTestSubj = callWithItemIfFunction(item)(action['data-test-subj']); + const ariaLabelId = useGeneratedHtmlId(); + let ariaLabelledBy: ReactNode; + if (action.type === 'icon') { if (!icon) { throw new Error(`Cannot render item action [${action.name}]. It is configured to render as an icon but no icon is provided. Make sure to set the 'icon' property of the action`); } button = ( - <> - - {/* actionContent (action.name) is a ReactNode and must be rendered to an element and referenced by ID for screen readers */} - - {actionContent} - - + + ); + // actionContent (action.name) is a ReactNode and must be rendered + // to an element and referenced by ID for screen readers + ariaLabelledBy = ( + + {actionContent} + ); } else { button = ( @@ -91,9 +102,9 @@ export const DefaultItemAction = ({ color={color as EuiButtonEmptyProps['color']} iconType={icon} onClick={onClick} - href={action.href} + href={href} target={action.target} - data-test-subj={action['data-test-subj']} + data-test-subj={dataTestSubj} flush="right" > {actionContent} @@ -101,11 +112,19 @@ export const DefaultItemAction = ({ ); } - return enabled && action.description ? ( - - {button} - + return enabled ? ( + <> + + {button} + + {/* SR text has to be rendered outside the tooltip, + otherwise EuiToolTip's own aria-labelledby won't properly clone */} + {ariaLabelledBy} + ) : ( - button + <> + {button} + {ariaLabelledBy} + ); }; diff --git a/src/components/basic_table/expanded_item_actions.tsx b/src/components/basic_table/expanded_item_actions.tsx index 6aa89b2b2e7..677f3db8bac 100644 --- a/src/components/basic_table/expanded_item_actions.tsx +++ b/src/components/basic_table/expanded_item_actions.tsx @@ -51,7 +51,7 @@ export const ExpandedItemActions = ({ expandedItemActions__completelyHide: moreThanThree && index < 2, }); - if (isCustomItemAction(action)) { + if (isCustomItemAction(action)) { // custom action has a render function tools.push( `; + +exports[`EuiContextMenuItem tooltip behavior 1`] = ` + +
+ + + +
+
+ + +`; diff --git a/src/components/context_menu/context_menu_item.styles.ts b/src/components/context_menu/context_menu_item.styles.ts index 7f83f0b6bbc..0672ad4fa94 100644 --- a/src/components/context_menu/context_menu_item.styles.ts +++ b/src/components/context_menu/context_menu_item.styles.ts @@ -27,13 +27,15 @@ export const euiContextMenuItemStyles = (euiThemeContext: UseEuiTheme) => { color: ${euiTheme.colors.text}; outline-offset: -${euiTheme.focus.width}; - &:enabled:hover, - &:enabled:focus { - text-decoration: underline; - } + &:not(:disabled) { + &:hover, + &:focus { + text-decoration: underline; + } - &:enabled:focus { - background-color: ${euiTheme.focus.backgroundColor}; + &:focus { + background-color: ${euiTheme.focus.backgroundColor}; + } } `, disabled: css` diff --git a/src/components/context_menu/context_menu_item.test.tsx b/src/components/context_menu/context_menu_item.test.tsx index 7ec564cc5e2..8c26344cfcc 100644 --- a/src/components/context_menu/context_menu_item.test.tsx +++ b/src/components/context_menu/context_menu_item.test.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { fireEvent } from '@testing-library/react'; -import { render } from '../../test/rtl'; +import { render, waitForEuiToolTipVisible } from '../../test/rtl'; import { shouldRenderCustomStyles } from '../../test/internal'; import { requiredProps } from '../../test/required_props'; @@ -17,6 +17,18 @@ import { EuiContextMenuItem, SIZES } from './context_menu_item'; describe('EuiContextMenuItem', () => { shouldRenderCustomStyles(); + shouldRenderCustomStyles( + , + { + childProps: ['toolTipProps', 'toolTipProps.anchorProps'], + skip: { parentTest: true }, + renderCallback: async ({ getByTestSubject }) => { + fireEvent.mouseOver(getByTestSubject('trigger')); + await waitForEuiToolTipVisible(); + }, + } + ); + it('renders', () => { const { container } = render( @@ -121,4 +133,22 @@ describe('EuiContextMenuItem', () => { ).toBeInTheDocument(); }); }); + + test('tooltip behavior', async () => { + const { getByRole, baseElement } = render( + + Hello + + ); + + fireEvent.mouseOver(getByRole('button')); + await waitForEuiToolTipVisible(); + + expect(baseElement).toMatchSnapshot(); + }); }); diff --git a/src/components/context_menu/context_menu_item.tsx b/src/components/context_menu/context_menu_item.tsx index 50c6158e213..a531be2431e 100644 --- a/src/components/context_menu/context_menu_item.tsx +++ b/src/components/context_menu/context_menu_item.tsx @@ -25,7 +25,7 @@ import { import { validateHref } from '../../services/security/href_validator'; import { CommonProps, keysOf } from '../common'; import { EuiIcon } from '../icon'; -import { EuiToolTip, ToolTipPositions } from '../tool_tip'; +import { EuiToolTip, EuiToolTipProps, ToolTipPositions } from '../tool_tip'; import { euiContextMenuItemStyles } from './context_menu_item.styles'; @@ -46,11 +46,16 @@ export interface EuiContextMenuItemProps extends CommonProps { */ toolTipContent?: ReactNode; /** - * Optional title for the tooltip + * Optional configuration to pass to the underlying [EuiToolTip](/#/display/tooltip). + * Accepts any prop that EuiToolTip does, except for `content` and `children`. + */ + toolTipProps?: Partial>; + /** + * @deprecated Use toolTipProps.title instead */ toolTipTitle?: ReactNode; /** - * Dictates the position of the tooltip. + * @deprecated Use tooltipProps.position instead */ toolTipPosition?: ToolTipPositions; href?: string; @@ -94,6 +99,7 @@ export const EuiContextMenuItem: FunctionComponent = ({ toolTipTitle, toolTipContent, toolTipPosition = 'right', + toolTipProps, href, target, rel, @@ -173,7 +179,7 @@ export const EuiContextMenuItem: FunctionComponent = ({ {buttonContent} ); - } else if (href || rest.onClick) { + } else if (href || rest.onClick || toolTipContent) { button = (