diff --git a/CHANGELOG.md b/CHANGELOG.md index b9c77a58db3..63c9462b25b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ - Wrap `EuiHorizontalStep` text instead of truncating it ([#653](https://github.com/elastic/eui/pull/653)) - Fixed a bug where `EuiSideNavItem` wouldn't pass an `onClick` handler down to `` tags if they also had an `href`. ([#664](https://github.com/elastic/eui/pull/664)) +**Bug fixes** + +- Fixed `EuiBasicTable` re-rendering on hover of table rows ([#665](https://github.com/elastic/eui/pull/665)) + **Breaking changes** - `EuiHorizontalSteps` now requires an `onClick` prop be provided for each step configuration object ([#653](https://github.com/elastic/eui/pull/653)) diff --git a/src-docs/src/views/tables/actions/actions.js b/src-docs/src/views/tables/actions/actions.js index 8c0b922914b..02afa933b80 100644 --- a/src-docs/src/views/tables/actions/actions.js +++ b/src-docs/src/views/tables/actions/actions.js @@ -11,6 +11,7 @@ import { EuiFlexItem, EuiSwitch, EuiSpacer, + EuiText, } from '../../../../../src/components'; /* @@ -47,7 +48,8 @@ export class Table extends Component { sortField: 'firstName', sortDirection: 'asc', selectedItems: [], - multiAction: false + multiAction: false, + customAction: false, }; } @@ -79,6 +81,10 @@ export class Table extends Component { }); }; + onSelectionChange = (selectedItems) => { + this.setState({ selectedItems }); + }; + renderDeleteButton() { const { selectedItems } = this.state; @@ -101,6 +107,10 @@ export class Table extends Component { this.setState(prevState => ({ multiAction: !prevState.multiAction })); }; + toggleCustomAction = () => { + this.setState(prevState => ({ customAction: !prevState.customAction })); + }; + deleteUser = user => { store.deleteUsers(user.id); this.setState({ selectedItems: [] }); @@ -117,6 +127,8 @@ export class Table extends Component { pageSize, sortField, sortDirection, + multiAction, + customAction, } = this.state; const { @@ -126,6 +138,63 @@ export class Table extends Component { const deleteButton = this.renderDeleteButton(); + let actions = null; + + if(multiAction) { + actions = customAction + ? [{ + render: (item) => { + return ( + this.cloneUser(item)}> + Clone + + ); + } + }, { + render: (item) => { + return ( + this.deleteUser(item)}> + Delete + + ); + } + }] + : [{ + name: 'Clone', + description: 'Clone this person', + icon: 'copy', + onClick: this.cloneUser + }, { + name: 'Delete', + description: 'Delete this person', + icon: 'trash', + color: 'danger', + onClick: this.deleteUser + }]; + } else { + actions = customAction + ? [{ + render: (item) => { + return ( + this.deleteUser(item)} + color="danger" + > + Delete + + ); + } + }] + : [{ + name: 'Delete', + description: 'Delete this person', + icon: 'trash', + color: 'danger', + type: 'icon', + onClick: this.deleteUser + }]; + } + const columns = [{ field: 'firstName', name: 'First Name', @@ -166,25 +235,7 @@ export class Table extends Component { sortable: true }, { name: 'Actions', - actions: this.state.multiAction ? [{ - name: 'Clone', - description: 'Clone this person', - icon: 'copy', - onClick: this.cloneUser - }, { - name: 'Delete', - description: 'Delete this person', - icon: 'trash', - color: 'danger', - onClick: this.deleteUser - }] : [{ - name: 'Delete', - type: 'icon', - description: 'Delete this person', - icon: 'trash', - color: 'danger', - onClick: this.deleteUser - }] + actions }]; const pagination = { @@ -219,6 +270,13 @@ export class Table extends Component { onChange={this.toggleMultiAction} /> + + + diff --git a/src/components/basic_table/__snapshots__/basic_table.test.js.snap b/src/components/basic_table/__snapshots__/basic_table.test.js.snap index c490b87cf5d..e76b1ebec9e 100644 --- a/src/components/basic_table/__snapshots__/basic_table.test.js.snap +++ b/src/components/basic_table/__snapshots__/basic_table.test.js.snap @@ -116,8 +116,6 @@ exports[`EuiBasicTable basic - with items 1`] = ` @@ -819,8 +783,6 @@ exports[`EuiBasicTable with pagination, selection, sorting and a single record a @@ -876,8 +838,6 @@ exports[`EuiBasicTable with pagination, selection, sorting and a single record a @@ -981,8 +941,6 @@ exports[`EuiBasicTable with pagination, selection, sorting and column dataType 1 @@ -1319,8 +1265,6 @@ exports[`EuiBasicTable with pagination, selection, sorting and multiple record a @@ -1374,8 +1318,6 @@ exports[`EuiBasicTable with pagination, selection, sorting and multiple record a @@ -1477,8 +1419,6 @@ exports[`EuiBasicTable with pagination, selection, sorting, column renderer and default1 , - , + , ] } /> diff --git a/src/components/basic_table/__snapshots__/custom_item_action.test.js.snap b/src/components/basic_table/__snapshots__/custom_item_action.test.js.snap index 322fd9657dc..8094d63208a 100644 --- a/src/components/basic_table/__snapshots__/custom_item_action.test.js.snap +++ b/src/components/basic_table/__snapshots__/custom_item_action.test.js.snap @@ -2,10 +2,6 @@ exports[`CustomItemAction render 1`] = `
`; diff --git a/src/components/basic_table/__snapshots__/default_item_action.test.js.snap b/src/components/basic_table/__snapshots__/default_item_action.test.js.snap index d9fb8db8206..2bc22651bf9 100644 --- a/src/components/basic_table/__snapshots__/default_item_action.test.js.snap +++ b/src/components/basic_table/__snapshots__/default_item_action.test.js.snap @@ -6,15 +6,8 @@ exports[`DefaultItemAction render - button 1`] = ` fill={false} iconSide="left" isDisabled={false} - onBlur={[Function]} onClick={[Function]} - onFocus={[Function]} size="s" - style={ - Object { - "opacity": 1, - } - } title="action 1" type="button" > @@ -28,14 +21,7 @@ exports[`DefaultItemAction render - icon 1`] = ` color="primary" iconType="trash" isDisabled={false} - onBlur={[Function]} onClick={[Function]} - onFocus={[Function]} - style={ - Object { - "opacity": 1, - } - } title="action 1" type="button" /> diff --git a/src/components/basic_table/__snapshots__/expanded_item_actions.test.js.snap b/src/components/basic_table/__snapshots__/expanded_item_actions.test.js.snap index 276e44fb751..cad74905029 100644 --- a/src/components/basic_table/__snapshots__/expanded_item_actions.test.js.snap +++ b/src/components/basic_table/__snapshots__/expanded_item_actions.test.js.snap @@ -19,7 +19,6 @@ Array [ } itemId="xyz" key="item_action_xyz_0" - visible={true} />, , ] `; diff --git a/src/components/basic_table/basic_table.js b/src/components/basic_table/basic_table.js index 7f79490c027..4f84a135cd0 100644 --- a/src/components/basic_table/basic_table.js +++ b/src/components/basic_table/basic_table.js @@ -149,7 +149,6 @@ export class EuiBasicTable extends Component { constructor(props) { super(props); this.state = { - hoverRow: null, selection: [] }; } @@ -240,14 +239,6 @@ export class EuiBasicTable extends Component { this.props.onChange(criteria); } - onRowHover(row) { - this.setState({ hoverRow: row }); - } - - clearRowHover() { - this.setState({ hoverRow: null }); - } - componentWillReceiveProps(nextProps) { // Don't call changeSelection here or else we can get into an infinite loop: // changeSelection calls props.onSelectionChanged on owner -> @@ -460,9 +451,6 @@ export class EuiBasicTable extends Component { } }); - const onMouseOver = () => this.onRowHover(rowIndex); - const onMouseOut = () => this.clearRowHover(); - // Occupy full width of table, taking checkbox column into account. const expandedRowColSpan = selection ? columns.length + 1 : columns.length; // We'll use the ID to associate the expanded row with the original. @@ -480,8 +468,6 @@ export class EuiBasicTable extends Component { {cells} @@ -523,9 +509,7 @@ export class EuiBasicTable extends Component { ); } - renderItemActionsCell(itemId, item, column, columnIndex, rowIndex) { - const visible = this.state.hoverRow === rowIndex; - + renderItemActionsCell(itemId, item, column, columnIndex) { const actionEnabled = (action) => this.state.selection.length === 0 && (!action.enabled || action.enabled(item)); @@ -545,7 +529,6 @@ export class EuiBasicTable extends Component { return ( + {tools} ); diff --git a/src/components/basic_table/collapsed_item_actions.js b/src/components/basic_table/collapsed_item_actions.js index 3603ebfd532..05736e84715 100644 --- a/src/components/basic_table/collapsed_item_actions.js +++ b/src/components/basic_table/collapsed_item_actions.js @@ -45,7 +45,7 @@ export class CollapsedItemActions extends Component { render() { - const { actions, itemId, item, actionEnabled, onFocus } = this.props; + const { actions, itemId, item, actionEnabled, onFocus, className } = this.props; const isOpen = this.state.popoverOpen; @@ -60,8 +60,9 @@ export class CollapsedItemActions extends Component { allDisabled = allDisabled && !enabled; if (action.render) { const actionControl = action.render(item, enabled); + const actionControlOnClick = actionControl && actionControl.props && actionControl.props.onClick; controls.push( - + {}}> {actionControl} ); @@ -82,6 +83,7 @@ export class CollapsedItemActions extends Component { const popoverButton = ( { } } ], - visible: true, itemId: 'id', item: { id: 'xyz' }, actionEnabled: () => true, diff --git a/src/components/basic_table/custom_item_action.js b/src/components/basic_table/custom_item_action.js index aabecece419..8435b47bc3d 100644 --- a/src/components/basic_table/custom_item_action.js +++ b/src/components/basic_table/custom_item_action.js @@ -41,12 +41,12 @@ export class CustomItemAction extends Component { }; render() { - const { action, enabled, visible, item } = this.props; + const { action, enabled, item, className } = this.props; const tool = action.render(item, enabled); const clonedTool = cloneElement(tool, { onFocus: this.onFocus, onBlur: this.onBlur }); - const style = this.hasFocus() || visible ? { opacity: 1 } : { opacity: 0 }; + const style = this.hasFocus() ? { opacity: 1 } : null; return ( -
+
{clonedTool}
); diff --git a/src/components/basic_table/custom_item_action.test.js b/src/components/basic_table/custom_item_action.test.js index 63a4708a0d7..dd06b580a20 100644 --- a/src/components/basic_table/custom_item_action.test.js +++ b/src/components/basic_table/custom_item_action.test.js @@ -13,7 +13,6 @@ describe('CustomItemAction', () => { render: () => 'test' }, enabled: true, - visible: true, item: { id: 'xyz' } }; diff --git a/src/components/basic_table/default_item_action.js b/src/components/basic_table/default_item_action.js index f4bb460ec63..212f4dd4e2c 100644 --- a/src/components/basic_table/default_item_action.js +++ b/src/components/basic_table/default_item_action.js @@ -10,44 +10,10 @@ export class DefaultItemAction extends Component { constructor(props) { super(props); - this.state = { hasFocus: false }; - - // while generally considered an anti-pattern, here we require - // to do that as the onFocus/onBlur events of the action controls - // may trigger while this component is unmounted. An alternative - // (at least the workarounds suggested by react is to unregister - // the onFocus/onBlur listeners from the action controls... this - // unfortunately will lead to unecessarily complex code... so we'll - // stick to this approach for now) - this.mounted = false; - } - - componentWillMount() { - this.mounted = true; - } - - componentWillUnmount() { - this.mounted = false; } - onFocus = () => { - if (this.mounted) { - this.setState({ hasFocus: true }); - } - }; - - onBlur = () => { - if (this.mounted) { - this.setState({ hasFocus: false }); - } - }; - - hasFocus = () => { - return this.state.hasFocus; - }; - render() { - const { action, enabled, visible, item } = this.props; + const { action, enabled, item, className } = this.props; if (!action.onClick) { throw new Error(`Cannot render item action [${action.name}]. Missing required 'onClick' callback. If you want to provide a custom action control, make sure to define the 'render' callback`); @@ -55,7 +21,6 @@ export class DefaultItemAction extends Component { const onClick = () => action.onClick(item); const color = this.resolveActionColor(); const icon = this.resolveActionIcon(); - const style = this.hasFocus() || visible ? { opacity: 1 } : { opacity: 0 }; 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 @@ -63,31 +28,27 @@ export class DefaultItemAction extends Component { } return ( ); } return ( {action.name} diff --git a/src/components/basic_table/default_item_action.test.js b/src/components/basic_table/default_item_action.test.js index 59caeb22ab3..d4bca8b00a4 100644 --- a/src/components/basic_table/default_item_action.test.js +++ b/src/components/basic_table/default_item_action.test.js @@ -17,7 +17,6 @@ describe('DefaultItemAction', () => { onClick: () => {} }, enabled: true, - visible: true, item: { id: 'xyz' } }; @@ -40,7 +39,6 @@ describe('DefaultItemAction', () => { onClick: () => {} }, enabled: true, - visible: true, item: { id: 'xyz' } }; diff --git a/src/components/basic_table/expanded_item_actions.js b/src/components/basic_table/expanded_item_actions.js index 642adcb9a56..d9bb928b97e 100644 --- a/src/components/basic_table/expanded_item_actions.js +++ b/src/components/basic_table/expanded_item_actions.js @@ -2,7 +2,7 @@ import React from 'react'; import { DefaultItemAction } from './default_item_action'; import { CustomItemAction } from './custom_item_action'; -export const ExpandedItemActions = ({ actions, visible, itemId, item, actionEnabled }) => { +export const ExpandedItemActions = ({ actions, itemId, item, actionEnabled, className }) => { return actions.reduce((tools, action, index) => { const available = action.available ? action.available(item) : true; @@ -16,10 +16,10 @@ export const ExpandedItemActions = ({ actions, visible, itemId, item, actionEnab tools.push( @@ -28,10 +28,10 @@ export const ExpandedItemActions = ({ actions, visible, itemId, item, actionEnab tools.push( diff --git a/src/components/basic_table/expanded_item_actions.test.js b/src/components/basic_table/expanded_item_actions.test.js index 19d54b76aad..a0210dcd09c 100644 --- a/src/components/basic_table/expanded_item_actions.test.js +++ b/src/components/basic_table/expanded_item_actions.test.js @@ -21,7 +21,6 @@ describe('ExpandedItemActions', () => { } } ], - visible: true, itemId: 'xyz', item: { id: 'xyz' }, actionEnabled: () => true diff --git a/src/components/table/_table.scss b/src/components/table/_table.scss index 8accafadd65..2ca7d44056e 100644 --- a/src/components/table/_table.scss +++ b/src/components/table/_table.scss @@ -144,3 +144,14 @@ overflow: visible; /* 1 */ } } + +.euiTableCellContent--showOnHover { + .euiTableCellContent__hoverItem { + opacity: 0; + + .euiTableRow:hover &, + &:hover, &:focus { + opacity: 1; + } + } +} diff --git a/src/components/table/table_row_cell.js b/src/components/table/table_row_cell.js index 5dff2cda7c4..1f290c269bc 100644 --- a/src/components/table/table_row_cell.js +++ b/src/components/table/table_row_cell.js @@ -19,6 +19,7 @@ export const EuiTableRowCell = ({ children, className, truncateText, + showOnHover, textOnly, colSpan, ...rest @@ -26,20 +27,30 @@ export const EuiTableRowCell = ({ const contentClasses = classNames('euiTableCellContent', className, { 'euiTableCellContent--alignRight': align === RIGHT_ALIGNMENT, 'euiTableCellContent--alignCenter': align === CENTER_ALIGNMENT, + 'euiTableCellContent--showOnHover': showOnHover, 'euiTableCellContent--truncateText': truncateText, // We're doing this rigamarole instead of creating `euiTableCellContent--textOnly` for BWC // purposes for the time-being. - 'euiTableCellContent--overflowingContent': !textOnly, + 'euiTableCellContent--overflowingContent': textOnly !== true, }); + const childClasses = classNames({ + 'euiTableCellContent__text': textOnly === true, + 'euiTableCellContent__hoverItem': showOnHover, + }); + + let modifiedChildren = children; + + if(textOnly === true) { + modifiedChildren = {children}; + } else if(React.isValidElement(modifiedChildren)) { + modifiedChildren = React.Children.map(children, child => React.cloneElement(child, { className: childClasses })); + } + return (
- { - textOnly === true - ? {children} - : children - } + {modifiedChildren}
); @@ -47,6 +58,7 @@ export const EuiTableRowCell = ({ EuiTableRowCell.propTypes = { align: PropTypes.oneOf(ALIGNMENT), + showOnHover: PropTypes.bool, truncateText: PropTypes.bool, children: PropTypes.node, className: PropTypes.string,