From faeb616d80dd24752cd0b74b5a5aae902ffd8438 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 30 May 2024 16:28:35 +0200 Subject: [PATCH 01/14] Rewrite context popup to stateless --- routers/web/repo/issue.go | 4 +- templates/base/head_script.tmpl | 1 - web_src/js/components/ContextPopup.vue | 78 +++++++------------------ web_src/js/features/contextpopup.js | 79 ++++++++++++++------------ web_src/js/utils/vue.js | 14 +++++ 5 files changed, 79 insertions(+), 97 deletions(-) create mode 100644 web_src/js/utils/vue.js diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 18f975b4a6bd8..08ecccfaab38f 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -2181,8 +2181,8 @@ func GetIssueInfo(ctx *context.Context) { } ctx.JSON(http.StatusOK, map[string]any{ - "convertedIssue": convert.ToIssue(ctx, ctx.Doer, issue), - "renderedLabels": templates.RenderLabels(ctx, ctx.Locale, issue.Labels, ctx.Repo.RepoLink, issue), + "issue": convert.ToIssue(ctx, ctx.Doer, issue), + "labelsHtml": templates.RenderLabels(ctx, ctx.Locale, issue.Labels, ctx.Repo.RepoLink, issue), }) } diff --git a/templates/base/head_script.tmpl b/templates/base/head_script.tmpl index 22e08e9c8f69c..fcd5567742f6c 100644 --- a/templates/base/head_script.tmpl +++ b/templates/base/head_script.tmpl @@ -36,7 +36,6 @@ If you introduce mistakes in it, Gitea JavaScript code wouldn't run correctly. i18n: { copy_success: {{ctx.Locale.Tr "copy_success"}}, copy_error: {{ctx.Locale.Tr "copy_error"}}, - error_occurred: {{ctx.Locale.Tr "error.occurred"}}, network_error: {{ctx.Locale.Tr "error.network_error"}}, remove_label_str: {{ctx.Locale.Tr "remove_label_str"}}, modal_confirm: {{ctx.Locale.Tr "modal.confirm"}}, diff --git a/web_src/js/components/ContextPopup.vue b/web_src/js/components/ContextPopup.vue index 8f389ea003e43..7a01d35480c00 100644 --- a/web_src/js/components/ContextPopup.vue +++ b/web_src/js/components/ContextPopup.vue @@ -1,18 +1,18 @@ diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index 6a9325ed1cd89..17b0842b6967e 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -1,45 +1,50 @@ -import {createApp} from 'vue'; import ContextPopup from '../components/ContextPopup.vue'; +import {createVueRoot} from '../utils/vue.js'; import {parseIssueHref} from '../utils.js'; import {createTippy} from '../modules/tippy.js'; +import {GET} from '../modules/fetch.js'; -export function initContextPopups() { - const refIssues = document.querySelectorAll('.ref-issue'); - attachRefIssueContextPopup(refIssues); +const {appSubUrl} = window.config; + +async function show(e) { + const link = e.currentTarget; + const {owner, repo, index} = parseIssueHref(link.getAttribute('href')); + if (!owner) return; + + const res = await GET(`${appSubUrl}/${owner}/${repo}/issues/${index}/info`); // backend: GetIssueInfo + if (!res.ok) return; + + let issue, labelsHtml; + try { + ({issue, labelsHtml} = await res.json()); + } catch {} + if (!issue) return; + + const content = createVueRoot(ContextPopup, {issue, labelsHtml}); + if (!content) return; + + const tippy = createTippy(link, { + theme: 'default', + trigger: 'mouseenter focus', + content, + placement: 'top-start', + interactive: true, + role: 'dialog', + interactiveBorder: 15, + }); + + // show immediately because this runs during mouseenter and focus + tippy.show(); } -export function attachRefIssueContextPopup(refIssues) { - for (const refIssue of refIssues) { - if (refIssue.classList.contains('ref-external-issue')) { - return; - } - - const {owner, repo, index} = parseIssueHref(refIssue.getAttribute('href')); - if (!owner) return; - - const el = document.createElement('div'); - el.classList.add('tw-p-3'); - refIssue.parentNode.insertBefore(el, refIssue.nextSibling); - - const view = createApp(ContextPopup); - - try { - view.mount(el); - } catch (err) { - console.error(err); - el.textContent = 'ContextPopup failed to load'; - } - - createTippy(refIssue, { - theme: 'default', - content: el, - placement: 'top-start', - interactive: true, - role: 'dialog', - interactiveBorder: 5, - onShow: () => { - el.firstChild.dispatchEvent(new CustomEvent('ce-load-context-popup', {detail: {owner, repo, index}})); - }, - }); +export function attachRefIssueContextPopup(els) { + for (const link of els) { + link.addEventListener('mouseenter', show); + link.addEventListener('focus', show); } } + +export function initContextPopups() { + // TODO: Use MutationObserver to detect newly inserted .ref-issue + attachRefIssueContextPopup(document.querySelectorAll('.ref-issue:not(.ref-external-issue)')); +} diff --git a/web_src/js/utils/vue.js b/web_src/js/utils/vue.js new file mode 100644 index 0000000000000..1558cfa1a0b20 --- /dev/null +++ b/web_src/js/utils/vue.js @@ -0,0 +1,14 @@ +import {createApp} from 'vue'; + +// create a new vue root and container and mount a component into it +export function createVueRoot(component, props) { + const container = document.createElement('div'); + const view = createApp(component, props); + try { + view.mount(container); + return container; + } catch (err) { + console.error(err); + return null; + } +} From 758918ed36b4fec800ba1d132dc8cbaa6aa9c1c0 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 30 May 2024 17:09:30 +0200 Subject: [PATCH 02/14] init only once, use tooltip role to enable singleton --- web_src/js/features/contextpopup.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index 17b0842b6967e..e425b0d635597 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -5,9 +5,13 @@ import {createTippy} from '../modules/tippy.js'; import {GET} from '../modules/fetch.js'; const {appSubUrl} = window.config; +const initAttr = 'data-contextpopup-init-done'; -async function show(e) { +async function init(e) { const link = e.currentTarget; + if (link.hasAttribute(initAttr)) return; + link.setAttribute(initAttr, 'true'); + const {owner, repo, index} = parseIssueHref(link.getAttribute('href')); if (!owner) return; @@ -29,7 +33,7 @@ async function show(e) { content, placement: 'top-start', interactive: true, - role: 'dialog', + role: 'tooltip', interactiveBorder: 15, }); @@ -38,9 +42,9 @@ async function show(e) { } export function attachRefIssueContextPopup(els) { - for (const link of els) { - link.addEventListener('mouseenter', show); - link.addEventListener('focus', show); + for (const el of els) { + el.addEventListener('mouseenter', init, {once: true}); + el.addEventListener('focus', init, {once: true}); } } From eaea1e76521db01d048c3c434b577636c9527d09 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 30 May 2024 17:13:33 +0200 Subject: [PATCH 03/14] fmt --- routers/web/repo/issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 08ecccfaab38f..b9605880b8df1 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -2181,7 +2181,7 @@ func GetIssueInfo(ctx *context.Context) { } ctx.JSON(http.StatusOK, map[string]any{ - "issue": convert.ToIssue(ctx, ctx.Doer, issue), + "issue": convert.ToIssue(ctx, ctx.Doer, issue), "labelsHtml": templates.RenderLabels(ctx, ctx.Locale, issue.Labels, ctx.Repo.RepoLink, issue), }) } From 3ec74c939531df38274c913f6f23ded018901305 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 30 May 2024 17:45:39 +0200 Subject: [PATCH 04/14] try fix test --- tests/integration/issue_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index 308b82d4b950b..130bd24250047 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -641,13 +641,13 @@ func TestGetIssueInfo(t *testing.T) { req := NewRequest(t, "GET", urlStr) resp := session.MakeRequest(t, req, http.StatusOK) var respStruct struct { - ConvertedIssue api.Issue - RenderedLabels template.HTML + Issue api.Issue `json:"issue"` + LabelsHTML template.HTML `json:"labelsHtml"` } DecodeJSON(t, resp, &respStruct) - assert.EqualValues(t, issue.ID, respStruct.ConvertedIssue.ID) - assert.Contains(t, string(respStruct.RenderedLabels), `"labels-list"`) + assert.EqualValues(t, issue.ID, respStruct.Issue.ID) + assert.Contains(t, string(respStruct.LabelsHTML), `"labels-list"`) } func TestUpdateIssueDeadline(t *testing.T) { From bc8dcbca3dd92a419dc94cf37f5389d5dcd7f5a2 Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 31 May 2024 13:49:10 +0200 Subject: [PATCH 05/14] just check for _tippy --- web_src/js/features/contextpopup.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index e425b0d635597..a070224bc639a 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -5,12 +5,10 @@ import {createTippy} from '../modules/tippy.js'; import {GET} from '../modules/fetch.js'; const {appSubUrl} = window.config; -const initAttr = 'data-contextpopup-init-done'; async function init(e) { const link = e.currentTarget; - if (link.hasAttribute(initAttr)) return; - link.setAttribute(initAttr, 'true'); + if (link._tippy) return; // link already has a tooltip const {owner, repo, index} = parseIssueHref(link.getAttribute('href')); if (!owner) return; @@ -43,8 +41,8 @@ async function init(e) { export function attachRefIssueContextPopup(els) { for (const el of els) { - el.addEventListener('mouseenter', init, {once: true}); - el.addEventListener('focus', init, {once: true}); + el.addEventListener('mouseenter', init); + el.addEventListener('focus', init); } } From 0ec0e43fc87f6c99445be80ce8f747f7fc8028e6 Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 31 May 2024 13:54:29 +0200 Subject: [PATCH 06/14] support the case of changing link url --- web_src/js/features/contextpopup.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index a070224bc639a..b8637e369c175 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -5,15 +5,18 @@ import {createTippy} from '../modules/tippy.js'; import {GET} from '../modules/fetch.js'; const {appSubUrl} = window.config; +const urlAttribute = 'data-issue-ref-url'; async function init(e) { const link = e.currentTarget; - if (link._tippy) return; // link already has a tooltip const {owner, repo, index} = parseIssueHref(link.getAttribute('href')); if (!owner) return; - const res = await GET(`${appSubUrl}/${owner}/${repo}/issues/${index}/info`); // backend: GetIssueInfo + const url = `${appSubUrl}/${owner}/${repo}/issues/${index}/info`; // backend: GetIssueInfo + if (link.getAttribute(urlAttribute) === url) return; // link already has a tooltip with this url + + const res = await GET(url); if (!res.ok) return; let issue, labelsHtml; @@ -35,6 +38,8 @@ async function init(e) { interactiveBorder: 15, }); + link.setAttribute(urlAttribute, url); + // show immediately because this runs during mouseenter and focus tippy.show(); } From d553d4cba1b712c155d94fc6934ab08f32f54222 Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 31 May 2024 13:57:00 +0200 Subject: [PATCH 07/14] add comment --- web_src/js/features/contextpopup.js | 1 + 1 file changed, 1 insertion(+) diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index b8637e369c175..3bb827b7f6df4 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -38,6 +38,7 @@ async function init(e) { interactiveBorder: 15, }); + // set attribute on the link that indicates which url the tooltip currently renders link.setAttribute(urlAttribute, url); // show immediately because this runs during mouseenter and focus From c5cde74f175e462ebc22a2472b9aee10edf9cb3b Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 31 May 2024 15:15:18 +0200 Subject: [PATCH 08/14] also try-catch the GET --- web_src/js/features/contextpopup.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index 3bb827b7f6df4..47405f7849817 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -16,7 +16,10 @@ async function init(e) { const url = `${appSubUrl}/${owner}/${repo}/issues/${index}/info`; // backend: GetIssueInfo if (link.getAttribute(urlAttribute) === url) return; // link already has a tooltip with this url - const res = await GET(url); + let res; + try { + res = await GET(url); + } catch {} if (!res.ok) return; let issue, labelsHtml; From e350fdc0975f5c38322607e97b3a3a2b374e86b9 Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 31 May 2024 15:17:31 +0200 Subject: [PATCH 09/14] move check for ref-external-issue into init --- web_src/js/features/contextpopup.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index 47405f7849817..d002109ed4af0 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -9,6 +9,7 @@ const urlAttribute = 'data-issue-ref-url'; async function init(e) { const link = e.currentTarget; + if (link.classList.contains('ref-external-issue')) return; const {owner, repo, index} = parseIssueHref(link.getAttribute('href')); if (!owner) return; @@ -57,5 +58,5 @@ export function attachRefIssueContextPopup(els) { export function initContextPopups() { // TODO: Use MutationObserver to detect newly inserted .ref-issue - attachRefIssueContextPopup(document.querySelectorAll('.ref-issue:not(.ref-external-issue)')); + attachRefIssueContextPopup(document.querySelectorAll('.ref-issue')); } From 5323ead361cdb7d968de7de56a0867cbdfc845b7 Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 31 May 2024 15:18:07 +0200 Subject: [PATCH 10/14] rename init to attach --- web_src/js/features/contextpopup.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index d002109ed4af0..ed5c40eec66a1 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -7,7 +7,7 @@ import {GET} from '../modules/fetch.js'; const {appSubUrl} = window.config; const urlAttribute = 'data-issue-ref-url'; -async function init(e) { +async function attach(e) { const link = e.currentTarget; if (link.classList.contains('ref-external-issue')) return; @@ -51,8 +51,8 @@ async function init(e) { export function attachRefIssueContextPopup(els) { for (const el of els) { - el.addEventListener('mouseenter', init); - el.addEventListener('focus', init); + el.addEventListener('mouseenter', attach); + el.addEventListener('focus', attach); } } From e05b59b696a690500862b5b72d82c5e4ad329a5d Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 31 May 2024 15:30:27 +0200 Subject: [PATCH 11/14] comment --- web_src/js/features/contextpopup.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index ed5c40eec66a1..b2e98e3c1c3f5 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -9,6 +9,8 @@ const urlAttribute = 'data-issue-ref-url'; async function attach(e) { const link = e.currentTarget; + + // ignore external issues if (link.classList.contains('ref-external-issue')) return; const {owner, repo, index} = parseIssueHref(link.getAttribute('href')); From 434287a6ae46e910c8e0d3654480d8492537471e Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 31 May 2024 17:23:22 +0200 Subject: [PATCH 12/14] add tracking for loading state --- web_src/js/features/contextpopup.js | 58 ++++++++++++++++------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index b2e98e3c1c3f5..aa411e0f5edb3 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -5,50 +5,56 @@ import {createTippy} from '../modules/tippy.js'; import {GET} from '../modules/fetch.js'; const {appSubUrl} = window.config; -const urlAttribute = 'data-issue-ref-url'; async function attach(e) { const link = e.currentTarget; // ignore external issues if (link.classList.contains('ref-external-issue')) return; + // ignore links that are already loading + if (link.hasAttribute('data-issue-ref-loading')) return; const {owner, repo, index} = parseIssueHref(link.getAttribute('href')); if (!owner) return; const url = `${appSubUrl}/${owner}/${repo}/issues/${index}/info`; // backend: GetIssueInfo - if (link.getAttribute(urlAttribute) === url) return; // link already has a tooltip with this url + if (link.getAttribute('data-issue-ref-url') === url) return; // link already has a tooltip with this url - let res; try { - res = await GET(url); - } catch {} - if (!res.ok) return; + link.setAttribute('data-issue-ref-loading', 'true'); + let res; + try { + res = await GET(url); + } catch {} + if (!res.ok) return; - let issue, labelsHtml; - try { - ({issue, labelsHtml} = await res.json()); - } catch {} - if (!issue) return; + let issue, labelsHtml; + try { + ({issue, labelsHtml} = await res.json()); + } catch {} + if (!issue) return; - const content = createVueRoot(ContextPopup, {issue, labelsHtml}); - if (!content) return; + const content = createVueRoot(ContextPopup, {issue, labelsHtml}); + if (!content) return; - const tippy = createTippy(link, { - theme: 'default', - trigger: 'mouseenter focus', - content, - placement: 'top-start', - interactive: true, - role: 'tooltip', - interactiveBorder: 15, - }); + const tippy = createTippy(link, { + theme: 'default', + trigger: 'mouseenter focus', + content, + placement: 'top-start', + interactive: true, + role: 'tooltip', + interactiveBorder: 15, + }); - // set attribute on the link that indicates which url the tooltip currently renders - link.setAttribute(urlAttribute, url); + // set attribute on the link that indicates which url the tooltip currently renders + link.setAttribute('data-issue-ref-url', url); - // show immediately because this runs during mouseenter and focus - tippy.show(); + // show immediately because this runs during mouseenter and focus + tippy.show(); + } finally { + link.removeAttribute('data-issue-ref-loading'); + } } export function attachRefIssueContextPopup(els) { From abf514f1064edb99a4273c91038b71bfdad8fdc8 Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 31 May 2024 17:25:08 +0200 Subject: [PATCH 13/14] rename attr --- web_src/js/features/contextpopup.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index aa411e0f5edb3..769389d76a287 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -18,7 +18,7 @@ async function attach(e) { if (!owner) return; const url = `${appSubUrl}/${owner}/${repo}/issues/${index}/info`; // backend: GetIssueInfo - if (link.getAttribute('data-issue-ref-url') === url) return; // link already has a tooltip with this url + if (link.getAttribute('data-issue-ref-info-url') === url) return; // link already has a tooltip with this url try { link.setAttribute('data-issue-ref-loading', 'true'); @@ -48,7 +48,7 @@ async function attach(e) { }); // set attribute on the link that indicates which url the tooltip currently renders - link.setAttribute('data-issue-ref-url', url); + link.setAttribute('data-issue-ref-info-url', url); // show immediately because this runs during mouseenter and focus tippy.show(); From 9b3a92218674f535aaf008a11cdb083a152faf68 Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 31 May 2024 18:19:05 +0200 Subject: [PATCH 14/14] turn repo name into a link --- web_src/js/components/ContextPopup.vue | 8 +++++++- web_src/js/features/contextpopup.js | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/web_src/js/components/ContextPopup.vue b/web_src/js/components/ContextPopup.vue index 7a01d35480c00..040452118bee9 100644 --- a/web_src/js/components/ContextPopup.vue +++ b/web_src/js/components/ContextPopup.vue @@ -12,6 +12,10 @@ export default { type: String, default: '', }, + repoUrl: { + type: String, + default: '', + }, }, computed: { createdAt() { @@ -61,7 +65,9 @@ export default {