jenkins
jenkins copied to clipboard
Replace YUI tooltips with Tippy.js
This is the first of two PRs which intend to replace the existing YUI implementation of tooltips and menus with Tippy.js. This PR focuses solely on tooltips and brings native support for Tippy.js to Jenkins.
https://user-images.githubusercontent.com/43062514/160298962-5e39f965-a868-4e0a-bc3a-78bd68fe0481.mov
This brings several advantages:
- Better experience for users, animations, better placement engine
- Cleaner and better defined codebase
- Tippy is far more flexible than YUI, so we can be more creative with our designs
- Far easier to find support online
- YUI 2 is also abandoned since 2011
This implementation intends to maintain 100% backwards compatibility with the existing implementation of tooltips, it also introduces a new way to host HTML content in tooltips which is used for the project status tooltip using html-tooltip=" ... "
This change is documented in the updated Jenkins Design Library plugin.
Proposed changelog entries
- Update appearance and framework for tooltips
For plugin developers
- For developers that are using Tippy.js as part of their plugin, they can now remove the dependency and instead rely on the Jenkins implementations (
tooltip="hello world"
orhtml-tooltip="<p>hello world</p>"
)
Proposed upgrade guidelines
N/A
Submitter checklist
- [ ] (If applicable) Jira issue is well described
- [ ] Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change) and are in the imperative mood. Examples
- Fill-in the
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
- Fill-in the
- [ ] Appropriate autotests or explanation to why this change has no tests
- [ ] New public classes, fields, and methods are annotated with
@Restricted
or have@since TODO
Javadoc, as appropriate. - [ ] For dependency updates: links to external changelogs and, if possible, full diffs
Desired reviewers
@mention
Maintainer checklist
Before the changes are marked as ready-for-merge
:
- [ ] There are at least 2 approvals for the pull request and no outstanding requests for change
- [ ] Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
- [ ] Changelog entries in the PR title and/or
Proposed changelog entries
are accurate, human-readable, and in the imperative mood - [ ] Proper changelog labels are set so that the changelog can be generated automatically
- [ ] If the change needs additional upgrade steps from users,
upgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example) - [ ] If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as
lts-candidate
to be considered (see query).
- HTML doesn't seem to be handled properly here. I'm not sure if this is a special occasion, because the tooltip is provided as translation string in a .properties file, where
<br>
is not escaped at all:![]()
- Tooltips are shown twice on this maven project if it's still building
![]()
- The history widget displays an empty tooltip if the job didn't complete yet to show the overall time it took:
![]()
This brings several advantages:
Also worth to note that YUI 3 is abandoned since 2014.
Great finds! They should all be fixed now :)
YUI 3 is also abandoned since 2014
Wait, why do we care? We're on 2.x 😎 :trollface:
HTML doesn't seem to be handled properly here.
IMO we should consider making all tooltips plain text if we can get away with it. They've been a common source of XSS issues in the past. Of course, some minimal formatting ability may need to remain, like newlines.
CC @Wadeck
YUI 3 is also abandoned since 2014
Wait, why do we care? We're on 2.x 😎 :trollface:
From 2011 by the looks of it not that I can find the source of the latest upgrade, 2.9.0 doesn't seem to be in the repo
HTML doesn't seem to be handled properly here.
IMO we should consider making all tooltips plain text if we can get away with it. They've been a common source of XSS issues in the past. Of course, some minimal formatting ability may need to remain, like newlines.
CC @Wadeck
Looking at the code only <br>
has been added by default.
You can opt-in to html processing with html-tooltip.
- I'm not sure if this is intended, but the warnings-ng plugin overview in columns does now unfold into the column itself stretching it a lot, rather than floating around somewhere near the actual issue # it introduces.
Maybe my fix in https://github.com/jenkinsci/warnings-ng-plugin/pull/1206 helps. I did not yet release it because it bumps the LTS version to the latest one but hopefully that should work out.
Additionally: we need to check compatibility with the plugin views that are using Bootstrap since they depend on Popper as well. We need to make sure that Popper is just included once in our pages.
- I'm not sure if this is intended, but the warnings-ng plugin overview in columns does now unfold into the column itself stretching it a lot, rather than floating around somewhere near the actual issue # it introduces.
Maybe my fix in jenkinsci/warnings-ng-plugin#1206 helps. I did not yet release it because it bumps the LTS version to the latest one but hopefully that should work out.
It solely utilizes the modernized table view (in a sharp box 👀) but the position didn't change:
Yet on the other hand, analyzer tooltips work out fine
Repeatable element don't seem to be triggering tooltips:
YUI currently overrides title
to the value of tooltip
if it's set on an element, do we want to continue this behaviour going forward?
- I'm not sure if this is intended, but the warnings-ng plugin overview in columns does now unfold into the column itself stretching it a lot, rather than floating around somewhere near the actual issue # it introduces.
Maybe my fix in jenkinsci/warnings-ng-plugin#1206 helps. I did not yet release it because it bumps the LTS version to the latest one but hopefully that should work out.
Additionally: we need to check compatibility with the plugin views that are using Bootstrap since they depend on Popper as well. We need to make sure that Popper is just included once in our pages.
warnings-ng
would need changes like this file https://github.com/jenkinsci/jenkins/blob/68b0aea0e62a1ca16dd88f9585778c25fcdda03d/core/src/main/resources/lib/hudson/buildHealth.jelly - essentially moving the table to its own var then setting html-tooltip
to that value.
If no agent is available, tooltips are stacked for every attempt to trigger a build:
Though even if they're split into separate lines, it looks odd to have +1 message for every attempt, considering you don't enqueue several builds in that way, but that may be a different issue.
Would you mind rechecking this? I can't seem to reproduce it locally but I have made some changes to that area.
- Pipeline steps' tooltips are no longer attached to the step
![]()
Opened a small PR to address that and a couple other tiny things, cheers 👍
https://github.com/jenkinsci/workflow-support-plugin/pull/158
If no agent is available, tooltips are stacked for every attempt to trigger a build: Though even if they're split into separate lines, it looks odd to have +1 message for every attempt, considering you don't enqueue several builds in that way, but that may be a different issue. Would you mind rechecking this? I can't seem to reproduce it locally but I have made some changes to that area.
New entries are appended properly now, I attached the steps below demonstrating what I mean by "+1 message for every attempt".
https://user-images.githubusercontent.com/13383509/161003876-deebd727-3047-40d0-b403-45debc8dee84.mp4
If no agent is available, tooltips are stacked for every attempt to trigger a build: Though even if they're split into separate lines, it looks odd to have +1 message for every attempt, considering you don't enqueue several builds in that way, but that may be a different issue. Would you mind rechecking this? I can't seem to reproduce it locally but I have made some changes to that area.
New entries are appended properly now, I attached the steps below demonstrating what I mean by "+1 message for every attempt".
2022-03-31.09-39-12.mp4
Ah seems like an existing issue - managed to reproduce it on master
.

:P
I saw that the tooltips have a background slightly transparent. In some cases, it could make it harder to see the content of the tooltip, no?
In this example, I had to look a bit more closely than on other places.
I saw that the tooltips have a background slightly transparent. In some cases, it could make it harder to see the content of the tooltip, no?
In this example, I had to look a bit more closely than on other places.
Are you using Firefox by any chance? I'm using backdrop-filter
to blur anything behind the tooltips but I don't think Firefox supports it atm, I'll put in a fix for that.
Are you using Firefox by any chance?
yes, Firefox 98.0. Thanks.
New entries are appended properly now, I attached the steps below demonstrating what I mean by "+1 message for every attempt". 2022-03-31.09-39-12.mp4
Ah seems like an existing issue - managed to reproduce it on
master
.
Deliberate; a build can have any number of causes. Usually it is more useful than this, e.g. when it's triggered by the cron schedule but while it's in the queue, you click "Build Now", it's triggered both by the schedule and by you (and you probably accelerated its execution, as Build Now does not wait in the queue if it doesn't have to).
Considering the PR is in a solid state, I fired up ATH and BOM tests: 🔴 ATH shows various failures related to YUI: https://ci.jenkins.io/job/Core/job/acceptance-test-harness/job/PR-759/15/ 🟢 BOM fails too, but Jesse already filed a PR to matrix-auth to take care of that, that's not related to this PR: https://ci.jenkins.io/job/Tools/job/bom/job/PR-992/1/
For plugin developers
This should mention the need to register tooltips manually as done in the linked matrix-auth PR.
For plugin developers
This should mention the need to register tooltips manually as done in the linked matrix-auth PR.
Added
This change is documented in the updated Jenkins Design Library plugin.
html-tooltip
is not documented there currently, I think it was backed out of the main PR to only show released content
This change is documented in the updated Jenkins Design Library plugin.
html-tooltip
is not documented there currently, I think it was backed out of the main PR to only show released content
I'll add it back 👍
This change is documented in the updated Jenkins Design Library plugin.
html-tooltip
is not documented there currently, I think it was backed out of the main PR to only show released contentI'll add it back 👍
Could you mention the need to think about the source of the content and the need to potentially sanitize parts (perhaps even with example)? Thanks :)
Could you mention the need to think about the source of the content and the need to potentially sanitize parts (perhaps even with example)? Thanks :)
The warning was left on the page when the docs were removed 😆, see https://weekly.ci.jenkins.io/design-library/Tooltips/
Tooltips being interactable makes UI like the the permission grid of matrix-auth annoying to use. Move the cursor up a row and instead of being able to interact with checkboxes, you can copy & paste the text of the tooltip. This seems like a niche use case, so could we disable this behavior?
Before
https://user-images.githubusercontent.com/1831569/166409789-08d8d22d-4ce6-413b-abd8-0120f47cc3d5.mov
After
https://user-images.githubusercontent.com/1831569/166409799-b151db79-949d-4816-bbb4-0a04e066fd6a.mov
I cannot get the tooltip on the matrix-auth configuration to work on Firefox 100 now. To be specific, videos from @daniel-beck on https://github.com/jenkinsci/jenkins/pull/6408#issuecomment-1115761460 are not what I have locally.
I can see the <td/>
have data-tooltip-{enabled,disabled}
and the <input/>
have a tooltip
attribute, but there is no tooltip. There is no errors/warnings in the javascript console.
@alecharp are you running with this PR? https://github.com/jenkinsci/matrix-auth-plugin/pull/120
You are correct @timja, with the correct version of the plugin it's working fine. I missed that we need downstream pull requests.
Are there plans to look for plugins expecting to have HTML support in tooltips, and adapting the more popular ones? I know of https://plugins.jenkins.io/compact-columns/ (not that popular but an example of a resulting regression) which relies on this:
Similarly, like in matrix-auth
, the opposite situation may exist where content is escaped, expecting it to be interpreted as HTML, and then it's double-escaped.
Non-blocking (splitting HTML and non-HTML explicitly should prevent some problems in the future), just curious.
Are there plans to look for plugins expecting to have HTML support in tooltips, and adapting the more popular ones? I know of https://plugins.jenkins.io/compact-columns/ (not that popular but an example of a resulting regression) which relies on this:
![]()
Similarly, like in
matrix-auth
, the opposite situation may exist where content is escaped, expecting it to be interpreted as HTML, and then it's double-escaped.Non-blocking (splitting HTML and non-HTML explicitly should prevent some problems in the future), just curious.
There's nothing concrete planned but I'm more than happy to take a look at/fix any issues that arise with plugins. I'll take a look at that one now.
Update: Opened a PR here for that: https://github.com/jenkinsci/compact-columns-plugin/pull/52