Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiDataGrid] Enforce a cell popover if cell actions are present #5710

Merged
merged 5 commits into from
Mar 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions src-docs/src/views/datagrid/_snippets.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,12 @@ inMemory={{ level: 'sorting' }}`,
actions: { showMoveLeft: false, showMoveRight: false }, // doesn't show move actions in column header
schema: 'franchise', // custom schema later defined under schemaDetectors
cellActions: [ // provides one additional cell action that triggers an alert once clicked
{
label: 'test',
iconType: 'heart',
callback: ()=> alert('test')
}
]
}
({ Component }) => <Component iconType="heart" onClick={() => alert('test')}>Custom action</Component>,
],
},
]}`,
columnVisibility: `columnVisibility={{
visibleColumns: ['A', 'C'],
visibleColumns: ['A'],
setVisibleColumns: () => {},
}}`,
leadingControlColumns: `leadingControlColumns={[
Expand Down Expand Up @@ -61,7 +57,7 @@ inMemory={{ level: 'sorting' }}`,
onChangeItemsPerPage: () => {},
}}`,
sorting: `sorting={{
columns: [{ id: 'C', direction: 'asc' }],
columns: [{ id: 'A', direction: 'asc' }],
onSort: () => {},
}}`,
toolbarVisibility: `toolbarVisibility={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const columns: EuiDataGridColumn[] = [
},
{
id: 'lastName',
isExpandable: false,
isExpandable: false, // Overridden by the fact that cellActions is set
cellActions,
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { GuideSectionTypes } from '../../../components';
import { EuiCode, EuiCallOut, EuiSpacer } from '../../../../../src';

import IsDetailsPopover from './cell_popover_is_details';
const IsDetailsPopoverSource = require('!!raw-loader!./cell_popover_is_details');
const isDetailsPopoverSource = require('!!raw-loader!./cell_popover_is_details');

import RenderCellPopover from './cell_popover_rendercellpopover';
const renderCellPopoverSource = require('!!raw-loader!./cell_popover_rendercellpopover');
Expand All @@ -19,7 +19,6 @@ import {
} from '!!prop-loader!../../../../../src/components/datagrid/data_grid_types';

export const DataGridCellPopoverExample = {
title: 'Data grid cell popovers',
sections: [
{
title: 'Conditionally customizing cell popover content',
Expand Down Expand Up @@ -48,7 +47,7 @@ export const DataGridCellPopoverExample = {
source: [
{
type: GuideSectionTypes.TSX,
code: IsDetailsPopoverSource,
code: isDetailsPopoverSource,
},
],
props: {
Expand Down Expand Up @@ -137,10 +136,10 @@ export const DataGridCellPopoverExample = {
text: (
<>
<p>
Popovers can sometimes be unnecessary for short form content. In the
example below we&apos;ve turned them off by setting{' '}
<EuiCode>isExpandable=false</EuiCode> on specific{' '}
<EuiCode>columns</EuiCode>.
Popovers can sometimes be unnecessary for short form content, and
can be disabled by setting <EuiCode>columns.isExpandable</EuiCode>{' '}
to <EuiCode>false</EuiCode>. In the example below, we&apos;ve turned
off expansion on the suffix column.
</p>
<p>
To set <EuiCode>isExpandable</EuiCode> at a per-cell level instead
Expand All @@ -149,6 +148,16 @@ export const DataGridCellPopoverExample = {
example conditionally disables the expansion popover for boolean
cells that are &apos;false&apos;.
</p>
<EuiCallOut
color="warning"
iconType="alert"
title="Cells with actions are always expandable"
>
If <EuiCode>columns.cellActions</EuiCode> is defined,{' '}
<EuiCode>isExpandable</EuiCode> will always be forced to true. This
ensures that keyboard and screen reader users have access to all
cell actions.
</EuiCallOut>
</>
),
demo: <IsExpandablePopover />,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { Fragment } from 'react';
import { Link } from 'react-router-dom';

import { GuideSectionTypes } from '../../../components';
import {
Expand All @@ -7,6 +8,7 @@ import {
EuiCallOut,
EuiBasicTable,
EuiSpacer,
EuiText,
EuiListGroupItem,
} from '../../../../../src';

Expand All @@ -27,6 +29,28 @@ import {

export const DataGridCellsExample = {
title: 'Data grid cells & popovers',
intro: (
<EuiText>
<p>
Data grid cells are rendered using the{' '}
<EuiCode>renderCellValue</EuiCode> prop. This prop accepts a function
which is treated as a React component behind the scenes. This means the
data grid passes cell-specific props (e.g. row index, column/schema
info, etc.) to your render function, which can ouput anything from a
string to any JSX element.
</p>
<p>
Each data grid cell will automatically render an expand action button
and an expansion popover (
<Link to="#disabling-cell-expansion-popovers">
which can be disabled
</Link>
). For cells with overflowing or truncated content, this expansion
popover helps display the full amount of content, or can be customized
to show extra details.
</p>
</EuiText>
),
sections: [
{
source: [
Expand Down
12 changes: 12 additions & 0 deletions src/components/datagrid/body/data_grid_cell.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,18 @@ describe('EuiDataGridCell', () => {
});

describe('isExpandable', () => {
it('always returns true if column.cellActions exists', () => {
const component = mount(
<EuiDataGridCell
{...requiredProps}
column={{ id: 'someId', cellActions: [() => <button />] }}
isExpandable={false}
/>
);

expect(component.find('renderCellValue').prop('isExpandable')).toBe(true);
});

it('falls back to props.isExpandable which is derived from the column config', () => {
const component = mount(
<EuiDataGridCell {...requiredProps} isExpandable={true} />
Expand Down
8 changes: 5 additions & 3 deletions src/components/datagrid/body/data_grid_cell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,10 @@ export class EuiDataGridCell extends Component<
};

isExpandable = () => {
// A cell must always show an expansion popover if it has cell actions,
// otherwise keyboard and screen reader users have no way of accessing them
if (this.props.column?.cellActions?.length) return true;

// props.isExpandable inherits from column.isExpandable
// state.cellProps allows consuming applications to override isExpandable on a per-cell basis
return this.state.cellProps.isExpandable ?? this.props.isExpandable;
Expand Down Expand Up @@ -516,7 +520,6 @@ export class EuiDataGridCell extends Component<

const isExpandable = this.isExpandable();
const popoverIsOpen = this.isPopoverOpen();
const hasCellActions = isExpandable || column?.cellActions;
const showCellActions =
this.state.isFocused ||
this.state.isEntered ||
Expand Down Expand Up @@ -652,7 +655,7 @@ export class EuiDataGridCell extends Component<
</EuiFocusTrap>
);

if (hasCellActions) {
if (isExpandable) {
innerContent = (
<div className={anchorClass} ref={this.popoverAnchorRef}>
<div className={expandClass}>
Expand All @@ -663,7 +666,6 @@ export class EuiDataGridCell extends Component<
rowIndex={rowIndex}
colIndex={colIndex}
column={column}
isExpandable={isExpandable}
closePopover={closeCellPopover}
onExpandClick={() => {
if (popoverIsOpen) {
Expand Down
44 changes: 13 additions & 31 deletions src/components/datagrid/body/data_grid_cell_actions.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@
import React from 'react';
import { shallow } from 'enzyme';

import { EuiDataGridColumnCellAction } from '../data_grid_types';
import {
EuiDataGridCellActions,
EuiDataGridCellPopoverActions,
} from './data_grid_cell_actions';

const MockAction: EuiDataGridColumnCellAction = ({ Component }) => (
<Component iconType="starEmpty" data-test-subj="mockCellAction" />
);

describe('EuiDataGridCellActions', () => {
const requiredProps = {
closePopover: jest.fn(),
Expand All @@ -23,9 +28,7 @@ describe('EuiDataGridCellActions', () => {
};

it('renders an expand button', () => {
const component = shallow(
<EuiDataGridCellActions {...requiredProps} isExpandable={true} />
);
const component = shallow(<EuiDataGridCellActions {...requiredProps} />);

expect(component).toMatchInlineSnapshot(`
<div
Expand Down Expand Up @@ -61,27 +64,10 @@ describe('EuiDataGridCellActions', () => {
const component = shallow(
<EuiDataGridCellActions
{...requiredProps}
isExpandable={false}
column={{ id: 'someId', cellActions: [() => <button />] }}
column={{ id: 'someId', cellActions: [MockAction] }}
/>
);

expect(component).toMatchInlineSnapshot(`
<div
className="euiDataGridRowCell__expandActions"
>
<Component
Component={[Function]}
closePopover={[MockFunction]}
colIndex={0}
columnId="someId"
isExpanded={false}
key="0"
rowIndex={0}
/>
</div>
`);

const button = component.childAt(0).renderProp('Component');
expect(button({ iconType: 'eye' })).toMatchInlineSnapshot(`
<EuiButtonIcon
Expand All @@ -97,16 +83,15 @@ describe('EuiDataGridCellActions', () => {
const component = shallow(
<EuiDataGridCellActions
{...requiredProps}
isExpandable={true}
column={{ id: 'someId', cellActions: [() => <button />] }}
column={{ id: 'someId', cellActions: [MockAction] }}
/>
);

expect(component).toMatchInlineSnapshot(`
<div
className="euiDataGridRowCell__expandActions"
>
<Component
<MockAction
Component={[Function]}
closePopover={[MockFunction]}
colIndex={0}
Expand All @@ -133,7 +118,7 @@ describe('EuiDataGridCellPopoverActions', () => {
<EuiDataGridCellPopoverActions
colIndex={0}
rowIndex={0}
column={{ id: 'someId', cellActions: [() => <button />] }}
column={{ id: 'someId', cellActions: [MockAction] }}
/>
);

Expand All @@ -147,7 +132,7 @@ describe('EuiDataGridCellPopoverActions', () => {
<EuiFlexItem
key="0"
>
<Component
<MockAction
Component={[Function]}
colIndex={0}
columnId="someId"
Expand All @@ -159,11 +144,8 @@ describe('EuiDataGridCellPopoverActions', () => {
</EuiPopoverFooter>
`);

const button = component
.childAt(0)
.childAt(0)
.childAt(0) // .find('Component') doesn't work, for whatever reason
.renderProp('Component');
const action = component.find('MockAction') as any;
const button = action.renderProp('Component');
expect(button({ iconType: 'function' })).toMatchInlineSnapshot(`
<EuiButtonEmpty
iconType="function"
Expand Down
10 changes: 6 additions & 4 deletions src/components/datagrid/body/data_grid_cell_actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,23 @@ import { EuiFlexGroup, EuiFlexItem } from '../../flex';
import { EuiPopoverFooter } from '../../popover';

export const EuiDataGridCellActions = ({
isExpandable,
closePopover,
onExpandClick,
column,
rowIndex,
colIndex,
}: {
isExpandable: boolean;
closePopover: () => void;
onExpandClick: () => void;
column?: EuiDataGridColumn;
rowIndex: number;
colIndex: number;
}) => {
const expandButton = isExpandable ? (
// Note: The cell expand button/expansion popover is *always* rendered if
// column.cellActions is present (regardless of column.isExpandable).
// This is because cell actions are not otherwise accessible to keyboard
// or screen reader users
const expandButton = (
<EuiI18n
key={'expand'}
token="euiDataGridCellActions.expandButtonTitle"
Expand All @@ -54,7 +56,7 @@ export const EuiDataGridCellActions = ({
/>
)}
</EuiI18n>
) : null;
);

const additionalButtons = useMemo(() => {
const ButtonComponent = (props: EuiButtonIconProps) => (
Expand Down
1 change: 1 addition & 0 deletions upcoming_changelogs/5710.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- `EuiDataGrid` now forces `isExpandable` to be true if any `cellActions` are passed, as keyboard users are otherwise unable to access cell actions without the expansion popover