diff --git a/changelogs/upcoming/7640.md b/changelogs/upcoming/7640.md new file mode 100644 index 00000000000..a24b726ba53 --- /dev/null +++ b/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. diff --git a/src-docs/src/views/tables/actions/actions.tsx b/src-docs/src/views/tables/actions/actions.tsx index 38e6befa478..6021ffbf523 100644 --- a/src-docs/src/views/tables/actions/actions.tsx +++ b/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 {}}>Edit; }, }, - ...actions, ]; } return actions; diff --git a/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap b/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap index 42549e0c5a5..177c5f5a488 100644 --- a/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap +++ b/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap @@ -327,16 +327,16 @@ exports[`EuiBasicTable renders (kitchen sink) with pagination, selection, sortin
+ +
- - + + + + + showOnHover + + + + +
`; diff --git a/src/components/basic_table/action_types.ts b/src/components/basic_table/action_types.ts index cb8d6981cbf..1ac557ee5dd 100644 --- a/src/components/basic_table/action_types.ts +++ b/src/components/basic_table/action_types.ts @@ -39,8 +39,21 @@ export interface DefaultItemActionBase { * 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 @@ -70,7 +83,7 @@ export type DefaultItemAction = ExclusiveUnion< DefaultItemIconButtonAction >; -export interface CustomItemAction { +export type CustomItemAction = { /** * Allows rendering a totally custom action */ @@ -83,8 +96,7 @@ export interface CustomItemAction { * A callback that defines whether the action is enabled */ enabled?: (item: T) => boolean; - isPrimary?: boolean; -} +} & Pick, 'isPrimary' | 'showOnHover'>; export type Action = | DefaultItemAction diff --git a/src/components/basic_table/basic_table.styles.ts b/src/components/basic_table/basic_table.styles.ts index a11071b592a..2aaea2280e0 100644 --- a/src/components/basic_table/basic_table.styles.ts +++ b/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; -`; diff --git a/src/components/basic_table/basic_table.test.tsx b/src/components/basic_table/basic_table.test.tsx index 8db29197dac..49d9eae7ac3 100644 --- a/src/components/basic_table/basic_table.test.tsx +++ b/src/components/basic_table/basic_table.test.tsx @@ -664,9 +664,12 @@ describe('EuiBasicTable', () => { }, ], }; - const { getAllByText } = render(); + const { getAllByText, container } = render(); 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( ); @@ -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', () => { diff --git a/src/components/basic_table/basic_table.tsx b/src/components/basic_table/basic_table.tsx index 75b9e6b542f..2b060f15831 100644 --- a/src/components/basic_table/basic_table.tsx +++ b/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 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 extends Component< actualActions.push({ name: 'All actions', - render: (item: T) => { - return ( - - ); - }, + render: (item: T) => ( + + ), }); } const key = `record_actions_${itemId}_${columnIndex}`; return ( { - 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(); - const component = shallow(); - - expect(component).toMatchSnapshot(); + expect(container).toMatchSnapshot(); }); }); diff --git a/src/components/basic_table/expanded_item_actions.tsx b/src/components/basic_table/expanded_item_actions.tsx index aa07ebece8c..3fd319cb0be 100644 --- a/src/components/basic_table/expanded_item_actions.tsx +++ b/src/components/basic_table/expanded_item_actions.tsx @@ -33,8 +33,6 @@ export const ExpandedItemActions = ({ actionsDisabled, className, }: ExpandedItemActionsProps): ReactElement => { - const moreThanThree = actions.length > 2; - return ( <> {actions.reduce((tools, action, index) => { @@ -48,7 +46,7 @@ export const ExpandedItemActions = ({ const key = `item_action_${itemId}_${index}`; const classes = classNames(className, { - expandedItemActions__completelyHide: moreThanThree && index < 2, + 'euiBasicTableAction-showOnHover': action.showOnHover, }); if (isCustomItemAction(action)) { diff --git a/src/components/table/__snapshots__/table_row_cell.test.tsx.snap b/src/components/table/__snapshots__/table_row_cell.test.tsx.snap index 38fd06c3fe9..128c42f9dbf 100644 --- a/src/components/table/__snapshots__/table_row_cell.test.tsx.snap +++ b/src/components/table/__snapshots__/table_row_cell.test.tsx.snap @@ -68,10 +68,10 @@ exports[`children's className merges new classnames into existing ones 1`] = ` class="euiTableRowCell emotion-euiTableRowCell-middle-desktop" >
diff --git a/src/components/table/_responsive.scss b/src/components/table/_responsive.scss index 2ebcdd8034f..0c91e7acc56 100644 --- a/src/components/table/_responsive.scss +++ b/src/components/table/_responsive.scss @@ -3,24 +3,6 @@ @include euiBreakpoint('xs', 's') { .euiTable.euiTable--responsive { - // never show hidden items and always show hover items on mobile, - .euiTableRow-hasActions .euiTableCellContent--showOnHover { - > * { - margin-left: 0; - } - - .expandedItemActions__completelyHide { - display: none; - } - - .euiTableCellContent__hoverItem { - opacity: 1; - filter: none; - margin-left: 0; - margin-bottom: $euiSizeS; - } - } - // force all content back to left side .euiTableCellContent--alignRight { justify-content: flex-start; diff --git a/src/components/table/_table.scss b/src/components/table/_table.scss index 6ef3c9a0c98..c8fd7333225 100644 --- a/src/components/table/_table.scss +++ b/src/components/table/_table.scss @@ -42,33 +42,3 @@ // /* 4 */ overflow-wrap is not supported on flex parents word-break: break-word; } - -.euiTableCellContent--showOnHover { - > *:not(:first-child) { - margin-left: $euiSizeS; - } -} - -.euiTableRow-hasActions { - .euiTableCellContent--showOnHover { - .euiTableCellContent__hoverItem { - flex-shrink: 0; - opacity: .7; - filter: grayscale(100%); - transition: opacity $euiAnimSpeedNormal $euiAnimSlightResistance, filter $euiAnimSpeedNormal $euiAnimSlightResistance; - } - - .expandedItemActions__completelyHide { - filter: grayscale(0%); - opacity: 0; - } - } - - &:hover .euiTableCellContent--showOnHover, - .euiTableCellContent--showOnHover:focus-within { - .euiTableCellContent__hoverItem { - opacity: 1; - filter: grayscale(0%); - } - } -} diff --git a/src/components/table/table_row.tsx b/src/components/table/table_row.tsx index 95840f0384f..05830f37924 100644 --- a/src/components/table/table_row.tsx +++ b/src/components/table/table_row.tsx @@ -35,8 +35,8 @@ export interface EuiTableRowProps { */ isSelected?: boolean; /** - * Indicates if the table has a dedicated column for icon-only actions - * (used for mobile styling) + * Indicates if the table has a dedicated column for actions + * (used for mobile styling and desktop action hover behavior) */ hasActions?: boolean; /** diff --git a/src/components/table/table_row_cell.styles.ts b/src/components/table/table_row_cell.styles.ts index 8a2b3931c04..d68b90414ac 100644 --- a/src/components/table/table_row_cell.styles.ts +++ b/src/components/table/table_row_cell.styles.ts @@ -18,10 +18,27 @@ export const euiTableRowCellStyles = (euiThemeContext: UseEuiTheme) => { const { mobileSizes } = euiTableVariables(euiThemeContext); + // Unsets the extra strut caused by inline-block display of buttons/icons/tooltips. + // Without this, the row height jumps whenever actions are disabled. + const hasIcons = `line-height: 1;`; + return { euiTableRowCell: css` color: ${euiTheme.colors.text}; `, + isExpander: css` + ${hasIcons} + `, + hasActions: css` + ${hasIcons} + + /* TODO: Move this to EuiTableCellContent, once we're further along in the Emotion conversion */ + .euiTableCellContent { + display: flex; + align-items: center; + gap: ${euiTheme.size.s}; + } + `, // valign middle: css` @@ -37,9 +54,30 @@ export const euiTableRowCellStyles = (euiThemeContext: UseEuiTheme) => { vertical-align: bottom; `, - desktop: css` - ${logicalCSS('border-vertical', euiTheme.border.thin)} - `, + desktop: { + desktop: css` + ${logicalCSS('border-vertical', euiTheme.border.thin)} + `, + actions: css` + /* TODO: Move this to EuiTableCellContent, once we're further along in the Emotion conversion */ + .euiTableCellContent { + flex-wrap: wrap; + } + + .euiBasicTableAction-showOnHover { + opacity: 0; + transition: opacity ${euiTheme.animation.normal} + ${euiTheme.animation.resistance}; + } + + &:focus-within, + .euiTableRow-hasActions:hover & { + .euiBasicTableAction-showOnHover { + opacity: 1; + } + } + `, + }, mobile: { mobile: css` @@ -64,6 +102,7 @@ export const euiTableRowCellStyles = (euiThemeContext: UseEuiTheme) => { } `, get actions() { + // Note: Visible-on-hover actions on desktop always show on mobile return css` ${this.rightColumnContent} ${logicalCSS('top', mobileSizes.actions.offset)} diff --git a/src/components/table/table_row_cell.test.tsx b/src/components/table/table_row_cell.test.tsx index b2f5a315a44..8a78c33c3d5 100644 --- a/src/components/table/table_row_cell.test.tsx +++ b/src/components/table/table_row_cell.test.tsx @@ -128,7 +128,7 @@ describe('truncateText', () => { describe("children's className", () => { test('merges new classnames into existing ones', () => { const { container } = renderInTableRow( - +
); diff --git a/src/components/table/table_row_cell.tsx b/src/components/table/table_row_cell.tsx index 3586f6b9172..a607e6c92cd 100644 --- a/src/components/table/table_row_cell.tsx +++ b/src/components/table/table_row_cell.tsx @@ -36,10 +36,6 @@ interface EuiTableRowCellSharedPropsShape { * Horizontal alignment of the text in the cell */ align?: HorizontalAlignment; - /** - * _Should only be used for action cells_ - */ - showOnHover?: boolean; /** * Creates a text wrapper around cell content that helps word break or truncate * long text correctly. @@ -95,8 +91,8 @@ export interface EuiTableRowCellProps extends EuiTableRowCellSharedPropsShape { */ setScopeRow?: boolean; /** - * Indicates if the column is dedicated to icon-only actions (currently - * affects mobile only) + * Indicates if the cell is dedicated to row actions + * (used for mobile styling and desktop action hover behavior) */ hasActions?: boolean | 'custom'; /** @@ -121,7 +117,6 @@ export const EuiTableRowCell: FunctionComponent = ({ className, truncateText, setScopeRow, - showOnHover, textOnly = true, hasActions, isExpander, @@ -137,6 +132,8 @@ export const EuiTableRowCell: FunctionComponent = ({ const styles = useEuiMemoizedStyles(euiTableRowCellStyles); const cssStyles = [ styles.euiTableRowCell, + isExpander && styles.isExpander, + hasActions && styles.hasActions, styles[valign], ...(isResponsive ? [ @@ -146,7 +143,7 @@ export const EuiTableRowCell: FunctionComponent = ({ hasActions === true && styles.mobile.actions, isExpander && styles.mobile.expander, ] - : [styles.desktop]), + : [styles.desktop.desktop, hasActions && styles.desktop.actions]), ]; const cellClasses = classNames('euiTableRowCell', className, { @@ -158,7 +155,6 @@ export const EuiTableRowCell: FunctionComponent = ({ const contentClasses = classNames('euiTableCellContent', { 'euiTableCellContent--alignRight': align === RIGHT_ALIGNMENT, 'euiTableCellContent--alignCenter': align === CENTER_ALIGNMENT, - 'euiTableCellContent--showOnHover': showOnHover, 'euiTableCellContent--truncateText': truncateText === true, // We're doing this rigamarole instead of creating `euiTableCellContent--textOnly` for BWC // purposes for the time-being. @@ -170,8 +166,6 @@ export const EuiTableRowCell: FunctionComponent = ({ mobileOptions.align === RIGHT_ALIGNMENT || align === RIGHT_ALIGNMENT, 'euiTableCellContent--alignCenter': mobileOptions.align === CENTER_ALIGNMENT || align === CENTER_ALIGNMENT, - 'euiTableCellContent--showOnHover': - mobileOptions.showOnHover ?? showOnHover, 'euiTableCellContent--truncateText': mobileOptions.truncateText ?? truncateText, // We're doing this rigamarole instead of creating `euiTableCellContent--textOnly` for BWC @@ -182,7 +176,6 @@ export const EuiTableRowCell: FunctionComponent = ({ const childClasses = classNames({ euiTableCellContent__text: textOnly === true, - euiTableCellContent__hoverItem: showOnHover, }); const widthValue = isResponsive