[UsersProfilePopover] Fix email sometimes is not visible #184318

Dosant · 2024-05-27T15:30:25Z

Summary

fix #182945

This fixes UsersProfilePopover to always show email on the 2nd line. To keep the list virtualized had to increase the height a bit for every row

Before:

After:

Also adds email in a label to make it appear in a browser tooltip

Also fixes <UserAvatarTip/> doesn't handle long names well #182561 and adds email to Avatar "name" so it appears in a native tooltip

Increase popover width in TableListView

The only concern that I have is that cases popover was working around that issue with email visibility by making the popover very wide, I wonder if this fix is OK for them and if we could reduce the width now? cc @cnasikas @MichaelMarcialis
The downside of this fix is that each row takes a bit more height.

packages/kbn-user-profile-components/src/user_profiles_selectable.tsx

elena-shostak · 2024-05-28T08:47:13Z

packages/kbn-user-profile-components/src/user_profiles_selectable.tsx

+                <EuiHighlight search={searchValue}>{displayName}</EuiHighlight>
+              </div>
+              {option.user.email && option.user.email !== displayName ? (
+                <div className="eui-textTruncate" css={{ fontSize: '0.8em', lineHeight: 1.2 }}>


may we just use EuiText component here? with color and size options

agree, looks better 👍

cnasikas · 2024-05-28T08:53:22Z

The only concern that I have is that cases popover was working around that issue with email visibility by making the popover very wide, I wonder if this fix is OK for them and if we could reduce the width now? cc @cnasikas @MichaelMarcialis
The downside of this fix is that each row takes a bit more height.

The new changes LGTM for Cases! About the width, I am not sure to be honest. I think it depends if we want to be consistent across Kibana. I will leave the decision to @MichaelMarcialis or @mdefazio.

…ofile-email

elena-shostak

LGTM!

elasticmachine · 2024-05-29T07:55:10Z

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

Dosant · 2024-06-03T08:34:00Z

@elasticmachine merge upstream

MichaelMarcialis · 2024-06-04T13:53:51Z

The only concern that I have is that cases popover was working around that issue with email visibility by making the popover very wide, I wonder if this fix is OK for them and if we could reduce the width now? cc @cnasikas @MichaelMarcialis
The downside of this fix is that each row takes a bit more height.

Yes, I think reducing the cases popover to a more reasonable width would be warranted with the improvements you've made.

…ofile-email

Dosant · 2024-06-04T15:07:57Z

Yes, I think reducing the cases popover to a more reasonable width would be warranted with the improvements you've made.

I am reducing to 400px to align with other popovers on that page

MichaelMarcialis

Thanks for putting this together, @Dosant. It looks great. I've left a few small comments below, but nothing worth holding you up for. Assuming you're able to address them, I'll approve now.

My first comment is regarding the assignee dropdown menu when creating a new case. It looks like this new stacking of display name and email wasn't applied here. Can/should we do so?

MichaelMarcialis · 2024-06-04T15:58:57Z

packages/kbn-user-profile-components/src/user_tooltip.tsx

+            <div>
+              <strong>{displayName}</strong>
+            </div>
+            {user.email && user.email !== displayName ? <div>{user.email}</div> : undefined}


Should we take the same approach in the tooltips that we are for the selectable lists? For example, should we:

Truncate both the display name and email to a single line.

Remove the strong element around the display name.

Reduce the email font size to 12px.

Doing so may be more consistent and yield a less crowded tooltip in situations with lengthy names/emails. Thoughts?

Truncate both the display name and email to a single line.

I initially did this but then reverted. I concluded that the idea of a tooltip is to show the full information, even if it is not that nice visually. So decided to better wrap, not truncate.

Remove the strong element around the display name.
Reduce the email font size to 12px.

agree, changed 👍

MichaelMarcialis · 2024-06-04T16:42:30Z

packages/kbn-user-profile-components/src/user_profiles_selectable.tsx

+                <EuiText
+                  size={'xs'}
+                  color={option.disabled ? 'disabled' : 'subdued'}
+                  className="eui-textTruncate"
+                >
+                  {searchValue ? (
+                    <EuiHighlight search={searchValue}>{option.user.email}</EuiHighlight>
+                  ) : (
+                    option.user.email
+                  )}
+                </EuiText>


On hover and focus of one of the users, the underline text decoration is blue for the subued-colored email. Any chance you could change it so the text-decoration-color for the email matches its text color?

Just checked that it isn't flexible, this would require a EUI change.
Text decoration is applied on the whole euiSelectableListItem__text and the text decoration color is not changed if I change it on a child . euiText with email

…ofile-email

Dosant

@MichaelMarcialis, thanks for the review, I improved what I could

My first comment is regarding the assignee dropdown menu when creating a new case. It looks like this new stacking of display name and email wasn't applied here. Can/should we do so?

Looks like this is a different component :(
I won't touch it

Also in that case because how wide the input is, it should fit the longer names nicely. Unfortunetly, not consistent with this other component

Dosant · 2024-06-05T10:40:15Z

packages/kbn-user-profile-components/src/user_profiles_selectable.tsx

+                <EuiText
+                  size={'xs'}
+                  color={option.disabled ? 'disabled' : 'subdued'}
+                  className="eui-textTruncate"
+                >
+                  {searchValue ? (
+                    <EuiHighlight search={searchValue}>{option.user.email}</EuiHighlight>
+                  ) : (
+                    option.user.email
+                  )}
+                </EuiText>


Just checked that it isn't flexible, this would require a EUI change.
Text decoration is applied on the whole euiSelectableListItem__text and the text decoration color is not changed if I change it on a child . euiText with email

Dosant · 2024-06-05T10:47:26Z

packages/kbn-user-profile-components/src/user_tooltip.tsx

+            <div>
+              <strong>{displayName}</strong>
+            </div>
+            {user.email && user.email !== displayName ? <div>{user.email}</div> : undefined}


Truncate both the display name and email to a single line.

I initially did this but then reverted. I concluded that the idea of a tooltip is to show the full information, even if it is not that nice visually. So decided to better wrap, not truncate.

Remove the strong element around the display name.
Reduce the email font size to 12px.

agree, changed 👍

kibana-ci · 2024-06-05T12:12:09Z

💚 Build Succeeded

Buildkite Build
Commit: 3bace9b

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`	479.3KB	479.2KB	-69.0B
`dashboard`	495.9KB	495.7KB	-191.0B
`eventAnnotationListing`	295.4KB	295.2KB	-192.0B
`filesManagement`	104.4KB	104.2KB	-192.0B
`graph`	396.9KB	396.7KB	-192.0B
`maps`	3.0MB	3.0MB	-192.0B
`observabilityAIAssistantApp`	151.3KB	151.4KB	+74.0B
`searchPlayground`	166.5KB	166.6KB	+74.0B
`security`	587.2KB	587.0KB	-181.0B
`securitySolution`	13.6MB	13.6MB	-63.0B
`visualizations`	285.8KB	285.6KB	-190.0B
total			-1.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`security`	69.0KB	69.1KB	+111.0B

History

💛 Build #213908 was flaky 83c1dd1
💛 Build #213476 was flaky ca29d0b
💛 Build #212581 was flaky c4544d4
💚 Build #212552 succeeded aeaaf91
💛 Build #212437 was flaky 639d5b6

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

cc @Dosant

wip

639d5b6

Dosant added release_note:skip Skip the PR/issue when compiling release notes Feature:Security/User Profile labels May 27, 2024

Dosant requested a review from MichaelMarcialis May 27, 2024 15:30

Dosant requested review from a team as code owners May 27, 2024 15:30

elena-shostak reviewed May 28, 2024

View reviewed changes

sebelga approved these changes May 28, 2024

View reviewed changes

Dosant added 2 commits May 28, 2024 14:27

more fixes

80341d8

Merge branch 'main' of github.com:elastic/kibana into d/2024-05-27-pr…

aeaaf91

…ofile-email

Dosant requested a review from elena-shostak May 28, 2024 12:31

Dosant self-assigned this May 28, 2024

Dosant added 2 commits May 28, 2024 15:58

wrap instead of truncate

008ed6c

fix test

c4544d4

elena-shostak approved these changes May 28, 2024

View reviewed changes

Dosant requested a review from a team May 29, 2024 07:54

Dosant added the Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) label May 29, 2024

Merge branch 'main' into d/2024-05-27-profile-email

ca29d0b

Merge branch 'main' of github.com:elastic/kibana into d/2024-05-27-pr…

03531ec

…ofile-email

reduce cases assignees width 520 -> 400

83c1dd1

Dosant requested a review from a team as a code owner June 4, 2024 15:08

cnasikas approved these changes Jun 4, 2024

View reviewed changes

MichaelMarcialis approved these changes Jun 4, 2024

View reviewed changes

Merge branch 'main' of github.com:elastic/kibana into d/2024-05-27-pr…

fa7e47d

…ofile-email

Dosant commented Jun 5, 2024

View reviewed changes

review improvements

3bace9b

Dosant merged commit db9e530 into elastic:main Jun 5, 2024
19 checks passed

kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels Jun 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UsersProfilePopover] Fix email sometimes is not visible #184318

[UsersProfilePopover] Fix email sometimes is not visible #184318

Dosant commented May 27, 2024 •

edited

Loading

elena-shostak May 28, 2024

Dosant May 28, 2024

cnasikas commented May 28, 2024 •

edited

Loading

elena-shostak left a comment

elasticmachine commented May 29, 2024

Dosant commented Jun 3, 2024

MichaelMarcialis commented Jun 4, 2024

Dosant commented Jun 4, 2024

MichaelMarcialis left a comment

MichaelMarcialis Jun 4, 2024

Dosant Jun 5, 2024

MichaelMarcialis Jun 4, 2024

Dosant Jun 5, 2024

Dosant left a comment

Dosant Jun 5, 2024

Dosant Jun 5, 2024

kibana-ci commented Jun 5, 2024

[UsersProfilePopover] Fix email sometimes is not visible #184318

[UsersProfilePopover] Fix email sometimes is not visible #184318

Conversation

Dosant commented May 27, 2024 • edited Loading

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cnasikas commented May 28, 2024 • edited Loading

elena-shostak left a comment

Choose a reason for hiding this comment

elasticmachine commented May 29, 2024

Dosant commented Jun 3, 2024

MichaelMarcialis commented Jun 4, 2024

Dosant commented Jun 4, 2024

MichaelMarcialis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dosant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Jun 5, 2024

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Dosant commented May 27, 2024 •

edited

Loading

cnasikas commented May 28, 2024 •

edited

Loading