[EuiDataGrid][BugFix]: Fix visible overflowing truncated text

[mgadewoll]

Summary

closes #7780

This PR aims to fix an issue on EuiDataGrid cells where truncation is applied with line-clamp when the following props are set:

rowHeightsOptions: {
  defaultHeight: {
     lineCount: 1,
  }
}

This results in the overflown text being still visible partially. This depends on available space created by the fontSize and cellPadding passed via gridStyle.

before

To ensure the correct space is considered by line-clamp, we need to separate the styles for padding from the truncation step while still ensuring that the padding is included in the height calculation.

after

This PR also adds a more specific story for row height checks: https://eui.elastic.co/pr_7793/storybook/index.html?path=/story/tabular-content-euidatagrid--row-height

QA

• open storybook on production and on this PR branch and set the following props:
  • change fontSize on gridStyles to "s"

  {
    "fontSize": "s",
    "cellPadding": "l",
    "border": "all",
    "stripes": false,
    "header": "shade",
    "footer": "overline",
    "stickyFooter": true,
    "rowHover": "highlight",
    "rowClasses": {}
  }
  

  • change defaultHeight on rowHeightsOptions to {"lineCount": 1}

  {
    "defaultHeight": {
      "lineCount": 1
    },
    "rowHeights": {}
  }
  

  • verify that the on the fixed branch storybook the truncated text after the ellipsis is not visible
  • verify no other visual changes have been introduced

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 -N/A
• Code quality checklist
  • Added or updated VRT images
• 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

[mgadewoll]

I generally don't like adding additional wrappers but we need to separate the line-clamp from the padding to ensure that the truncated text is hidden.

[cee-chen]

I think my hangup is... I just honestly don't feel like it's worth it 🙈 it feels like an outsized performance impact (extra DOM node per cell, which will scale enormously on datagrids) for what's purely a visual effect. I'm tempted to look into other approaches if possible (pseudo element that "covers" the text? some other overflow CSS workaround?)

[cee-chen]

I found the world's hackiest CSS workaround using a transparent border! Let me know what you think? main...cee-chen:eui:datagrid/line-clamp-css-workaround

[mgadewoll]

Yeah, I totally agree - adding a wrapper is a big performance impact on DataGrid. That's why I'm also not really happy with it.
I checked your hacky workaround and it seems to work great! 🪄 I'd vote for that one as it's definitely way more lightweight 🪶 🎉

[mgadewoll]

One small downside I notice is that it does not currently update when changing the controls in the story (e.g. gridStyle.fontSize - it renders correctly on reload though.)

[cee-chen]

After poking at it, I think this is an issue in production datagrid because gridStyles.fontSize doesn't correctly update the expected row height on the production storybook either (which is the underlying issue).

I'm not totally sure this is worth chasing down right now as consumers typically don't change custom gridStyle.fontSize on the fly (vs users using the density toolbar control). Am I way off on that?

[mgadewoll]

1. I reverted the wrapper changes in favor of your suggested transparent border hack here in this commit.

2. I agree that the gridStyle issue is not a storybook issue but the prop generally doesn't trigger a recalculation.

In general this should not be an issue, as production would likely never on the fly change the styles via gridStyle prop, but in Storybook it seems weird/broken as we showcase the prop object and IMHO it's not apparent it's expected to not update. 😐

I gave it a quick shot to ensure the cell height calculation is triggered on fontSize and cellPadding change here in this commit. WDYT? (We don't have to implement this here if you don't agree with it, it's a suggestion in case we can fix it this way rather easily 🙂)

[cee-chen]

😬 I would kinda strongly prefer not to in this PR - if we do want to resolve it, I'd prefer it be a separate issue + PR to make it easy to roll back standalone if necessary.

For context, I'm generally paranoid/conservative about anything involving EuiDataGrid due to its scale which results in outsized rerender/performance issues, and the cascading impact the component has on power users.

[mgadewoll]

Sure, that's fair. I'll revert the last commit and move it to a separate PR then 🙂

[mgadewoll]

Reverted in this commit.

ℹ️ I added a separate issue for the gridStyles update.

[kibanamachine]

Preview staging links for this PR:

• Docs site: https://eui.elastic.co/pr_7793/
• Storybook: https://eui.elastic.co/pr_7793/storybook

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: bce20ea

History

• 💚 Build #2097 succeeded 9f0996d
• 💚 Build #2092 succeeded de2cfd5
• 💚 Build #2050 succeeded c47968c
• 💚 Build #2045 succeeded 4e4ca05

[cee-chen]

Definitely not me forgetting to actually select approve 🤦 ✅