Skip to content

Commit 9be90a5

Browse files
authored
Use a general approach to show tooltip, fix temporary tooltip bug (#23574)
## TLDR * Improve performance: lazy creating the tippy instances. * Transparently support all "tooltip" elements, no need to call `initTooltip` again and again. * Fix a temporary tooltip re-entrance bug, which causes showing temp content forever. * Upgrade vue3-calendar-heatmap to 2.0.2 with lazy tippy init (initHeatmap time decreases from 100ms to 50ms) ## Details ### The performance Creating a lot of tippy tooltip instances is expensive. This PR doesn't create all tippy tooltip instances, instead, it only adds "mouseover" event listener to necessary elements, and then switches to the tippy tooltip ### The general approach for all tooltips Before, dynamically generated tooltips need to be called with `initTooltip`. After, use MutationObserver to: * Attach the event listeners to newly created tooltip elements, work for Vue (easier than before) * Catch changed attributes and update the tooltip content (better than before) It does help a lot, eg: https://github.com/go-gitea/gitea/blob/1a4efa0ee9a49d48549be7479a46be133b9bc260/web_src/js/components/PullRequestMergeForm.vue#L33-L36 ### Temporary tooltip re-entrance bug To reproduce, on try.gitea.io, click the "copy clone url" quickly, then the tooltip will be "Copied!" forever. After this PR, with the help of `attachTippyTooltip`, the tooltip content could be reset to the default correctly. ### Other changes * `data-tooltip-content` is preferred from now on, the old `data-content` may cause conflicts with other modules. * `data-placement` was only used for tooltip, so it's renamed to `data-tooltip-placement`, and removed from `createTippy`.
1 parent e7f0bcf commit 9be90a5

13 files changed

+111
-64
lines changed

package-lock.json

+6-6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
"vue": "3.2.47",
4444
"vue-bar-graph": "2.0.0",
4545
"vue-loader": "17.0.1",
46-
"vue3-calendar-heatmap": "2.0.0",
46+
"vue3-calendar-heatmap": "2.0.2",
4747
"webpack": "5.76.2",
4848
"webpack-cli": "5.0.1",
4949
"workbox-routing": "6.5.4",

templates/repo/issue/view_content/sidebar.tmpl

+2-2
Original file line numberDiff line numberDiff line change
@@ -661,9 +661,9 @@
661661
{{if and (not (eq .Issue.PullRequest.HeadRepo.FullName .Issue.PullRequest.BaseRepo.FullName)) .CanWriteToHeadRepo}}
662662
<div class="ui divider"></div>
663663
<div class="inline field">
664-
<div class="ui checkbox" id="allow-edits-from-maintainers"
664+
<div class="ui checkbox tooltip" id="allow-edits-from-maintainers"
665665
data-url="{{.Issue.Link}}"
666-
data-prompt-tip="{{.locale.Tr "repo.pulls.allow_edits_from_maintainers_desc"}}"
666+
data-tooltip-content="{{.locale.Tr "repo.pulls.allow_edits_from_maintainers_desc"}}"
667667
data-prompt-error="{{.locale.Tr "repo.pulls.allow_edits_from_maintainers_err"}}"
668668
>
669669
<label><strong>{{.locale.Tr "repo.pulls.allow_edits_from_maintainers"}}</strong></label>

templates/repo/sub_menu.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
</div>
4141
<a class="ui segment language-stats">
4242
{{range .LanguageStats}}
43-
<div class="bar tooltip" style="width: {{.Percentage}}%; background-color: {{.Color}}" data-placement="top" data-content={{.Language}}>&nbsp;</div>
43+
<div class="bar tooltip" style="width: {{.Percentage}}%; background-color: {{.Color}}" data-tooltip-placement="top" data-tooltip-content={{.Language}}>&nbsp;</div>
4444
{{end}}
4545
</a>
4646
{{end}}

web_src/js/components/DashboardRepoList.vue

-4
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@
147147
<script>
148148
import {createApp, nextTick} from 'vue';
149149
import $ from 'jquery';
150-
import {initTooltip} from '../modules/tippy.js';
151150
import {SvgIcon} from '../svg.js';
152151
153152
const {appSubUrl, assetUrlPrefix, pageData} = window.config;
@@ -238,9 +237,6 @@ const sfc = {
238237
mounted() {
239238
const el = document.getElementById('dashboard-repo-list');
240239
this.changeReposFilter(this.reposFilter);
241-
for (const elTooltip of el.querySelectorAll('.tooltip')) {
242-
initTooltip(elTooltip);
243-
}
244240
$(el).find('.dropdown').dropdown();
245241
nextTick(() => {
246242
this.$refs.search.focus();

web_src/js/components/DiffFileList.vue

-12
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
</template>
2222

2323
<script>
24-
import {initTooltip} from '../modules/tippy.js';
2524
import {doLoadMoreFiles} from '../features/repo-diff.js';
2625
2726
const {pageData} = window.config;
@@ -30,17 +29,6 @@ export default {
3029
data: () => {
3130
return pageData.diffFileInfo;
3231
},
33-
watch: {
34-
fileListIsVisible(newValue) {
35-
if (newValue === true) {
36-
this.$nextTick(() => {
37-
for (const el of this.$refs.root.querySelectorAll('.tooltip')) {
38-
initTooltip(el);
39-
}
40-
});
41-
}
42-
}
43-
},
4432
mounted() {
4533
document.getElementById('show-file-list-btn').addEventListener('click', this.toggleFileList);
4634
},

web_src/js/components/DiffFileTreeItem.vue

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<template>
2-
<div v-show="show" class="tooltip" :title="item.name">
2+
<div v-show="show" :title="item.name">
33
<!--title instead of tooltip above as the tooltip needs too much work with the current methods, i.e. not being loaded or staying open for "too long"-->
44
<div class="item" :class="item.isFile ? 'filewrapper gt-p-1' : ''">
55
<!-- Files -->

web_src/js/features/common-global.js

-7
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import {createDropzone} from './dropzone.js';
55
import {initCompColorPicker} from './comp/ColorPicker.js';
66
import {showGlobalErrorMessage} from '../bootstrap.js';
77
import {handleGlobalEnterQuickSubmit} from './comp/QuickSubmit.js';
8-
import {initTooltip} from '../modules/tippy.js';
98
import {svg} from '../svg.js';
109
import {hideElem, showElem, toggleElem} from '../utils/dom.js';
1110

@@ -66,12 +65,6 @@ export function initGlobalButtonClickOnEnter() {
6665
});
6766
}
6867

69-
export function initGlobalTooltips() {
70-
for (const el of document.getElementsByClassName('tooltip')) {
71-
initTooltip(el);
72-
}
73-
}
74-
7568
export function initGlobalCommon() {
7669
// Undo Safari emoji glitch fix at high enough zoom levels
7770
if (navigator.userAgent.match('Safari')) {

web_src/js/features/contextpopup.js

+1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export function initContextPopups() {
3030

3131
createTippy(this, {
3232
content: el,
33+
placement: 'top-start',
3334
interactive: true,
3435
interactiveBorder: 5,
3536
onShow: () => {

web_src/js/features/repo-diff.js

-5
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import {initCompReactionSelector} from './comp/ReactionSelector.js';
33
import {initRepoIssueContentHistory} from './repo-issue-content.js';
44
import {validateTextareaNonEmpty} from './comp/EasyMDE.js';
55
import {initViewedCheckboxListenerFor, countAndUpdateViewedFiles} from './pull-view-file.js';
6-
import {initTooltip} from '../modules/tippy.js';
76

87
const {csrfToken} = window.config;
98

@@ -60,10 +59,6 @@ export function initRepoDiffConversationForm() {
6059
const $newConversationHolder = $(await $.post($form.attr('action'), formDataString));
6160
const {path, side, idx} = $newConversationHolder.data();
6261

63-
$newConversationHolder.find('.tooltip').each(function () {
64-
initTooltip(this);
65-
});
66-
6762
$form.closest('.conversation-holder').replaceWith($newConversationHolder);
6863
if ($form.closest('tr').data('line-type') === 'same') {
6964
$(`[data-path="${path}"] a.add-code-comment[data-idx="${idx}"]`).addClass('invisible');

web_src/js/features/repo-issue.js

+1-4
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {attachTribute} from './tribute.js';
44
import {createCommentEasyMDE, getAttachedEasyMDE} from './comp/EasyMDE.js';
55
import {initEasyMDEImagePaste} from './comp/ImagePaste.js';
66
import {initCompMarkupContentPreviewTab} from './comp/MarkupContentPreview.js';
7-
import {initTooltip, showTemporaryTooltip, createTippy} from '../modules/tippy.js';
7+
import {showTemporaryTooltip, createTippy} from '../modules/tippy.js';
88
import {hideElem, showElem, toggleElem} from '../utils/dom.js';
99
import {setFileFolding} from './file-fold.js';
1010

@@ -280,10 +280,7 @@ export function initRepoPullRequestAllowMaintainerEdit() {
280280
const $checkbox = $('#allow-edits-from-maintainers');
281281
if (!$checkbox.length) return;
282282

283-
const promptTip = $checkbox.attr('data-prompt-tip');
284283
const promptError = $checkbox.attr('data-prompt-error');
285-
286-
initTooltip($checkbox[0], {content: promptTip});
287284
$checkbox.checkbox({
288285
'onChange': () => {
289286
const checked = $checkbox.checkbox('is checked');

web_src/js/index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ import {
5656
initGlobalFormDirtyLeaveConfirm,
5757
initGlobalLinkActions,
5858
initHeadNavbarContentToggle,
59-
initGlobalTooltips,
6059
} from './features/common-global.js';
6160
import {initRepoTopicBar} from './features/repo-home.js';
6261
import {initAdminEmails} from './features/admin/emails.js';
@@ -91,6 +90,7 @@ import {initCaptcha} from './features/captcha.js';
9190
import {initRepositoryActionView} from './components/RepoActionView.vue';
9291
import {initAriaCheckboxPatch} from './modules/aria/checkbox.js';
9392
import {initAriaDropdownPatch} from './modules/aria/dropdown.js';
93+
import {initGlobalTooltips} from './modules/tippy.js';
9494

9595
// Run time-critical code as soon as possible. This is safe to do because this
9696
// script appears at the end of <body> and rendered HTML is accessible at that point.

web_src/js/modules/tippy.js

+97-20
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import tippy from 'tippy.js';
33
export function createTippy(target, opts = {}) {
44
const instance = tippy(target, {
55
appendTo: document.body,
6-
placement: target.getAttribute('data-placement') || 'top-start',
76
animation: false,
87
allowHTML: false,
98
hideOnClick: false,
@@ -25,38 +24,116 @@ export function createTippy(target, opts = {}) {
2524
return instance;
2625
}
2726

28-
export function initTooltip(el, props = {}) {
29-
const content = el.getAttribute('data-content') || props.content;
27+
/**
28+
* Attach a tooltip tippy to the given target element.
29+
* If the target element already has a tooltip tippy attached, the tooltip will be updated with the new content.
30+
* If the target element has no content, then no tooltip will be attached, and it returns null.
31+
*
32+
* Note: "tooltip" doesn't equal to "tippy". "tooltip" means a auto-popup content, it just uses tippy as the implementation.
33+
*
34+
* @param target {HTMLElement}
35+
* @param content {null|string}
36+
* @returns {null|tippy}
37+
*/
38+
function attachTooltip(target, content = null) {
39+
content = content ?? getTooltipContent(target);
3040
if (!content) return null;
31-
if (!el.hasAttribute('aria-label')) el.setAttribute('aria-label', content);
32-
return createTippy(el, {
41+
42+
const props = {
3343
content,
3444
delay: 100,
3545
role: 'tooltip',
36-
...(el.getAttribute('data-tooltip-interactive') === 'true' ? {interactive: true} : {}),
37-
...props,
38-
});
39-
}
46+
placement: target.getAttribute('data-tooltip-placement') || 'top-start',
47+
...(target.getAttribute('data-tooltip-interactive') === 'true' ? {interactive: true} : {}),
48+
};
4049

41-
export function showTemporaryTooltip(target, content) {
42-
let tippy, oldContent;
43-
if (target._tippy) {
44-
tippy = target._tippy;
45-
oldContent = tippy.props.content;
50+
if (!target._tippy) {
51+
createTippy(target, props);
4652
} else {
47-
tippy = initTooltip(target, {content});
53+
target._tippy.setProps(props);
54+
}
55+
return target._tippy;
56+
}
57+
58+
/**
59+
* Creating tooltip tippy instance is expensive, so we only create it when the user hovers over the element
60+
* According to https://www.w3.org/TR/DOM-Level-3-Events/#events-mouseevent-event-order , mouseover event is fired before mouseenter event
61+
* Some old browsers like Pale Moon doesn't support "mouseenter(capture)"
62+
* The tippy by default uses "mouseenter" event to show, so we use "mouseover" event to switch to tippy
63+
* @param e {Event}
64+
*/
65+
function lazyTooltipOnMouseHover(e) {
66+
e.target.removeEventListener('mouseover', lazyTooltipOnMouseHover, true);
67+
attachTooltip(this);
68+
}
69+
70+
function getTooltipContent(target) {
71+
// prefer to always use the "[data-tooltip-content]" attribute
72+
// for backward compatibility, we also support the ".tooltip[data-content]" attribute
73+
// in next PR, refactor all the ".tooltip[data-content]" to "[data-tooltip-content]"
74+
let content = target.getAttribute('data-tooltip-content');
75+
if (!content && target.classList.contains('tooltip')) {
76+
content = target.getAttribute('data-content');
77+
}
78+
return content;
79+
}
80+
81+
/**
82+
* Activate the tooltip for all children elements
83+
* And if the element has no aria-label, use the tooltip content as aria-label
84+
* @param target {HTMLElement}
85+
*/
86+
function attachChildrenLazyTooltip(target) {
87+
// the selector must match the logic in getTippyTooltipContent
88+
for (const el of target.querySelectorAll('[data-tooltip-content], .tooltip[data-content]')) {
89+
el.addEventListener('mouseover', lazyTooltipOnMouseHover, true);
90+
91+
// meanwhile, if the element has no aria-label, use the tooltip content as aria-label
92+
if (!el.hasAttribute('aria-label')) {
93+
const content = getTooltipContent(el);
94+
if (content) {
95+
el.setAttribute('aria-label', content);
96+
}
97+
}
4898
}
99+
}
49100

101+
export function initGlobalTooltips() {
102+
// use MutationObserver to detect new elements added to the DOM, or attributes changed
103+
const observer = new MutationObserver((mutationList) => {
104+
for (const mutation of mutationList) {
105+
if (mutation.type === 'childList') {
106+
// mainly for Vue components and AJAX rendered elements
107+
for (const el of mutation.addedNodes) {
108+
// handle all "tooltip" elements in added nodes which have 'querySelectorAll' method, skip non-related nodes (eg: "#text")
109+
if ('querySelectorAll' in el) {
110+
attachChildrenLazyTooltip(el);
111+
}
112+
}
113+
} else if (mutation.type === 'attributes') {
114+
// sync the tooltip content if the attributes change
115+
attachTooltip(mutation.target);
116+
}
117+
}
118+
});
119+
observer.observe(document, {
120+
subtree: true,
121+
childList: true,
122+
attributeFilter: ['data-tooltip-content', 'data-content'],
123+
});
124+
125+
attachChildrenLazyTooltip(document.documentElement);
126+
}
127+
128+
export function showTemporaryTooltip(target, content) {
129+
const tippy = target._tippy ?? attachTooltip(target, content);
50130
tippy.setContent(content);
51131
if (!tippy.state.isShown) tippy.show();
52132
tippy.setProps({
53133
onHidden: (tippy) => {
54-
if (oldContent) {
55-
tippy.setContent(oldContent);
56-
tippy.setProps({onHidden: undefined});
57-
} else {
134+
// reset the default tooltip content, if no default, then this temporary tooltip could be destroyed
135+
if (!attachTooltip(target)) {
58136
tippy.destroy();
59-
// after destroy, the `_tippy` is detached, it can't do "setProps (etc...)" anymore
60137
}
61138
},
62139
});

0 commit comments

Comments
 (0)