Skip to content

Fix duplicate tooltip issue when hovering over status

What does this MR do?

This MR address two issues:

#25807 (closed)
#214089 (closed)

It splits the link that triggers the user popover up into two separate links. This allows us to have the emoji between John Smith and @johnsmith without being in the link that triggers the user popover. It also allows us to ensure that the "GitLab Employee Badge" is always next to the @username. It opens up opportunity for better stacking on mobile to improve readability.

Considerations

Performance

We needed to add mouseenter and mouseleave event listeners to the username link so the popover can be triggered by hovering over the @username. This doubles the number of event listeners that we previously had.

The upside to this approach is we are not mounting a second user popover to @username and we can take advantage of the popover caching that is already in place.

I wasn't sure of the best way to test the performance of this. After some research decided to use the Vue Dev Tools Performance tab. I rendered the application 10 times before and after and averaged the render time of note_header.vue.

Before (ms) After (ms)
90 97
89 88
93 88
84 92
88 87
85 99
98 85
83 84
91 86
85 89
88.6 89.5 Average (ms)

Open to other suggestions for benchmarking this change!

a11y

This MR splits one link into two that go to the same place. This adds one extra item to the tab order when navigating with the keyboard. How does this negatively affect a11y? Possible solution?

UX

The popover is bound to the user's name link (e.g. John Smith) which is now a smaller width because it does not contain @username. This shifts the popover to the left to center over just the user's name instead of the combined link. See gifs below for an example.

Screenshots

Before

before

After

after

Page Before After
Mobile comment Screen_Shot_2020-04-10_at_12.06.08_PM Screen_Shot_2020-04-10_at_12.02.28_PM

Local testing

  1. Enable :gitlab_employee_badge feature flag.
    • bin/rails console
    • Feature.enable(:gitlab_employee_badge)
  2. In lib/gitlab.rb change self.com? to return true

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Peter Hegman

Merge request reports

Loading