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
After
Page | Before | After |
---|---|---|
Mobile comment |
Local testing
- Enable
:gitlab_employee_badge
feature flag.bin/rails console
Feature.enable(:gitlab_employee_badge)
- In
lib/gitlab.rb
changeself.com?
to returntrue
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry - [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides - [-] Database guides
- [-] Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers Note: Except Edge due to this known Bootstrap bug: https://github.com/twbs/bootstrap/pull/19434 - [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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