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

Create new EuiTextBlockTruncate component #7250

Merged
merged 3 commits into from
Oct 4, 2023

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Oct 4, 2023

Summary

⚠️ I suggest turning off whitespace changes when reviewing diffs, as docs changes contain a good amount of "changes" that are just indentation

closes #6312

Part 1 of #7226 (note: does not close the issue, as @walterra would like the component to be baked into EuiBasicTable's columns API - I'm separating that second part into another PR for easier review)

screencap

QA

General checklist

  • Browser QA
    - [ ] Checked in both light and dark modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • 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 - N/A, designers aren't really sure how to add truncation to Figma

+ update `<GuideTabbedPage>` to not require a `page` component, and use the `sections` array instead
@cee-chen cee-chen marked this pull request as ready for review October 4, 2023 02:43
@cee-chen cee-chen requested a review from a team as a code owner October 4, 2023 02:43
@cee-chen
Copy link
Member Author

cee-chen commented Oct 4, 2023

@elastic/eui-team I'm looking for feedback on the component name! I struggled a lot with trying to come up with something that wasn't overly wordy - EuiTextTruncateMultiLine felt like it was starting to stray into Java territory for me, despite feeling the most accurate.

I ended up going with Block to follow EuiCode vs. EuiCodeBlock, where one component is inline/block or single-line/multi-line. MDN's docs also specify the CSS has to be used on block elements which is neat.

If you have any other suggestions, I'd love to hear them!

@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

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

👍 LGTM! I stepped through the Storybook demos using VoiceOver on Firefox and Safari to get a sense how the line-clamp would drive. The text is all still available and readable as expected. Safari + VO even allows users to move which lines of text are shown in the container viewport which is pretty cool.

@cee-chen
Copy link
Member Author

cee-chen commented Oct 4, 2023

Safari + VO even allows users to move which lines of text are shown in the container viewport which is pretty cool.

Haha whoa, that's wild! I wonder if that's intentional (because the ... still remains on the clamped line) 🤔 I can't reproduce it outside of screen readers, so very neat nonetheless!

@cee-chen
Copy link
Member Author

cee-chen commented Oct 4, 2023

@1Copenut @breehall Before I merge this in, can I get a thumbs up or down (with suggestions, if down) on the component name? We could theoretically change it after merge/release since it's a beta component, but if y'all can think of a better name, I'd be happy to use it!

@1Copenut
Copy link
Contributor

1Copenut commented Oct 4, 2023

@cee-chen I'm 👍 on the name. Having the word Block in the component name gave me a sense it was a multi-line truncate not a single (inline) item.

@breehall
Copy link
Contributor

breehall commented Oct 4, 2023

@cee-chen To echo Trevor, I'm good with this name. Block is better than the alternative MultiLine. Your point of Code & CodeBlock also points to this being a good name!

@cee-chen
Copy link
Member Author

cee-chen commented Oct 4, 2023

Thank you all, that's super helpful!

@cee-chen cee-chen merged commit 08c7af8 into elastic:main Oct 4, 2023
8 checks passed
@cee-chen cee-chen deleted the text-truncation-multi branch October 4, 2023 16:19
1Copenut pushed a commit to elastic/kibana that referenced this pull request Oct 11, 2023
`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))
dej611 pushed a commit to dej611/kibana that referenced this pull request Oct 17, 2023
`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))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[euiTextTruncate utility] Support multi-line truncation
5 participants