[Discover] Truncate table header to 3 lines

[mbondyra]

Truncates table headers to 3 lines:

[elasticmachine]

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

[kertal]

Thx a lot for this POC @mbondyra 🎉 , I've a question, would it be possible to limit the height of the table header to e.g. 3 lines and on top of that apply truncation? Thx

[mbondyra]

@kertal it is possible and simple to implement if you want to apply truncation at the end of the string. IF you want to middle truncate in multi-line string, that's unfortunately not possible atm.

[davismcphee]

Looks and works great, thanks! We're going to discuss merging this early next week.

[opauloh]

Hi @mbondyra, I'm testing it locally from a plugin that uses the UnifiedDataTable, is it intended to truncate only when passing the showColumnTokens to the UnifiedDataTable?

with showColumnTokens = true

with showColumnTokens = false

[opauloh]

I think it's a good idea that we can show more header information, particularly useful for longer ECS fields, I only think that the Icons are taking up unneeded space on lines 2 and 3, and if that space could be filled with text would be more valuable.

What do you think about making the Token Icon inline as suggested below, so we have more space to show the column header?

[opauloh]

Could we include this feature as an optional property (i.e headerRowHeight)in the <UnifiedDataTable/> component and fallback it to 1, so plugins that use the Table (such as cloud_security_posture can opt for enabling that feature and also have more control over how much to truncate?

Usage suggestion (on Discovers page to apply the 3 row truncate):

<UnifiedDataTable
   //...
  headerRowHeight={3}
/>

[mbondyra]

Hi @opauloh!

is it intended to truncate only when passing the showColumnTokens to the UnifiedDataTable?

No, that's a bug, thank you for finding it!

I think it's a good idea that we can show more header information, particularly useful for longer ECS fields, I only think that the Icons are taking up unneeded space on lines 2 and 3, and if that space could be filled with text would be more valuable.

I like it, but I would like a designer's opinion too! @andreadelrio, could you let us know what you think?

Could we include this feature as an optional property (i.e headerRowHeight)in the component and fallback it to 1

Sure, that's a good idea, I'll add it 👌🏼

[kertal]

thx for the feedback @opauloh, I've a question. While those field tokens make sense in the Discover context (exploring the structure of the data, providing quick visual feedback about field types), I wonder if the are useful in the Security / Findings context, where I assume the type of the field might be redundant information?

[mbondyra]

@davismcphee I added the inline icon! I also noticed I didn't update the timestamp column header so I added that too. For numeric columns, the text is aligned to the right and it looks weird, but if we want to wrap it to 3 lines, I don't see other option.

[davismcphee]

Working great overall, just about ready to merge!

For numeric columns, the text is aligned to the right and it looks weird, but if we want to wrap it to 3 lines, I don't see other option.

I played around with this a bit and found that if we use float: right instead of text-align: right it allows us to right align the header content but still have the text wrap from the left. I think this should solve the issue for us.

Since I made the changes in my local branch anyway, I figured I'd just open a PR against yours here: mbondyra#9. I was also going to leave a couple of other very minor nits, but figured since I was in here anyway I'd just take care of them too.

If you could take a sec to test and confirm that the change works, and merge it into this branch if it does, this PR will be all good to go on my end. Thanks for all the work on this!

[jughosta]

Fyi: there are some updates coming to Eui data grid header #173569
Would be great to see how it works together.

[mbondyra]

@jughosta thanks for the heads up! I'll wait for the eui update to be merged and test again.
@davismcphee thanks for stepping in and a wonderful float:right solution, TIL 😀

[davismcphee]

Noooo, we were so close to being good to merge and now there's conflicts from the EUI upgrade 😢 But this is all ready to go on my end now, so once you get a chance to resolve the conflicts I'll do a super quick test again and click the approve button 👍

[davismcphee]

Tested one final time and it's still working as expected after the EUI upgrade 👍 The new three dots icon on hover causes things to look a little odd when the header text wraps since it shifts the text, but I don't think there's much we can do about that.

Thanks for the patience and all the work on this @mbondyra! Now quick, let's merge this before they change things on us again 😄

[mbondyra]

@andreadelrio @MichaelMarcialis if you could take a look too that would be great!

[andreadelrio]

Design changes LGTM. Thanks @mbondyra !

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 791f854

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
cloudSecurityPosture	414.1KB	414.9KB	+837.0B
discover	558.6KB	559.5KB	+901.0B
total			+1.7KB

Unknown metric groups

API count

id	before	after	diff
@kbn/unified-data-table	111	112	+1

History

• 💚 Build #186677 succeeded c45c258
• 💛 Build #186321 was flaky dc877af
• 💛 Build #186269 was flaky f8d0309
• 💚 Build #186133 succeeded 5f2ea9a
• 💔 Build #186090 failed c50c37f
• 💔 Build #186043 failed eda5c86

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @mbondyra

[achyutjhunjhunwala]

@mbondyra I use font size large on my chrome. Due to this i see the scrollbar appearing on the table header which is quite weird. Not sure if its a bug but should be checked from accessibility perspective