[#55581] popover for user information on hover #17255

EinLama · 2024-11-22T08:46:17Z

Ticket

https://community.openproject.org/wp/55581

What are you trying to accomplish?

User hover cards when hovering over the avatar. I tried also showing the hover card when hovering over the name, but it felt wonky and overly verbose. EDIT: as discussed in the frontend daily, the wonkyness is no longer present with an increased delay before showing a hover card. So showing it for names is an option now. We decided that I will create an open point for this topic so that we can decide how to proceed with this. In case we want to add this, I would supply another PR for that, as it will be much smaller in scope.

Please take extra care when reviewing this. I have tried to check every place where a user principal is rendered, but might have missed some due to the sheer number of them.

Screenshots

user.popover.information.mov

Note: the video is not completely accurate. The arrow pointing downwards towards the avatar has been removed because it's not possible to position it reliably.

What approach did you choose and why?

Reused the HoverCard component from work packages and made some adjustments.

Some changes made to the trigger service (Typescript) also affect the work packages. This includes a longer delay before a hover card appears, as well as a slightly longer delay before it closes. Bugs happening when you quickly over multiple different hover cards have been fixed.

The AvatarComponent as well as the op-principal-component now support an option for enabling hover cards. By default, the popover is enabled. I had to disable the popover in some places where it caused hard-to-fix bugs, such as: it is not possible to render another spot modal within an existing spot modal.

These places include:

the global search result dropdown
the time tracking modal
the work package share modal

Merge checklist

Added/updated tests
Added/updated documentation in Lookbook (patterns, previews, etc)
Tested major browsers (Chrome, Firefox, Edge, ...)

config/initializers/permissions.rb

HDinger

Hi @EinLama

as discussed, the amount of cards feels rather annoying. I'd like the challenge the requirement that the card should really be shown all the time. Alternatively, the delay must be larger. But that has to be discussed with the product team.

Until then, here are the first findings from clicking through:

Hovering a placeholder user leads to an error
As a non-admin user, there is an error as well due to a missing permission
Opening the share modal on a WP creates a black overlay
When the LogTime modal on a WP is opened, and the user dropdown is opened: Hovering the avatar, will close the logTime modal
The emailAdress is supposed to smaller (12px)
When having a very long name, it overflows the card without truncation or wrapping:

When the hovercard is opened at the very bottom of the page, it closes immideatly

Bildschirmaufnahme.2024-11-22.um.14.55.49.mov

For the record, there are some places where the card is not shown yet. But as said above, I'd wait until the decision of the product team

Team planner module
Admin/user table
Project overview / project attribute of type user
Project activity page
Users page (/users/:id)
My Page / News widget
Multi-User custom field

app/components/users/hover_card_component.html.erb

HDinger

It feels like you could save a lot of code by turning the default for the hoverCards to true. Is there a reason it is set to false?

app/components/users/avatar_component.rb

app/components/users/hover_card_component.html.erb

HDinger · 2024-11-25T08:51:56Z

app/components/users/hover_card_component.rb

+  # or
+  # "Member of group1, group2 and 3 more"
+  # The latter string is cut off since the complete list of group names would exceed the allowed `max_length`.
+  def group_membership_summary(max_length = 40)


Where is the value 40 coming from?

A true magic number 🔮🧙🏻‍♂️ found by, uh... guessing and trying it out! 🪄

Seemed sensible when used with the English translation and should look okay with most other languages. The caveat is that only the group names are considered when deciding where to shorten the list. To make it perfect, we'd have to load the translation first, factor it's length into the calculation and THEN decide where to cut off the group list. I figured the value 40 is good enough in most cases and strikes a balance between pleasant looks and code complexity :)

app/components/users/hover_card_component.sass

HDinger · 2024-11-25T09:47:29Z

frontend/src/app/shared/components/fields/display/field-types/user-display-field.module.ts

@@ -48,7 +48,7 @@ export class UserDisplayField extends DisplayField {
        element,
        this.attribute,
        { hide: false, link: false },
-        { hide: false, size: 'medium' },
+        { hide: false, size: 'medium', hoverCard: { url: this.attribute.url } },


ESLint is somehow complaining correctly here.this.attribute can be anything, so we do not know for sure whether a url is defined. Ideally this case would at least be catched before.

There were other issues with this class, unrelated to this PR. I have made an attempt at fixing them. The compiler is happy, so far is the runtime. Will observe if this causes problems somewhere in the application.

frontend/src/app/core/setup/globals/global-listeners/hover-card-trigger.service.ts

frontend/src/app/shared/components/principal/principal-renderer.service.ts

app/components/users/hover_card_component.sass

EinLama · 2024-11-26T17:02:59Z

Thank you, @HDinger. I have fixed most of the issues and findings. I found another place that triggers erratic behavior:

global search closes when hovering over an avatar (e.g. work package authors)

It is closer to what we want to achieve.

This is only a draft since the links are still missing. But it gives a good first impression of what's to come.

We might have to patch a bigger possible size here later, the default size is currently the biggest and it looks a bit lost in the hover card.

Previously, it was attached to the wrapper, which led to weird behavior. With the new approach, it looks more natural while fitting the design better. Sneaked in some improvements such as using the hover card path helper.

compressed via ImageOptim

This was a hard one to debug. When you hover over an avatar as soon as the page loads, you will hover over the fallback first. Within the timeout, the actual avatar image loads and replaces the fallback. After that, the timeout of the hover trigger runs out and displays the hover cards, but does no longer have an element to position relative to. So, we check for the existence of the hovered element before showing a hover card to fix this.

If you quickly move from one hover target to the next, we know reset the hover card component into the correct state so that you can open the next hover card. Just keep hovering.

…-hover

oliverguenther · 2024-12-02T13:23:18Z

app/components/users/hover_card_component.sass

+  gap: 1rem
+  overflow: hidden
+
+  .op-user-hover-card--info


Suggested change

.op-user-hover-card--info

&--info

this could be just &--info. This ensures that you're using the BEM style

Very nice, thanks! The previous class definitions were very cumbersome. Much better like this.

app/components/users/hover_card_component.sass

oliverguenther

Behavior-wise, LGTM. I have some minor remarks

frontend/src/app/shared/components/principal/principal-renderer.service.ts

frontend/src/app/core/setup/globals/global-listeners/hover-card-trigger.service.ts

This was a leftover that can now be removed.

EinLama added feature pullpreview labels Nov 22, 2024

EinLama requested a review from HDinger November 22, 2024 08:46

EinLama commented Nov 22, 2024

View reviewed changes

config/initializers/permissions.rb Outdated Show resolved Hide resolved

EinLama force-pushed the feature/55581-popover-for-user-information-on-hover branch from 1f5454c to 4653b53 Compare November 22, 2024 13:55

HDinger requested changes Nov 22, 2024

View reviewed changes

app/components/users/hover_card_component.html.erb Outdated Show resolved Hide resolved

EinLama force-pushed the feature/55581-popover-for-user-information-on-hover branch from f3869f7 to 95183a7 Compare November 22, 2024 14:25

HDinger reviewed Nov 25, 2024

View reviewed changes

EinLama force-pushed the feature/55581-popover-for-user-information-on-hover branch 4 times, most recently from 709ca64 to eba7ab8 Compare November 26, 2024 14:42

EinLama force-pushed the feature/55581-popover-for-user-information-on-hover branch 4 times, most recently from 00addee to 6ce763e Compare November 29, 2024 08:09

EinLama added 13 commits November 29, 2024 15:19

[#55581] enforce hover card in project member list for now

3249503

[#55581] unwrap quotes for angular components

b1e38ec

[#55581] return early

7e72f62

[#55581] Users::HoverCardComponent

fe106c3

[#55581] prefer flex layout

89587fd

It is closer to what we want to achieve.

WIP

e7c16e7

[#55581] offer hover card trigger switch in avatar helper

8354574

[#55581] first draft for triangle pointing at hover source

ef22234

[#55581] construct project membership string, link to profile

3f2ab1d

This is only a draft since the links are still missing. But it gives a good first impression of what's to come.

avatar details

ca90dbe

We might have to patch a bigger possible size here later, the default size is currently the biggest and it looks a bit lost in the hover card.

[#55581] show email if allowed to do so

277b9dc

[#55581] link project names to projects

1f554f2

[#55581] attach hover card attributes to avatar image

4f6fd0d

Previously, it was attached to the wrapper, which led to weird behavior. With the new approach, it looks more natural while fitting the design better. Sneaked in some improvements such as using the hover card path helper.

EinLama added 10 commits November 29, 2024 15:19

[#55581] do not show the hover card within the log time dialog

8fe734b

[#55581] ensure the URL is a string

3db68a4

[#55581] disable hover cards in global search

5496530

[#55581] lookbook proposal

69a2eee

[#55581] add exemplary user hover card to lookbook

b4fed15

[#55581] reduce filesize of wp hover card image by 45%

7661b57

compressed via ImageOptim

[#55581] user generic error message for missing users

f60f744

[#55581] do not try to access non-existent field

ba84c3f

[#55581] animate hover card slightly to make it feel less awkward

8a1f7b9

EinLama force-pushed the feature/55581-popover-for-user-information-on-hover branch from e6aca92 to 8a1f7b9 Compare November 29, 2024 14:20

EinLama and others added 3 commits November 29, 2024 16:00

[#55581] deal with multiple hover triggers next to each other

01d9bde

If you quickly move from one hover target to the next, we know reset the hover card component into the correct state so that you can open the next hover card. Just keep hovering.

[#55581] prevent background layer from darkening in multi nested dialogs

ab9ea70

Merge branch 'dev' into feature/55581-popover-for-user-information-on…

f6ddf7f

…-hover

EinLama marked this pull request as ready for review December 2, 2024 09:25

EinLama added the needs review label Dec 2, 2024

oliverguenther reviewed Dec 2, 2024

View reviewed changes

app/components/users/hover_card_component.sass Outdated Show resolved Hide resolved

oliverguenther reviewed Dec 2, 2024

View reviewed changes

app/components/users/hover_card_component.sass Outdated Show resolved Hide resolved

oliverguenther requested changes Dec 2, 2024

View reviewed changes

frontend/src/app/shared/components/principal/principal-renderer.service.ts Outdated Show resolved Hide resolved

frontend/src/app/core/setup/globals/global-listeners/hover-card-trigger.service.ts Outdated Show resolved Hide resolved

EinLama added 3 commits December 2, 2024 14:37

[#55581] use BEM style class definitions

2462494

[#55581] use PathHelperService to construct hover card url

4c17609

[#55581] remove unneede clean up of angular double quotes

818876a

This was a leftover that can now be removed.

EinLama requested review from oliverguenther and HDinger December 2, 2024 13:54

oliverguenther approved these changes Dec 4, 2024

View reviewed changes

oliverguenther removed the needs review label Dec 4, 2024

EinLama merged commit cc72da4 into dev Dec 4, 2024
14 of 15 checks passed

EinLama deleted the feature/55581-popover-for-user-information-on-hover branch December 4, 2024 10:29

github-actions bot removed the pullpreview label Dec 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#55581] popover for user information on hover #17255

[#55581] popover for user information on hover #17255

EinLama commented Nov 22, 2024 •

edited

Loading

HDinger left a comment •

edited by EinLama

Loading

HDinger left a comment

HDinger Nov 25, 2024

EinLama Nov 25, 2024 •

edited

Loading

HDinger Nov 25, 2024

EinLama Nov 27, 2024

EinLama commented Nov 26, 2024 •

edited

Loading

oliverguenther Dec 2, 2024

EinLama Dec 2, 2024

oliverguenther left a comment

[#55581] popover for user information on hover #17255

[#55581] popover for user information on hover #17255

Conversation

EinLama commented Nov 22, 2024 • edited Loading

Ticket

What are you trying to accomplish?

Screenshots

What approach did you choose and why?

Merge checklist

HDinger left a comment • edited by EinLama Loading

Choose a reason for hiding this comment

HDinger left a comment

Choose a reason for hiding this comment

HDinger Nov 25, 2024

Choose a reason for hiding this comment

EinLama Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

HDinger Nov 25, 2024

Choose a reason for hiding this comment

EinLama Nov 27, 2024

Choose a reason for hiding this comment

EinLama commented Nov 26, 2024 • edited Loading

oliverguenther Dec 2, 2024

Choose a reason for hiding this comment

EinLama Dec 2, 2024

Choose a reason for hiding this comment

oliverguenther left a comment

Choose a reason for hiding this comment

EinLama commented Nov 22, 2024 •

edited

Loading

HDinger left a comment •

edited by EinLama

Loading

EinLama Nov 25, 2024 •

edited

Loading

EinLama commented Nov 26, 2024 •

edited

Loading