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

[EuiBasicTable][EuiInMemoryTable] Add new truncationText line API #7254

Merged
merged 6 commits into from
Oct 6, 2023
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: 9 additions & 5 deletions src-docs/src/views/tables/auto/auto.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,12 @@ const columns: Array<EuiTableFieldDataColumnType<User>> = [
{
field: 'jobTitle',
name: 'Job title',
truncateText: true,
},
{
field: 'address',
name: 'Address',
truncateText: true,
truncateText: { lines: 2 },
},
];

Expand Down Expand Up @@ -147,11 +148,12 @@ const alignButtons: EuiButtonGroupOptionProps[] = [

export default () => {
const [tableLayout, setTableLayout] = useState('tableLayoutFixed');
const [vAlign, setVAlign] = useState('columnVAlignTop');
const [vAlign, setVAlign] = useState('columnVAlignMiddle');
const [align, setAlign] = useState('columnAlignLeft');

const onTableLayoutChange = (id: string, value: string) => {
setTableLayout(id);
columns[4].width = value === 'custom' ? '100px' : undefined;
columns[5].width = value === 'custom' ? '20%' : undefined;
};

Expand All @@ -169,14 +171,16 @@ export default () => {

switch (tableLayout) {
case 'tableLayoutFixed':
callOutText = 'Address has truncateText set to true';
callOutText =
'Job title has truncateText set to true. Address is set to { lines: 2 }';
break;
case 'tableLayoutAuto':
callOutText =
'Address has truncateText set to true which is not applied since tableLayout is set to auto';
'Job title will not wrap or truncate since tableLayout is set to auto. Address will truncate if necessary';
break;
case 'tableLayoutCustom':
callOutText = 'Address has truncateText set to true and width set to 20%';
callOutText =
'Job title has a custom column width of 100px. Address has a custom column width of 20%';
break;
}

Expand Down
12 changes: 9 additions & 3 deletions src/components/basic_table/table_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import { Pagination } from './pagination_bar';
import { Action } from './action_types';
import { Primitive } from '../../services/sort/comparators';
import { CommonProps } from '../common';
import { EuiTableRowCellMobileOptionsShape } from '../table/table_row_cell';
import {
EuiTableRowCellProps,
EuiTableRowCellMobileOptionsShape,
} from '../table/table_row_cell';

export type ItemId<T> = string | number | ((item: T) => string);
export type ItemIdResolved = string | number;
Expand Down Expand Up @@ -68,9 +71,12 @@ export interface EuiTableFieldDataColumnType<T>
*/
align?: HorizontalAlignment;
/**
* Indicates whether this column should truncate its content when it doesn't fit
* Indicates whether this column should truncate overflowing text content.
* - Set to `true` to enable single-line truncation.
* - To enable multi-line truncation, use a configuration object with `lines`
* set to a number of lines to truncate to.
*/
truncateText?: boolean;
truncateText?: EuiTableRowCellProps['truncateText'];
mobileOptions?: Omit<EuiTableRowCellMobileOptionsShape, 'render'> & {
render?: (item: T) => ReactNode;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,21 @@ exports[`truncateText defaults to false 1`] = `
</td>
`;

exports[`truncateText is rendered when specified 1`] = `
exports[`truncateText renders lines configuration 1`] = `
<td
class="euiTableRowCell euiTableRowCell--middle"
>
<div
class="euiTableCellContent"
>
<span
class="euiTableCellContent__text euiTextBlockTruncate emotion-euiTextBlockTruncate"
/>
</div>
</td>
`;

exports[`truncateText renders true 1`] = `
<td
class="euiTableRowCell euiTableRowCell--middle"
>
Expand Down
21 changes: 19 additions & 2 deletions src/components/table/table_row_cell.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,32 @@ describe('textOnly', () => {
});

describe('truncateText', () => {
test('defaults to false', () => {
it('defaults to false', () => {
const { container } = render(<EuiTableRowCell />);

expect(container.firstChild).toMatchSnapshot();
});

test('is rendered when specified', () => {
it('renders true', () => {
const { container } = render(<EuiTableRowCell truncateText={true} />);

expect(
container.querySelector('.euiTableCellContent--truncateText')
).toBeInTheDocument();
expect(container.firstChild).toMatchSnapshot();
});

test('renders lines configuration', () => {
const { container } = render(
<EuiTableRowCell truncateText={{ lines: 2 }} />
);

expect(
container.querySelector('.euiTableCellContent--truncateText')
).not.toBeInTheDocument();
expect(container.querySelector('.euiTableCellContent__text')).toHaveClass(
'euiTextBlockTruncate'
);
expect(container.firstChild).toMatchSnapshot();
});
});
Expand Down
63 changes: 39 additions & 24 deletions src/components/table/table_row_cell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,24 @@

import React, {
CSSProperties,
Fragment,
FunctionComponent,
ReactElement,
ReactNode,
TdHTMLAttributes,
useCallback,
} from 'react';
import classNames from 'classnames';
import { CommonProps } from '../common';

import { CommonProps } from '../common';
import {
HorizontalAlignment,
LEFT_ALIGNMENT,
RIGHT_ALIGNMENT,
CENTER_ALIGNMENT,
useIsWithinBreakpoints,
} from '../../services';
import { isObject } from '../../services/predicate';
import { EuiTextBlockTruncate } from '../text_truncate';

import { resolveWidthAsStyle } from './utils';

Expand All @@ -42,9 +44,12 @@ interface EuiTableRowCellSharedPropsShape {
*/
textOnly?: boolean;
/**
* Don't allow line breaks within cells
* Indicates whether this column should truncate overflowing text content.
* - Set to `true` to enable single-line truncation.
* - To enable multi-line truncation, use a configuration object with `lines`
* set to a number of lines to truncate to.
*/
truncateText?: boolean;
truncateText?: boolean | { lines: number };
Comment on lines +47 to +52
Copy link
Member Author

@cee-chen cee-chen Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elastic/eui-team [Request for votes/feedback]

As always, naming things & consistency are the hardest part of our jobs. Here's EuiDataGrid's current line truncation API:

<EuiDataGrid
  rowHeightsOptions={{
    defaultHeight: { lineCount: 2 },
  }}
/>

And here's what the above proposed API would look like:

<EuiBasicTable
  columns={[
    {
      field: 'myField',
      name: 'My field',
      truncateText: { lines: 2 },
    }
  ]}
/>

My question for you all is whether we should change lines to lineCount to match EuiDataGrid, or let them be different (or, if someone else has a totally different naming suggestion, feel free to throw that in the ring as well).

I opted for lines when I was first writing this because it felt more natural sounding to me - I only remembered after I was done that EuiDataGrid had its own API.

FWIW, EuiDataGrid generally has a very different data structure and API architecture to our table components, so I don't think it would be a huge loss for this to be different either, but maybe I'm downplaying that. I'd love to hear people's takes!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@1Copenut FYI, hoping to get this merged in for next week's release - any chance you can review this PR some time Monday morning? Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm onboard with your keeping these as separate names lineCount and lines respectively. The APIs are distinct enough that I'd look them / wouldn't assume the key was the same.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Thanks Trevor!

width?: CSSProperties['width'];
}

Expand Down Expand Up @@ -138,7 +143,7 @@ export const EuiTableRowCell: FunctionComponent<Props> = ({
'euiTableCellContent--alignRight': align === RIGHT_ALIGNMENT,
'euiTableCellContent--alignCenter': align === CENTER_ALIGNMENT,
'euiTableCellContent--showOnHover': showOnHover,
'euiTableCellContent--truncateText': truncateText,
'euiTableCellContent--truncateText': truncateText === true,
// We're doing this rigamarole instead of creating `euiTableCellContent--textOnly` for BWC
// purposes for the time-being.
'euiTableCellContent--overflowingContent': textOnly !== true,
Expand Down Expand Up @@ -171,23 +176,33 @@ export const EuiTableRowCell: FunctionComponent<Props> = ({

const styleObj = resolveWidthAsStyle(style, widthValue);

function modifyChildren(children: ReactNode) {
let modifiedChildren = children;

if (textOnly === true) {
modifiedChildren = <span className={childClasses}>{children}</span>;
} else if (React.isValidElement(children)) {
modifiedChildren = React.Children.map(
children,
(child: ReactElement<CommonProps>) =>
React.cloneElement(child, {
className: classNames(child.props.className, childClasses),
})
);
}

return modifiedChildren;
}
const modifyChildren = useCallback(
(children: ReactNode) => {
let modifiedChildren = children;

if (textOnly === true) {
modifiedChildren = <span className={childClasses}>{children}</span>;
} else if (React.isValidElement(children)) {
modifiedChildren = React.Children.map(
children,
(child: ReactElement<CommonProps>) =>
React.cloneElement(child, {
className: classNames(child.props.className, childClasses),
})
);
}
if (isObject(truncateText) && truncateText.lines) {
modifiedChildren = (
<EuiTextBlockTruncate lines={truncateText.lines} cloneElement>
{modifiedChildren}
</EuiTextBlockTruncate>
);
}

return modifiedChildren;
},
[childClasses, textOnly, truncateText]
);

const childrenNode = modifyChildren(children);

Expand Down Expand Up @@ -223,14 +238,14 @@ export const EuiTableRowCell: FunctionComponent<Props> = ({

{/* Content depending on mobile render existing */}
{mobileOptions.render ? (
<Fragment>
<>
<div className={`${mobileContentClasses} ${showForMobileClasses}`}>
{modifyChildren(mobileOptions.render)}
</div>
<div className={`${contentClasses} ${hideForMobileClasses}`}>
{childrenNode}
</div>
</Fragment>
</>
) : (
<div className={contentClasses}>{childrenNode}</div>
)}
Expand Down
1 change: 1 addition & 0 deletions upcoming_changelogs/7254.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Updated `EuiBasicTable` and `EuiInMemoryTable` to support multi-line truncation. This can be set via `truncateText.lines` in the `columns` prop.
Loading