[EuiBasicTable][EuiInMemoryTable] Add new `truncationText` line API by cee-chen · Pull Request #7254 · elastic/eui

cee-chen · 2023-10-04T17:23:12Z

Summary

As always, I recommend reviewing by commit to sort out feature work from cleanups.

Proposed API:

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

Single (true) vs multi-line truncation:

QA

Go to https://eui.elastic.co/pr_7254/#/tabular-content/tables#table-layout
Confirm the Address column truncates after 2 lines (and does not truncate if the address is short enough) on all layouts

General checklist

Browser QA
~~- [ ] Checked in both light and dark modes~~
~~- [ ] Checked for accessibility including keyboard-only and screenreader modes~~
- Checked in mobile
- Checked in Chrome, Safari, Edge, and Firefox
Docs site QA
- Updated documentation
- Props have proper autodocs (using @default if default values are missing) and playground toggles
  ~~- [ ] Checked Code Sandbox works for any docs examples~~
Code quality checklist
- Added or updated jest ~~and cypress tests~~
Release checklist
- A changelog entry exists and is marked appropriately.
  ~~- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)~~
Designer checklist
~~- [ ] Updated the Figma library counterpart~~

- remove unnecessary fragments, import organization

+ fix default starting valign - should be middle, not top

cee-chen · 2023-10-04T18:44:14Z

src/components/table/table_row_cell.tsx

+   * 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 };


@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!

@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!

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.

Awesome. Thanks Trevor!

kibanamachine · 2023-10-04T19:11:41Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7254/
Storybook: https://eui.elastic.co/pr_7254/storybook

elasticmachine · 2023-10-04T19:11:46Z

💚 Build Succeeded

Buildkite Build
Commit: 122628d

History

💚 Build #760 succeeded 6c37c1d

1Copenut

👍 LGTM. I left a comment agreeing with your reasoning for having different API key values. LMK if there's any retesting or additional review needed Monday and I'll grab it first thing.

1Copenut · 2023-10-06T21:52:27Z

src/components/table/table_row_cell.tsx

+   * 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 };


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.

`v88.5.4`⏩`v89.0.0` --- ## [`89.0.0`](https://github.com/elastic/eui/tree/v89.0.0) - Added new `pushAnimation` prop to push `EuiFlyout`s, which enables a slide in animation ([#7239](elastic/eui#7239)) - Updated `EuiComboBox` to use `EuiInputPopover` under the hood ([#7246](elastic/eui#7246)) - Added `inputPopoverProps` to `EuiComboBox`, which allows customizing the underlying popover ([#7246](elastic/eui#7246)) - Added a new beta `EuiTextBlockTruncate` component for multi-line truncation ([#7250](elastic/eui#7250)) - Updated `EuiBasicTable` and `EuiInMemoryTable` to support multi-line truncation. This can be set via `truncateText.lines` in the `columns` prop. ([#7254](elastic/eui#7254)) **Bug fixes** - Fixed `EuiFlexGroup` and `EuiFlexGrid`'s `m` gutter size ([#7251](elastic/eui#7251)) - Fixed focus trap rerender issues in `EuiFlyout` with memoization ([#7259](elastic/eui#7259)) - Fixed a visual bug with `EuiContextMenu`'s animation between panels ([#7268](elastic/eui#7268)) **Breaking changes** - EUI's global body font-size now respects the `font.defaultUnits` token. This means that the global font size will use the `rem` unit by default, instead of `px`. ([#7182](elastic/eui#7182)) - Removed exported `accessibleClickKeys`, `comboBoxKeys`, and `cascadingMenuKeys` services. Use the generic `keys` service instead ([#7256](elastic/eui#7256)) - Removed `EuiColorStops` due to low usage ([#7262](elastic/eui#7262)) - Removed `EuiSuggest`. We recommend using `EuiSelectable` or `EuiComboBox` instead ([#7263](elastic/eui#7263)) - Removed `euiHeaderAffordForFixed` Sass mixin, and `$euiHeaderHeight` and `$euiHeaderHeightCompensation` Sass variables. Use the CSS variable `--var(euiFixedHeadersOffset, 0)` instead. ([#7264](elastic/eui#7264)) **Accessibility** - When using `rem` or `em` font units, EUI now respects, instead of ignoring, browser default font sizes set by end users. ([#7182](elastic/eui#7182))

`v88.5.4`⏩`v89.0.0` --- ## [`89.0.0`](https://github.com/elastic/eui/tree/v89.0.0) - Added new `pushAnimation` prop to push `EuiFlyout`s, which enables a slide in animation ([elastic#7239](elastic/eui#7239)) - Updated `EuiComboBox` to use `EuiInputPopover` under the hood ([elastic#7246](elastic/eui#7246)) - Added `inputPopoverProps` to `EuiComboBox`, which allows customizing the underlying popover ([elastic#7246](elastic/eui#7246)) - Added a new beta `EuiTextBlockTruncate` component for multi-line truncation ([elastic#7250](elastic/eui#7250)) - Updated `EuiBasicTable` and `EuiInMemoryTable` to support multi-line truncation. This can be set via `truncateText.lines` in the `columns` prop. ([elastic#7254](elastic/eui#7254)) **Bug fixes** - Fixed `EuiFlexGroup` and `EuiFlexGrid`'s `m` gutter size ([elastic#7251](elastic/eui#7251)) - Fixed focus trap rerender issues in `EuiFlyout` with memoization ([elastic#7259](elastic/eui#7259)) - Fixed a visual bug with `EuiContextMenu`'s animation between panels ([elastic#7268](elastic/eui#7268)) **Breaking changes** - EUI's global body font-size now respects the `font.defaultUnits` token. This means that the global font size will use the `rem` unit by default, instead of `px`. ([elastic#7182](elastic/eui#7182)) - Removed exported `accessibleClickKeys`, `comboBoxKeys`, and `cascadingMenuKeys` services. Use the generic `keys` service instead ([elastic#7256](elastic/eui#7256)) - Removed `EuiColorStops` due to low usage ([elastic#7262](elastic/eui#7262)) - Removed `EuiSuggest`. We recommend using `EuiSelectable` or `EuiComboBox` instead ([elastic#7263](elastic/eui#7263)) - Removed `euiHeaderAffordForFixed` Sass mixin, and `$euiHeaderHeight` and `$euiHeaderHeightCompensation` Sass variables. Use the CSS variable `--var(euiFixedHeadersOffset, 0)` instead. ([elastic#7264](elastic/eui#7264)) **Accessibility** - When using `rem` or `em` font units, EUI now respects, instead of ignoring, browser default font sizes set by end users. ([elastic#7182](elastic/eui#7182))

cee-chen force-pushed the text-truncation-multi-1 branch from 6c37c1d to c5cd36a Compare October 4, 2023 18:34

cee-chen marked this pull request as ready for review October 4, 2023 18:39

cee-chen requested a review from a team as a code owner October 4, 2023 18:39

cee-chen added 6 commits October 4, 2023 11:40

Set up new configuration / types

aaf6bb2

Update table cells to allow truncating multi-line

bafa9e7

[perf] Memoize modifyChildren fn

8226f80

[nit] misc cleanup

d3ec396

- remove unnecessary fragments, import organization

[docs] Update example w/ truncation to include new multiline feature

81a3b34

+ fix default starting valign - should be middle, not top

changelog

122628d

cee-chen force-pushed the text-truncation-multi-1 branch from c5cd36a to 122628d Compare October 4, 2023 18:40

cee-chen commented Oct 4, 2023

View reviewed changes

cee-chen requested a review from 1Copenut October 6, 2023 20:25

1Copenut approved these changes Oct 6, 2023

View reviewed changes

cee-chen merged commit dfe59ce into elastic:main Oct 6, 2023

cee-chen deleted the text-truncation-multi-1 branch October 6, 2023 22:33

cee-chen mentioned this pull request Oct 9, 2023

Upgrade EUI to v89.0.0 elastic/kibana#168396

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

[EuiBasicTable][EuiInMemoryTable] Add new `truncationText` line API#7254
cee-chen merged 6 commits intoelastic:mainfrom
cee-chen:text-truncation-multi-1

cee-chen commented Oct 4, 2023 •

edited by 1Copenut

Loading

Uh oh!

cee-chen Oct 4, 2023 •

edited

Loading

Uh oh!

cee-chen Oct 6, 2023

Uh oh!

1Copenut Oct 6, 2023

Uh oh!

cee-chen Oct 6, 2023

Uh oh!

kibanamachine commented Oct 4, 2023

Uh oh!

elasticmachine commented Oct 4, 2023

Uh oh!

1Copenut left a comment

Uh oh!

1Copenut Oct 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

cee-chen commented Oct 4, 2023 • edited by 1Copenut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

QA

General checklist

Uh oh!

cee-chen Oct 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cee-chen Oct 6, 2023

Choose a reason for hiding this comment

Uh oh!

1Copenut Oct 6, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen Oct 6, 2023

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Oct 4, 2023

Uh oh!

elasticmachine commented Oct 4, 2023

💚 Build Succeeded

History

Uh oh!

1Copenut left a comment

Choose a reason for hiding this comment

Uh oh!

1Copenut Oct 6, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cee-chen commented Oct 4, 2023 •

edited by 1Copenut

Loading

cee-chen Oct 4, 2023 •

edited

Loading