Fix UsersProfilePopover doesn't handle long names well

[Dosant]

Summary

Fix #182084

Before

After

[Dosant]

@elastic/eui-team could you take a look please, I've tried a combination of different options and this is what I've ended up with. Not sure if this is the best way from UI and code perspective. Would appreciate any feedback.

One possibility is to also not show an email in case the user label is too long, but, as far as I understand, this is non-trivial as we don't know ahead of rendering if the label can fit.

[cee-chen]

From a code perspective, the flex CSS looks reasonable to me! Just curious, is there any reason to use an inline style here over css={{ minWidth: 0, flexBasis: 'auto' }}?

One possibility is to also not show an email in case the user label is too long, but, as far as I understand, this is non-trivial as we don't know ahead of rendering if the label can fit.

You could possibly "work around" that in CSS by letting the user label grow to 1 and the email shrink to 0 width with overflow hidden 🤔 It's up to you how important you think that is though. We might want to loop in a designer like @MichaelMarcialis for input on the UI portion of things.

[Dosant]

Thank you!

ust curious, is there any reason to use an inline style here over css={{ minWidth: 0, flexBasis: 'auto' }}

😅 I still don't have much experience with emotion and don't know many details about how it works under the hood. Intuitively I am afraid that it adds some overhead, so I am inclined to switch to inline styles when there is no need for emotion. Just curious to learn more:

• Is there any advantage in using emotion here (apart from consistency)?
• regarding the other comment with DRY Fix UsersProfilePopover doesn't handle long names well #182155 (comment), is there also a performance advantage in moving the styles up?

You could possibly "work around" that in CSS by letting the user label grow to 1 and the email shrink to 0 width with overflow hidden

I gave it a try by adding flexShrink:0 to the name (maybe I misunderstood). Couldn't make it work, because in this case name truncation doesn't work. I am probably missing something

[cee-chen]

I gave it a try by adding flexShrink:0 to the name (maybe I misunderstood). Couldn't make it work, because in this case name truncation doesn't work. I am probably missing something

Try this on line 343 instead:

              <EuiFlexItem css={{ maxWidth: '100%' }}>
                <EuiHighlight className="eui-textTruncate" search={searchValue}>

with just that small tweak you should be all set, the email will get completely hidden if the name is too long!

[cee-chen]

Intuitively I am afraid that it adds some overhead, so I am inclined to switch to inline styles when there is no need for emotion.

From my understanding in production Emotion is quite performant compared to dev and perf impact is negligible. To clarify, we're definitely in the microperf territory where this kind of thing only starts to matter in the thousands of renders, so it likely doesn't matter either way and it's definitely not a blocking comment. FWIW I do believe that rendering thousands of inline styles is significantly less performant than thousands of classNames.

Is there any advantage in using emotion here (apart from consistency)?

Specificity would be the biggest one - inline styles have way higher specificity than CSS, and can be frustrating and annoying for other devs to override. Probably doesn't super matter in this scenario other than as a general best practice.

regarding the other comment with DRY #182155 (comment), is there also a performance advantage in moving the styles up?

Very minor but yes. Again we're talking microperf territory here, so please don't take this article as gospel especially for a component with very few renders/rerenders, but here's some more info: https://engineering.deptagency.com/optimizing-emotion-in-react

I would consider your current usage at this point more than fine, with no need to pull out the css prop to a static const.

[Dosant]

Thank you! The trick works.
Thanks for taking the time explaining about emotion

[MichaelMarcialis]

We might want to loop in a designer like @MichaelMarcialis for input on the UI portion of things.

Apologies for the late reply. Generally speaking, I think truncation to ellipses is a fine way to address a lack of horizontal space. However, it would be unfortunate to lose all mention of an accompanying email address, if that is indeed what is happening here (as that may be important in helping identify users).

Is this always the maximum width of this filter's popover? Perhaps we can increase this a bit to offer a bit more breathing room? Additionally, off the top of my head I'd propose one of the following:

• Option 1: Move the optionally appearing email address to a line below the user name and align the text left (so it aligns nicely with the user name above it). In doing so, I'd also recommend reducing the font size to its extra small variant. This way, both pieces of text can have text truncation applied, without losing either entirely.

• Option 2: If it is preferable to keep the user name and email address on the same line, apply a reasonable maximum width to the email address container, have the user name container fill the remaining available horizontal space and set both to truncate their text (so both are visible/represented, even if truncated heavily).

Thoughts?

[azasypkin]

Option 1: Move the optionally appearing email address to a line below the user name and align the text left (so it aligns nicely with the user name above it). In doing so, I'd also recommend reducing the font size to its extra small variant. This way, both pieces of text can have text truncation applied, without losing either entirely.

Not a UX/UI person or anything, but I really like this option from a functional perspective :) It's surprising how many namesakes a large organization might have, and having visibility to email might be vital.

[Dosant]

Great feedback, let's create an issue (I'll create)

[elasticmachine]

Pinging @elastic/appex-sharedux (Team:SharedUX)

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 945e62f

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
cases	476.1KB	476.2KB	+158.0B
dashboard	408.9KB	409.1KB	+163.0B
eventAnnotationListing	210.3KB	210.4KB	+162.0B
filesManagement	102.2KB	102.4KB	+162.0B
graph	398.0KB	398.1KB	+162.0B
maps	3.0MB	3.0MB	+162.0B
security	575.1KB	575.3KB	+162.0B
securitySolution	13.7MB	13.7MB	+158.0B
visualizations	283.3KB	283.5KB	+162.0B
total			+1.4KB

History

• 💚 Build #207157 succeeded 365c377
• 💚 Build #206976 succeeded eec9e85

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

cc @Dosant

[azasypkin]

Looks good, thank you for the fix!