From a43ea22479334ef3ac05fb8784223d45846b71a3 Mon Sep 17 00:00:00 2001 From: HesterG Date: Wed, 14 Jun 2023 16:01:37 +0800 Subject: [PATCH] Change form actions to fetch for submit review box (#25219) Co-author: @wxiaoguang Close #25096 The way to fix it in this PR is to change form submit to fetch using formData, and add flags to avoid post repeatedly. Should be able to apply to more forms that have the same issue after this PR. In the demo below, 'approve' is clicked several times, and then 'comment' is clicked several time after 'request changes' clicked. After: https://github.com/go-gitea/gitea/assets/17645053/beabeb1d-fe66-4b76-b048-4f022b4e83a0 Update: screenshots from /devtest > ![image](https://user-images.githubusercontent.com/2114189/245680011-ee4231e0-a53d-4c2a-a9c2-71ccd98005cc.png) > > ![image](https://user-images.githubusercontent.com/2114189/245680057-9215d348-63d8-406d-8828-17e171163aaa.png) > > ![image](https://user-images.githubusercontent.com/2114189/245680148-89d7b3d1-d7b6-442f-b69e-eadaee112482.png) --------- Co-authored-by: wxiaoguang --- modules/context/base.go | 4 ++ routers/web/devtest/devtest.go | 10 +++ routers/web/repo/pull_review.go | 9 ++- routers/web/web.go | 1 + templates/devtest/fetch-action.tmpl | 42 +++++++++++++ templates/devtest/gitea-ui.tmpl | 11 ++++ templates/devtest/list.tmpl | 17 ++--- templates/repo/diff/new_review.tmpl | 2 +- web_src/css/modules/animations.css | 21 ++++--- web_src/css/modules/tippy.css | 6 ++ web_src/js/features/common-global.js | 84 ++++++++++++++++++++++++- web_src/js/features/comp/QuickSubmit.js | 21 ++++--- web_src/js/features/repo-code.js | 2 +- web_src/js/modules/tippy.js | 14 ++++- 14 files changed, 208 insertions(+), 36 deletions(-) create mode 100644 templates/devtest/fetch-action.tmpl diff --git a/modules/context/base.go b/modules/context/base.go index ac9b52d51cf..5ae5e65d3ed 100644 --- a/modules/context/base.go +++ b/modules/context/base.go @@ -132,6 +132,10 @@ func (b *Base) JSON(status int, content interface{}) { } } +func (b *Base) JSONRedirect(redirect string) { + b.JSON(http.StatusOK, map[string]any{"redirect": redirect}) +} + // RemoteAddr returns the client machine ip address func (b *Base) RemoteAddr() string { return b.Req.RemoteAddr diff --git a/routers/web/devtest/devtest.go b/routers/web/devtest/devtest.go index 48875e306d2..64b732c0356 100644 --- a/routers/web/devtest/devtest.go +++ b/routers/web/devtest/devtest.go @@ -32,6 +32,16 @@ func List(ctx *context.Context) { ctx.HTML(http.StatusOK, "devtest/list") } +func FetchActionTest(ctx *context.Context) { + _ = ctx.Req.ParseForm() + ctx.Flash.Info(ctx.Req.Method + " " + ctx.Req.RequestURI + "
" + + "Form: " + ctx.Req.Form.Encode() + "
" + + "PostForm: " + ctx.Req.PostForm.Encode(), + ) + time.Sleep(2 * time.Second) + ctx.JSONRedirect("") +} + func Tmpl(ctx *context.Context) { now := time.Now() ctx.Data["TimeNow"] = now diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 90cfd5bfcdb..69d36ff4a47 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -193,7 +193,7 @@ func SubmitReview(ctx *context.Context) { } if ctx.HasError() { ctx.Flash.Error(ctx.Data["ErrorMsg"].(string)) - ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) + ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) return } @@ -214,7 +214,7 @@ func SubmitReview(ctx *context.Context) { } ctx.Flash.Error(translated) - ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) + ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) return } } @@ -228,14 +228,13 @@ func SubmitReview(ctx *context.Context) { if err != nil { if issues_model.IsContentEmptyErr(err) { ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty")) - ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) + ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) } else { ctx.ServerError("SubmitReview", err) } return } - - ctx.Redirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag())) + ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag())) } // DismissReview dismissing stale review by repo admin diff --git a/routers/web/web.go b/routers/web/web.go index 1e235a3c3ce..8683ef221dd 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1411,6 +1411,7 @@ func registerRoutes(m *web.Route) { if !setting.IsProd { m.Any("/devtest", devtest.List) + m.Any("/devtest/fetch-action-test", devtest.FetchActionTest) m.Any("/devtest/{sub}", devtest.Tmpl) } diff --git a/templates/devtest/fetch-action.tmpl b/templates/devtest/fetch-action.tmpl new file mode 100644 index 00000000000..2fb7289ebe9 --- /dev/null +++ b/templates/devtest/fetch-action.tmpl @@ -0,0 +1,42 @@ +{{template "base/head" .}} +
+ {{template "base/alert" .}} +
+

link-action

+
+ Use "window.fetch" to send a request to backend, the request is defined in an "A" or "BUTTON" element. + It might be renamed to "link-fetch-action" to match the "form-fetch-action". +
+
+ +
+
+
+

form-fetch-action

+
Use "window.fetch" to send a form request to backend
+
+
+ +
+
+
+
+
+
+
+
bad action url
+
+
+
+
+
+ +{{template "base/footer" .}} diff --git a/templates/devtest/gitea-ui.tmpl b/templates/devtest/gitea-ui.tmpl index 824b7d0db69..516b73cf095 100644 --- a/templates/devtest/gitea-ui.tmpl +++ b/templates/devtest/gitea-ui.tmpl @@ -89,6 +89,17 @@
text with interactive tooltip
+
+

Loading

+
loading ...
+
+

loading ...

+

loading ...

+

loading ...

+

loading ...

+
+
+

GiteaOriginUrl

diff --git a/templates/devtest/list.tmpl b/templates/devtest/list.tmpl index 5044f2a5019..90b1fcc9d04 100644 --- a/templates/devtest/list.tmpl +++ b/templates/devtest/list.tmpl @@ -1,12 +1,15 @@ - +{{template "base/head" .}} +
    {{range .SubNames}}
  • {{.}}
    • {{end}}
+ + + +{{template "base/footer" .}} diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index afb82a8d3d7..c407064176d 100644 --- a/templates/repo/diff/new_review.tmpl +++ b/templates/repo/diff/new_review.tmpl @@ -6,7 +6,7 @@
-
+ {{.CsrfTokenHtml}}
diff --git a/web_src/css/modules/animations.css b/web_src/css/modules/animations.css index 3f5e8bd2676..dcd0e075376 100644 --- a/web_src/css/modules/animations.css +++ b/web_src/css/modules/animations.css @@ -4,20 +4,22 @@ } .is-loading { - background: transparent !important; - color: transparent !important; - border: transparent !important; pointer-events: none !important; position: relative !important; overflow: hidden !important; } +.is-loading > * { + opacity: 0.3; +} + .is-loading::after { content: ""; position: absolute; display: block; - width: 4rem; height: 4rem; + max-height: 50%; + aspect-ratio: 1 / 1; left: 50%; top: 50%; transform: translate(-50%, -50%); @@ -28,18 +30,24 @@ border-radius: 100%; } +.is-loading.small-loading-icon::after { + border-width: 2px; +} + .markup pre.is-loading, .editor-loading.is-loading, .pdf-content.is-loading { height: var(--height-loading); } +/* TODO: not needed, use "is-loading small-loading-icon" instead */ .btn-octicon.is-loading::after { border-width: 2px; height: 1.25rem; width: 1.25rem; } +/* TODO: not needed, use "is-loading small-loading-icon" instead */ code.language-math.is-loading::after { padding: 0; border-width: 2px; @@ -47,11 +55,6 @@ code.language-math.is-loading::after { height: 1.25rem; } -#oauth2-login-navigator.is-loading::after { - width: 40px; - height: 40px; -} - @keyframes fadein { 0% { opacity: 0; diff --git a/web_src/css/modules/tippy.css b/web_src/css/modules/tippy.css index bd55b9d6b93..fe325972803 100644 --- a/web_src/css/modules/tippy.css +++ b/web_src/css/modules/tippy.css @@ -29,6 +29,12 @@ color: var(--color-text); } +.tippy-box[data-theme="form-fetch-error"] { + border-color: var(--color-error-border); + background-color: var(--color-error-bg); + color: var(--color-error-text); +} + .tippy-content { position: relative; padding: 1rem; diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index b1d3fa22d8c..c0e66be51c9 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -7,6 +7,7 @@ import {handleGlobalEnterQuickSubmit} from './comp/QuickSubmit.js'; import {svg} from '../svg.js'; import {hideElem, showElem, toggleElem} from '../utils/dom.js'; import {htmlEscape} from 'escape-goat'; +import {createTippy} from '../modules/tippy.js'; const {appUrl, csrfToken, i18n} = window.config; @@ -60,6 +61,81 @@ export function initGlobalButtonClickOnEnter() { }); } +async function formFetchAction(e) { + if (!e.target.classList.contains('form-fetch-action')) return; + + e.preventDefault(); + const formEl = e.target; + if (formEl.classList.contains('is-loading')) return; + + formEl.classList.add('is-loading'); + if (formEl.clientHeight < 50) { + formEl.classList.add('small-loading-icon'); + } + + const formMethod = formEl.getAttribute('method') || 'get'; + const formActionUrl = formEl.getAttribute('action'); + const formData = new FormData(formEl); + const [submitterName, submitterValue] = [e.submitter?.getAttribute('name'), e.submitter?.getAttribute('value')]; + if (submitterName) { + formData.append(submitterName, submitterValue || ''); + } + + let reqUrl = formActionUrl; + const reqOpt = {method: formMethod.toUpperCase(), headers: {'X-Csrf-Token': csrfToken}}; + if (formMethod.toLowerCase() === 'get') { + const params = new URLSearchParams(); + for (const [key, value] of formData) { + params.append(key, value.toString()); + } + const pos = reqUrl.indexOf('?'); + if (pos !== -1) { + reqUrl = reqUrl.slice(0, pos); + } + reqUrl += `?${params.toString()}`; + } else { + reqOpt.body = formData; + } + + let errorTippy; + const onError = (msg) => { + formEl.classList.remove('is-loading', 'small-loading-icon'); + if (errorTippy) errorTippy.destroy(); + errorTippy = createTippy(formEl, { + content: msg, + interactive: true, + showOnCreate: true, + hideOnClick: true, + role: 'alert', + theme: 'form-fetch-error', + trigger: 'manual', + arrow: false, + }); + }; + + const doRequest = async () => { + try { + const resp = await fetch(reqUrl, reqOpt); + if (resp.status === 200) { + const {redirect} = await resp.json(); + formEl.classList.remove('dirty'); // remove the areYouSure check before reloading + if (redirect) { + window.location.href = redirect; + } else { + window.location.reload(); + } + } else { + onError(`server error: ${resp.status}`); + } + } catch (e) { + onError(e.error); + } + }; + + // TODO: add "confirm" support like "link-action" in the future + await doRequest(); +} + export function initGlobalCommon() { // Semantic UI modules. const $uiDropdowns = $('.ui.dropdown'); @@ -114,6 +190,8 @@ export function initGlobalCommon() { if (btn.classList.contains('loading')) return e.preventDefault(); btn.classList.add('loading'); }); + + document.addEventListener('submit', formFetchAction); } export function initGlobalDropzone() { @@ -182,7 +260,7 @@ function linkAction(e) { const $this = $(e.target); const redirect = $this.attr('data-redirect'); - const request = () => { + const doRequest = () => { $this.prop('disabled', true); $.post($this.attr('data-url'), { _csrf: csrfToken @@ -201,7 +279,7 @@ function linkAction(e) { const modalConfirmHtml = htmlEscape($this.attr('data-modal-confirm') || ''); if (!modalConfirmHtml) { - request(); + doRequest(); return; } @@ -220,7 +298,7 @@ function linkAction(e) { $modal.appendTo(document.body); $modal.modal({ onApprove() { - request(); + doRequest(); }, onHidden() { $modal.remove(); diff --git a/web_src/js/features/comp/QuickSubmit.js b/web_src/js/features/comp/QuickSubmit.js index d598a596553..2587375a717 100644 --- a/web_src/js/features/comp/QuickSubmit.js +++ b/web_src/js/features/comp/QuickSubmit.js @@ -1,17 +1,24 @@ import $ from 'jquery'; export function handleGlobalEnterQuickSubmit(target) { - const $target = $(target); - const $form = $(target).closest('form'); - if ($form.length) { + const form = target.closest('form'); + if (form) { + if (!form.checkValidity()) { + form.reportValidity(); + return; + } + + if (form.classList.contains('form-fetch-action')) { + form.dispatchEvent(new SubmitEvent('submit', {bubbles: true, cancelable: true})); + return; + } + // here use the event to trigger the submit event (instead of calling `submit()` method directly) // otherwise the `areYouSure` handler won't be executed, then there will be an annoying "confirm to leave" dialog - if ($form[0].checkValidity()) { - $form.trigger('submit'); - } + $(form).trigger('submit'); } else { // if no form, then the editor is for an AJAX request, dispatch an event to the target, let the target's event handler to do the AJAX request. // the 'ce-' prefix means this is a CustomEvent - $target.trigger('ce-quick-submit'); + target.dispatchEvent(new CustomEvent('ce-quick-submit', {bubbles: true})); } } diff --git a/web_src/js/features/repo-code.js b/web_src/js/features/repo-code.js index 6a01a8445b5..306f38829fa 100644 --- a/web_src/js/features/repo-code.js +++ b/web_src/js/features/repo-code.js @@ -111,7 +111,7 @@ function showLineButton() { hideOnClick: true, content: menu, placement: 'right-start', - interactive: 'true', + interactive: true, onShow: (tippy) => { tippy.popper.addEventListener('click', () => { tippy.hide(); diff --git a/web_src/js/modules/tippy.js b/web_src/js/modules/tippy.js index b424cdfd50c..3409e1c7142 100644 --- a/web_src/js/modules/tippy.js +++ b/web_src/js/modules/tippy.js @@ -3,6 +3,11 @@ import tippy from 'tippy.js'; const visibleInstances = new Set(); export function createTippy(target, opts = {}) { + const {role, content, onHide: optsOnHide, onDestroy: optsOnDestroy, onShow: optOnShow} = opts; + delete opts.onHide; + delete opts.onDestroy; + delete opts.onShow; + const instance = tippy(target, { appendTo: document.body, animation: false, @@ -13,9 +18,11 @@ export function createTippy(target, opts = {}) { maxWidth: 500, // increase over default 350px onHide: (instance) => { visibleInstances.delete(instance); + return optsOnHide?.(instance); }, onDestroy: (instance) => { visibleInstances.delete(instance); + return optsOnDestroy?.(instance); }, onShow: (instance) => { // hide other tooltip instances so only one tooltip shows at a time @@ -25,18 +32,19 @@ export function createTippy(target, opts = {}) { } } visibleInstances.add(instance); + return optOnShow?.(instance); }, arrow: ``, role: 'menu', // HTML role attribute, only tooltips should use "tooltip" - theme: opts.role || 'menu', // CSS theme, we support either "tooltip" or "menu" + theme: role || 'menu', // CSS theme, we support either "tooltip" or "menu" ...opts, }); // for popups where content refers to a DOM element, we use the 'tippy-target' class // to initially hide the content, now we can remove it as the content has been removed // from the DOM by tippy - if (opts.content instanceof Element) { - opts.content.classList.remove('tippy-target'); + if (content instanceof Element) { + content.classList.remove('tippy-target'); } return instance;