From f7749432ab4bbb89942eba580031f168ac7af28e Mon Sep 17 00:00:00 2001 From: HesterG Date: Mon, 12 Jun 2023 14:30:12 +0800 Subject: [PATCH 01/24] save --- templates/repo/diff/new_review.tmpl | 17 ++++++++--------- web_src/js/features/repo-issue.js | 13 ++++++++++++- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index afb82a8d3d7ac..87c5cec6c5fd0 100644 --- a/templates/repo/diff/new_review.tmpl +++ b/templates/repo/diff/new_review.tmpl @@ -4,10 +4,9 @@ {{.PendingCodeCommentNumber}} {{svg "octicon-triangle-down" 14 "dropdown icon"}} -
+
-
- {{.CsrfTokenHtml}} +
{{$.locale.Tr "repo.diff.review.header"}}
@@ -32,20 +31,20 @@ {{$showSelfTooltip := (and $.IsSigned ($.Issue.IsPoster $.SignedUser.ID))}} {{if $showSelfTooltip}} - + {{else}} - + {{end}} - + {{if $showSelfTooltip}} - + {{else}} - + {{end}} - +
diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 0dc5728f58bc0..575bc8fde21ad 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -454,7 +454,18 @@ export function initRepoPullRequestReview() { if ($reviewBox.length === 1) { const _promise = initComboMarkdownEditor($reviewBox.find('.combo-markdown-editor')); } - + for (const btn of $reviewBox.querySelectorAll('.btn-submit')) { + btn.addEventListener('click', function(e) { + data = { + 'type': btn.getAttribute('data-type') + }; + fetch($reviewBox.getAttribute('data-link'), { + method: 'POST', + headers: {'X-Csrf-Token': csrfToken}, + body: JSON.stringify(data), + }); + }) + } // The following part is only for diff views if ($('.repository.pull.diff').length === 0) { return; From 3c56833c7ce2c8932f8875dab15fb16a57fdeb27 Mon Sep 17 00:00:00 2001 From: HesterG Date: Mon, 12 Jun 2023 15:26:41 +0800 Subject: [PATCH 02/24] change to fetch --- services/forms/repo_form.go | 8 +++--- templates/repo/diff/new_review.tmpl | 2 +- web_src/js/features/repo-issue.js | 38 ++++++++++++++++++++--------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 8108a55f7a35d..bb0fea22fe7c7 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -636,10 +636,10 @@ func (f *CodeCommentForm) Validate(req *http.Request, errs binding.Errors) bindi // SubmitReviewForm for submitting a finished code review type SubmitReviewForm struct { - Content string - Type string - CommitID string - Files []string + Content string `json:"content"` + Type string `json:"type"` + CommitID string `json:"commitID"` + Files []string `json:"files"` } // Validate validates the fields diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index 87c5cec6c5fd0..b5470c214e2c6 100644 --- a/templates/repo/diff/new_review.tmpl +++ b/templates/repo/diff/new_review.tmpl @@ -4,7 +4,7 @@ {{.PendingCodeCommentNumber}} {{svg "octicon-triangle-down" 14 "dropdown icon"}} -
+
diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 575bc8fde21ad..17518b49f11ea 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -391,6 +391,30 @@ export async function handleReply($el) { return editor; } +function prepareReviewBox () { + const files = document.querySelector('.files'); + let fileIds = []; + for (const file of files) { + fileIds.push(file.getAttribute('id')); + } + const markdown = document.querySelector('textarea.markdown-text-editor'); + for (const btn of $reviewBox.querySelectorAll('.btn-submit')) { + btn.addEventListener('click', function(e) { + data = { + 'commitID': $reviewBox.getAttribute('data-commit-id'), + 'type': btn.getAttribute('data-type'), + 'content': markdown.value, + 'files': fileIds, + }; + fetch($reviewBox.getAttribute('data-link'), { + method: 'POST', + headers: {'X-Csrf-Token': csrfToken}, + body: JSON.stringify(data), + }); + }) + } +} + export function initRepoPullRequestReview() { if (window.location.hash && window.location.hash.startsWith('#issuecomment-')) { // set scrollRestoration to 'manual' when there is a hash in url, so that the scroll position will not be remembered after refreshing @@ -453,19 +477,9 @@ export function initRepoPullRequestReview() { const $reviewBox = $('.review-box-panel'); if ($reviewBox.length === 1) { const _promise = initComboMarkdownEditor($reviewBox.find('.combo-markdown-editor')); + prepareReviewBox(); } - for (const btn of $reviewBox.querySelectorAll('.btn-submit')) { - btn.addEventListener('click', function(e) { - data = { - 'type': btn.getAttribute('data-type') - }; - fetch($reviewBox.getAttribute('data-link'), { - method: 'POST', - headers: {'X-Csrf-Token': csrfToken}, - body: JSON.stringify(data), - }); - }) - } + // The following part is only for diff views if ($('.repository.pull.diff').length === 0) { return; From 984a30bf46185f2fb921e7abcb04fdc766d3c5d6 Mon Sep 17 00:00:00 2001 From: HesterG Date: Mon, 12 Jun 2023 15:59:14 +0800 Subject: [PATCH 03/24] update --- web_src/js/features/repo-issue.js | 35 ++++++++++++++++++------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 17518b49f11ea..29db8ca312d74 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -391,22 +391,27 @@ export async function handleReply($el) { return editor; } -function prepareReviewBox () { - const files = document.querySelector('.files'); - let fileIds = []; - for (const file of files) { - fileIds.push(file.getAttribute('id')); - } - const markdown = document.querySelector('textarea.markdown-text-editor'); - for (const btn of $reviewBox.querySelectorAll('.btn-submit')) { - btn.addEventListener('click', function(e) { +function prepareReviewBox ($reviewBox) { + for (const btn of $reviewBox.find('.btn-submit')) { + const files = $('.files'); + let fileIds = []; + for (const file of files) { + fileIds.push($(file).attr('id')); + } + const markdown = $('textarea.markdown-text-editor'); + $(btn).on('click', (e) => { + console.log('commit-id', $reviewBox.attr('data-commit-id')); + console.log('type', $(btn).attr('data-type')); + console.log('content', markdown.val()); data = { - 'commitID': $reviewBox.getAttribute('data-commit-id'), - 'type': btn.getAttribute('data-type'), - 'content': markdown.value, - 'files': fileIds, + 'commitID': $reviewBox.attr('data-commit-id'), + 'type': $(btn).attr('data-type'), + 'content': markdown.val(), }; - fetch($reviewBox.getAttribute('data-link'), { + if (fileIds.length > 0) { + data['files'] = fileIds + } + fetch($reviewBox.attr('data-link'), { method: 'POST', headers: {'X-Csrf-Token': csrfToken}, body: JSON.stringify(data), @@ -477,7 +482,7 @@ export function initRepoPullRequestReview() { const $reviewBox = $('.review-box-panel'); if ($reviewBox.length === 1) { const _promise = initComboMarkdownEditor($reviewBox.find('.combo-markdown-editor')); - prepareReviewBox(); + prepareReviewBox($reviewBox); } // The following part is only for diff views From 2b7173425138056ee8bd8cc61660daae3f1f2e29 Mon Sep 17 00:00:00 2001 From: HesterG Date: Mon, 12 Jun 2023 17:17:43 +0800 Subject: [PATCH 04/24] change to post --- routers/web/repo/pull_review.go | 20 +++++++++++++++-- web_src/js/features/repo-issue.js | 37 ++++++++++++++++--------------- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 90cfd5bfcdb3a..14f1ece257a50 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -181,23 +181,33 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment) { ctx.HTML(http.StatusOK, tplConversation) } +type submitReviewResponse struct { + RedirectLink string `json:"redirectLink"` +} + // SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist func SubmitReview(ctx *context.Context) { form := web.GetForm(ctx).(*forms.SubmitReviewForm) issue := GetActionIssue(ctx) if !issue.IsPull { + fmt.Print("!issue.IsPull!issue.IsPull!issue.IsPull!issue.IsPull") return } if ctx.Written() { + fmt.Print("WrittenWrittenWritten") return } if ctx.HasError() { + fmt.Print("has error has error") ctx.Flash.Error(ctx.Data["ErrorMsg"].(string)) ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) return } reviewType := form.ReviewType() + fmt.Print("reviewType \n") + fmt.Print(form.Type) + fmt.Print(reviewType) switch reviewType { case issues_model.ReviewTypeUnknown: ctx.ServerError("ReviewType", fmt.Errorf("unknown ReviewType: %s", form.Type)) @@ -226,6 +236,8 @@ func SubmitReview(ctx *context.Context) { _, comm, err := pull_service.SubmitReview(ctx, ctx.Doer, ctx.Repo.GitRepo, issue, reviewType, form.Content, form.CommitID, attachments) if err != nil { + fmt.Print("errerrerrerr \n") + fmt.Print(err) 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)) @@ -234,8 +246,12 @@ func SubmitReview(ctx *context.Context) { } return } - - ctx.Redirect(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag())) + fmt.Print("RedirectRedirectRedirectRedirectRedirect \n") + fmt.Print(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag())) + resp := submitReviewResponse{ + RedirectLink: fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()), + } + ctx.JSON(http.StatusOK, resp) } // DismissReview dismissing stale review by repo admin diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 29db8ca312d74..035266e42012f 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -393,30 +393,31 @@ export async function handleReply($el) { function prepareReviewBox ($reviewBox) { for (const btn of $reviewBox.find('.btn-submit')) { - const files = $('.files'); - let fileIds = []; - for (const file of files) { - fileIds.push($(file).attr('id')); - } - const markdown = $('textarea.markdown-text-editor'); - $(btn).on('click', (e) => { - console.log('commit-id', $reviewBox.attr('data-commit-id')); - console.log('type', $(btn).attr('data-type')); - console.log('content', markdown.val()); - data = { + $(btn).on('click', async () => { + const fileIds = []; + for (const fileInput of $reviewBox.find('.dropzone .files input')) { + fileIds.push($(fileInput).attr('id')); + } + const $textEditor = $reviewBox.find('textarea.markdown-text-editor'); + const data = { 'commitID': $reviewBox.attr('data-commit-id'), 'type': $(btn).attr('data-type'), - 'content': markdown.val(), + 'content': $textEditor.val(), }; - if (fileIds.length > 0) { - data['files'] = fileIds - } - fetch($reviewBox.attr('data-link'), { + if (fileIds.length > 0) data['files'] = fileIds; + const response = await fetch($reviewBox.attr('data-url'), { method: 'POST', - headers: {'X-Csrf-Token': csrfToken}, + headers: { + 'Content-Type': 'application/json', + 'X-Csrf-Token': csrfToken, + }, body: JSON.stringify(data), }); - }) + if (response.status === 200) { + const {redirectLink} = await response.json(); + window.location.href = redirectLink; + } + }); } } From 2318bb30c35e1cd3ccc6affab5e893e17c403e14 Mon Sep 17 00:00:00 2001 From: HesterG Date: Mon, 12 Jun 2023 17:36:30 +0800 Subject: [PATCH 05/24] change redirect --- routers/web/repo/pull_review.go | 30 ++++++++++++------------------ web_src/js/features/repo-issue.js | 1 + 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 14f1ece257a50..508a8d2ae1969 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -181,10 +181,6 @@ func renderConversation(ctx *context.Context, comment *issues_model.Comment) { ctx.HTML(http.StatusOK, tplConversation) } -type submitReviewResponse struct { - RedirectLink string `json:"redirectLink"` -} - // SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist func SubmitReview(ctx *context.Context) { form := web.GetForm(ctx).(*forms.SubmitReviewForm) @@ -200,14 +196,13 @@ func SubmitReview(ctx *context.Context) { if ctx.HasError() { fmt.Print("has error has error") ctx.Flash.Error(ctx.Data["ErrorMsg"].(string)) - ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) + ctx.JSON(http.StatusOK, map[string]interface{}{ + "redirectLink": fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index), + }) return } reviewType := form.ReviewType() - fmt.Print("reviewType \n") - fmt.Print(form.Type) - fmt.Print(reviewType) switch reviewType { case issues_model.ReviewTypeUnknown: ctx.ServerError("ReviewType", fmt.Errorf("unknown ReviewType: %s", form.Type)) @@ -224,7 +219,9 @@ func SubmitReview(ctx *context.Context) { } ctx.Flash.Error(translated) - ctx.Redirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) + ctx.JSON(http.StatusOK, map[string]interface{}{ + "redirectLink": fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index), + }) return } } @@ -236,22 +233,19 @@ func SubmitReview(ctx *context.Context) { _, comm, err := pull_service.SubmitReview(ctx, ctx.Doer, ctx.Repo.GitRepo, issue, reviewType, form.Content, form.CommitID, attachments) if err != nil { - fmt.Print("errerrerrerr \n") - fmt.Print(err) 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.JSON(http.StatusOK, map[string]interface{}{ + "redirectLink": fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index), + }) } else { ctx.ServerError("SubmitReview", err) } return } - fmt.Print("RedirectRedirectRedirectRedirectRedirect \n") - fmt.Print(fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag())) - resp := submitReviewResponse{ - RedirectLink: fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()), - } - ctx.JSON(http.StatusOK, resp) + ctx.JSON(http.StatusOK, map[string]interface{}{ + "redirectLink": fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()), + }) } // DismissReview dismissing stale review by repo admin diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 035266e42012f..05a6fc9788abf 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -413,6 +413,7 @@ function prepareReviewBox ($reviewBox) { }, body: JSON.stringify(data), }); + console.log(response) if (response.status === 200) { const {redirectLink} = await response.json(); window.location.href = redirectLink; From 183dc8e22ae68887efcc3a41be3a63a082963144 Mon Sep 17 00:00:00 2001 From: HesterG Date: Mon, 12 Jun 2023 17:45:15 +0800 Subject: [PATCH 06/24] update --- routers/web/repo/pull_review.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 508a8d2ae1969..4859b9c6316c6 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -186,15 +186,12 @@ func SubmitReview(ctx *context.Context) { form := web.GetForm(ctx).(*forms.SubmitReviewForm) issue := GetActionIssue(ctx) if !issue.IsPull { - fmt.Print("!issue.IsPull!issue.IsPull!issue.IsPull!issue.IsPull") return } if ctx.Written() { - fmt.Print("WrittenWrittenWritten") return } if ctx.HasError() { - fmt.Print("has error has error") ctx.Flash.Error(ctx.Data["ErrorMsg"].(string)) ctx.JSON(http.StatusOK, map[string]interface{}{ "redirectLink": fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index), From ed545900b98affc4a99643a5d11f70bbd41d95d4 Mon Sep 17 00:00:00 2001 From: HesterG Date: Mon, 12 Jun 2023 18:07:06 +0800 Subject: [PATCH 07/24] add loading --- web_src/js/features/repo-issue.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 05a6fc9788abf..f2687b82c5896 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -394,6 +394,9 @@ export async function handleReply($el) { function prepareReviewBox ($reviewBox) { for (const btn of $reviewBox.find('.btn-submit')) { $(btn).on('click', async () => { + if ($reviewBox.hasClass('isFetching')) return; + $reviewBox.addClass('isFetching'); + $(btn).addClass('loading'); const fileIds = []; for (const fileInput of $reviewBox.find('.dropzone .files input')) { fileIds.push($(fileInput).attr('id')); @@ -413,10 +416,11 @@ function prepareReviewBox ($reviewBox) { }, body: JSON.stringify(data), }); - console.log(response) if (response.status === 200) { const {redirectLink} = await response.json(); window.location.href = redirectLink; + } else { + window.location.reload(); } }); } From 8f57fc188b70ca9178dd6eadfdd4a9c2fd6b8204 Mon Sep 17 00:00:00 2001 From: HesterG Date: Mon, 12 Jun 2023 18:10:38 +0800 Subject: [PATCH 08/24] add comments --- web_src/js/features/repo-issue.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index f2687b82c5896..7326723571725 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -416,11 +416,14 @@ function prepareReviewBox ($reviewBox) { }, body: JSON.stringify(data), }); + // if page is redirected, no need to remove loading status if (response.status === 200) { const {redirectLink} = await response.json(); window.location.href = redirectLink; + // error occurs, still on the page, remove loading status } else { - window.location.reload(); + $reviewBox.removeClass('isFetching'); + $(btn).removeClass('loading'); } }); } From 0dc11e1d9f5c8b94bff99095ca55b0aaca175a54 Mon Sep 17 00:00:00 2001 From: HesterG Date: Mon, 12 Jun 2023 18:13:49 +0800 Subject: [PATCH 09/24] update js --- templates/repo/diff/new_review.tmpl | 1 - web_src/js/features/repo-issue.js | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index b5470c214e2c6..3f96ec91cb21a 100644 --- a/templates/repo/diff/new_review.tmpl +++ b/templates/repo/diff/new_review.tmpl @@ -7,7 +7,6 @@
-
{{$.locale.Tr "repo.diff.review.header"}}
{{svg "octicon-x" 16}} diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 7326723571725..41821e278bf9b 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -391,12 +391,12 @@ export async function handleReply($el) { return editor; } -function prepareReviewBox ($reviewBox) { +function prepareReviewBoxButtons ($reviewBox) { for (const btn of $reviewBox.find('.btn-submit')) { $(btn).on('click', async () => { if ($reviewBox.hasClass('isFetching')) return; $reviewBox.addClass('isFetching'); - $(btn).addClass('loading'); + $(this).addClass('loading'); const fileIds = []; for (const fileInput of $reviewBox.find('.dropzone .files input')) { fileIds.push($(fileInput).attr('id')); @@ -404,7 +404,7 @@ function prepareReviewBox ($reviewBox) { const $textEditor = $reviewBox.find('textarea.markdown-text-editor'); const data = { 'commitID': $reviewBox.attr('data-commit-id'), - 'type': $(btn).attr('data-type'), + 'type': $(this).attr('data-type'), 'content': $textEditor.val(), }; if (fileIds.length > 0) data['files'] = fileIds; @@ -423,7 +423,7 @@ function prepareReviewBox ($reviewBox) { // error occurs, still on the page, remove loading status } else { $reviewBox.removeClass('isFetching'); - $(btn).removeClass('loading'); + $(this).removeClass('loading'); } }); } @@ -491,7 +491,7 @@ export function initRepoPullRequestReview() { const $reviewBox = $('.review-box-panel'); if ($reviewBox.length === 1) { const _promise = initComboMarkdownEditor($reviewBox.find('.combo-markdown-editor')); - prepareReviewBox($reviewBox); + prepareReviewBoxButtons($reviewBox); } // The following part is only for diff views From 4602642e235bbd8acf0601fcb1b997d8c11d2545 Mon Sep 17 00:00:00 2001 From: HesterG Date: Mon, 12 Jun 2023 18:23:33 +0800 Subject: [PATCH 10/24] update --- web_src/js/features/repo-issue.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 41821e278bf9b..aec251020bb17 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -393,7 +393,7 @@ export async function handleReply($el) { function prepareReviewBoxButtons ($reviewBox) { for (const btn of $reviewBox.find('.btn-submit')) { - $(btn).on('click', async () => { + $(btn).on('click', async function () { if ($reviewBox.hasClass('isFetching')) return; $reviewBox.addClass('isFetching'); $(this).addClass('loading'); @@ -420,7 +420,7 @@ function prepareReviewBoxButtons ($reviewBox) { if (response.status === 200) { const {redirectLink} = await response.json(); window.location.href = redirectLink; - // error occurs, still on the page, remove loading status + // error occurs, still on the same page, remove loading status } else { $reviewBox.removeClass('isFetching'); $(this).removeClass('loading'); From 9db88559b4a3447f97ab9635aa2ac0114cda3a46 Mon Sep 17 00:00:00 2001 From: HesterG Date: Tue, 13 Jun 2023 09:22:10 +0800 Subject: [PATCH 11/24] comment --- web_src/js/features/repo-issue.js | 1 + 1 file changed, 1 insertion(+) diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index aec251020bb17..4ece45919a227 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -394,6 +394,7 @@ export async function handleReply($el) { function prepareReviewBoxButtons ($reviewBox) { for (const btn of $reviewBox.find('.btn-submit')) { $(btn).on('click', async function () { + // Do not fetch repeatedly if post request has sent if ($reviewBox.hasClass('isFetching')) return; $reviewBox.addClass('isFetching'); $(this).addClass('loading'); From acdd3cd3fb0c469905f5816bddc32b16e6a35b0d Mon Sep 17 00:00:00 2001 From: HesterG Date: Tue, 13 Jun 2023 09:40:51 +0800 Subject: [PATCH 12/24] comment --- web_src/js/features/repo-issue.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 4ece45919a227..5ee97156fd743 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -394,7 +394,9 @@ export async function handleReply($el) { function prepareReviewBoxButtons ($reviewBox) { for (const btn of $reviewBox.find('.btn-submit')) { $(btn).on('click', async function () { - // Do not fetch repeatedly if post request has sent + // Do not fetch repeatedly if post request has sent. + // Added to $reviewBox to avoid sending several requests when approve, comment, + // and request change buttons are clicked. if ($reviewBox.hasClass('isFetching')) return; $reviewBox.addClass('isFetching'); $(this).addClass('loading'); From f029cfbeb1af0af06aaa158d7a0432302c890911 Mon Sep 17 00:00:00 2001 From: HesterG Date: Tue, 13 Jun 2023 11:05:22 +0800 Subject: [PATCH 13/24] change to resirect --- routers/web/repo/pull_review.go | 8 ++++---- web_src/js/features/repo-issue.js | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 4859b9c6316c6..8580578fc8370 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -194,7 +194,7 @@ func SubmitReview(ctx *context.Context) { if ctx.HasError() { ctx.Flash.Error(ctx.Data["ErrorMsg"].(string)) ctx.JSON(http.StatusOK, map[string]interface{}{ - "redirectLink": fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index), + "redirect": fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index), }) return } @@ -217,7 +217,7 @@ func SubmitReview(ctx *context.Context) { ctx.Flash.Error(translated) ctx.JSON(http.StatusOK, map[string]interface{}{ - "redirectLink": fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index), + "redirect": fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index), }) return } @@ -233,7 +233,7 @@ func SubmitReview(ctx *context.Context) { if issues_model.IsContentEmptyErr(err) { ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty")) ctx.JSON(http.StatusOK, map[string]interface{}{ - "redirectLink": fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index), + "redirect": fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index), }) } else { ctx.ServerError("SubmitReview", err) @@ -241,7 +241,7 @@ func SubmitReview(ctx *context.Context) { return } ctx.JSON(http.StatusOK, map[string]interface{}{ - "redirectLink": fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()), + "redirect": fmt.Sprintf("%s/pulls/%d#%s", ctx.Repo.RepoLink, issue.Index, comm.HashTag()), }) } diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 5ee97156fd743..e66c55ddeeb5d 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -421,8 +421,8 @@ function prepareReviewBoxButtons ($reviewBox) { }); // if page is redirected, no need to remove loading status if (response.status === 200) { - const {redirectLink} = await response.json(); - window.location.href = redirectLink; + const {redirect} = await response.json(); + window.location.href = redirect; // error occurs, still on the same page, remove loading status } else { $reviewBox.removeClass('isFetching'); From bfe1c6b6cae0e3d50588d62f4efde84bef8872ba Mon Sep 17 00:00:00 2001 From: HesterG Date: Tue, 13 Jun 2023 14:38:18 +0800 Subject: [PATCH 14/24] update to dataform --- services/forms/repo_form.go | 8 ++--- templates/repo/diff/new_review.tmpl | 19 ++++++----- web_src/js/features/common-global.js | 33 +++++++++++++++++++ web_src/js/features/repo-issue.js | 47 +++------------------------- 4 files changed, 53 insertions(+), 54 deletions(-) diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index bb0fea22fe7c7..8108a55f7a35d 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -636,10 +636,10 @@ func (f *CodeCommentForm) Validate(req *http.Request, errs binding.Errors) bindi // SubmitReviewForm for submitting a finished code review type SubmitReviewForm struct { - Content string `json:"content"` - Type string `json:"type"` - CommitID string `json:"commitID"` - Files []string `json:"files"` + Content string + Type string + CommitID string + Files []string } // Validate validates the fields diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index 3f96ec91cb21a..d01be57d484ca 100644 --- a/templates/repo/diff/new_review.tmpl +++ b/templates/repo/diff/new_review.tmpl @@ -4,9 +4,11 @@ {{.PendingCodeCommentNumber}} {{svg "octicon-triangle-down" 14 "dropdown icon"}} -
+
-
+
+ {{.CsrfTokenHtml}} +
{{$.locale.Tr "repo.diff.review.header"}}
{{svg "octicon-x" 16}} @@ -27,23 +29,24 @@
{{end}}
+ {{$showSelfTooltip := (and $.IsSigned ($.Issue.IsPoster $.SignedUser.ID))}} {{if $showSelfTooltip}} - + {{else}} - + {{end}} - + {{if $showSelfTooltip}} - + {{else}} - + {{end}} -
+
diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index 46a80beb5185c..05d5614cdd917 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -114,6 +114,39 @@ export function initGlobalCommon() { if (btn.classList.contains('loading')) return e.preventDefault(); btn.classList.add('loading'); }); + + function prepareFetchForm($form) { + $form.addEventListener('submit', async (e) => { + e.preventDefault(); + const submitter = e.submitter; + // Do not fetch repeatedly if post request has sent. + // Added to $form to avoid sending several requests if there are several buttons inside the form + if ($form.classList.contains('isFetching')) return; + $form.classList.add('isFetching'); + // if the submitter is a button, add loading class to it + if (submitter.tagName === 'BUTTON') submitter.classList.add('loading'); + const response = await fetch($form.getAttribute('action'), { + method: 'POST', + headers: { + 'X-Csrf-Token': csrfToken, + }, + body: new FormData($form), + }); + // if page is redirected, no need to remove loading status + if (response.status === 200) { + const {redirect} = await response.json(); + window.location.href = redirect; + // error occurs, still on the same page, remove loading status + } else { + $form.classList.remove('isFetching'); + if (submitter.tagName === 'BUTTON') submitter.classList.remove('loading'); + } + }); + } + + for (const el of document.querySelectorAll('.fetch-form')) { + prepareFetchForm(el); + } } export function initGlobalDropzone() { diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index e66c55ddeeb5d..cc6456e2fa750 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -391,47 +391,6 @@ export async function handleReply($el) { return editor; } -function prepareReviewBoxButtons ($reviewBox) { - for (const btn of $reviewBox.find('.btn-submit')) { - $(btn).on('click', async function () { - // Do not fetch repeatedly if post request has sent. - // Added to $reviewBox to avoid sending several requests when approve, comment, - // and request change buttons are clicked. - if ($reviewBox.hasClass('isFetching')) return; - $reviewBox.addClass('isFetching'); - $(this).addClass('loading'); - const fileIds = []; - for (const fileInput of $reviewBox.find('.dropzone .files input')) { - fileIds.push($(fileInput).attr('id')); - } - const $textEditor = $reviewBox.find('textarea.markdown-text-editor'); - const data = { - 'commitID': $reviewBox.attr('data-commit-id'), - 'type': $(this).attr('data-type'), - 'content': $textEditor.val(), - }; - if (fileIds.length > 0) data['files'] = fileIds; - const response = await fetch($reviewBox.attr('data-url'), { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-Csrf-Token': csrfToken, - }, - body: JSON.stringify(data), - }); - // if page is redirected, no need to remove loading status - if (response.status === 200) { - const {redirect} = await response.json(); - window.location.href = redirect; - // error occurs, still on the same page, remove loading status - } else { - $reviewBox.removeClass('isFetching'); - $(this).removeClass('loading'); - } - }); - } -} - export function initRepoPullRequestReview() { if (window.location.hash && window.location.hash.startsWith('#issuecomment-')) { // set scrollRestoration to 'manual' when there is a hash in url, so that the scroll position will not be remembered after refreshing @@ -494,9 +453,13 @@ export function initRepoPullRequestReview() { const $reviewBox = $('.review-box-panel'); if ($reviewBox.length === 1) { const _promise = initComboMarkdownEditor($reviewBox.find('.combo-markdown-editor')); - prepareReviewBoxButtons($reviewBox); } + for (const btn of $reviewBox.find('.btn-submit')) { + $(btn).on('click', function () { + $reviewBox.find('input[name="type"]').attr('value', $(this).attr('value')); + }) + } // The following part is only for diff views if ($('.repository.pull.diff').length === 0) { return; From 1a8472f6227016d78c574ef26c7a4985142bbd2d Mon Sep 17 00:00:00 2001 From: HesterG Date: Tue, 13 Jun 2023 14:52:32 +0800 Subject: [PATCH 15/24] add method --- web_src/js/features/common-global.js | 12 +++++++----- web_src/js/features/repo-issue.js | 1 + 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index 05d5614cdd917..56095b0c0b790 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -125,13 +125,15 @@ export function initGlobalCommon() { $form.classList.add('isFetching'); // if the submitter is a button, add loading class to it if (submitter.tagName === 'BUTTON') submitter.classList.add('loading'); - const response = await fetch($form.getAttribute('action'), { - method: 'POST', + const method = $form.getAttribute('method').toUpperCase(); + const req = { + method, headers: { 'X-Csrf-Token': csrfToken, - }, - body: new FormData($form), - }); + } + } + if (method !== 'GET') req['body'] = new FormData($form); + const response = await fetch($form.getAttribute('action'), req); // if page is redirected, no need to remove loading status if (response.status === 200) { const {redirect} = await response.json(); diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index cc6456e2fa750..6362999d3ccbf 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -455,6 +455,7 @@ export function initRepoPullRequestReview() { const _promise = initComboMarkdownEditor($reviewBox.find('.combo-markdown-editor')); } + // set the value of type to the button's type value for the submit review panel for (const btn of $reviewBox.find('.btn-submit')) { $(btn).on('click', function () { $reviewBox.find('input[name="type"]').attr('value', $(this).attr('value')); From 2d7d5d00c9d701febbc48f769de894606e26818d Mon Sep 17 00:00:00 2001 From: HesterG Date: Tue, 13 Jun 2023 15:17:29 +0800 Subject: [PATCH 16/24] rename and append submitter value --- templates/repo/diff/new_review.tmpl | 1 - web_src/js/features/common-global.js | 24 +++++++++++++++--------- web_src/js/features/repo-issue.js | 6 ------ 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index d01be57d484ca..4bbb9961e57cb 100644 --- a/templates/repo/diff/new_review.tmpl +++ b/templates/repo/diff/new_review.tmpl @@ -29,7 +29,6 @@
{{end}}
- {{$showSelfTooltip := (and $.IsSigned ($.Issue.IsPoster $.SignedUser.ID))}} {{if $showSelfTooltip}} diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index 56095b0c0b790..a7af9dc9475ca 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -115,32 +115,38 @@ export function initGlobalCommon() { btn.classList.add('loading'); }); - function prepareFetchForm($form) { - $form.addEventListener('submit', async (e) => { + function prepareFetchForm(formEl) { + formEl.addEventListener('submit', async (e) => { e.preventDefault(); const submitter = e.submitter; // Do not fetch repeatedly if post request has sent. - // Added to $form to avoid sending several requests if there are several buttons inside the form - if ($form.classList.contains('isFetching')) return; - $form.classList.add('isFetching'); + // Added to formEl to avoid sending several requests if there are several buttons inside the form + if (formEl.classList.contains('isFetching')) return; + formEl.classList.add('isFetching'); // if the submitter is a button, add loading class to it if (submitter.tagName === 'BUTTON') submitter.classList.add('loading'); - const method = $form.getAttribute('method').toUpperCase(); + const method = formEl.getAttribute('method').toUpperCase(); const req = { method, headers: { 'X-Csrf-Token': csrfToken, } } - if (method !== 'GET') req['body'] = new FormData($form); - const response = await fetch($form.getAttribute('action'), req); + if (method !== 'GET') { + const formData = new FormData(formEl); + if (submitter.hasAttribute('name') && submitter.hasAttribute('value')) { + formData.append(submitter.getAttribute('name'), submitter.getAttribute('value')); + } + req['body'] = formData; + } + const response = await fetch(formEl.getAttribute('action'), req); // if page is redirected, no need to remove loading status if (response.status === 200) { const {redirect} = await response.json(); window.location.href = redirect; // error occurs, still on the same page, remove loading status } else { - $form.classList.remove('isFetching'); + formEl.classList.remove('isFetching'); if (submitter.tagName === 'BUTTON') submitter.classList.remove('loading'); } }); diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 6362999d3ccbf..0dc5728f58bc0 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -455,12 +455,6 @@ export function initRepoPullRequestReview() { const _promise = initComboMarkdownEditor($reviewBox.find('.combo-markdown-editor')); } - // set the value of type to the button's type value for the submit review panel - for (const btn of $reviewBox.find('.btn-submit')) { - $(btn).on('click', function () { - $reviewBox.find('input[name="type"]').attr('value', $(this).attr('value')); - }) - } // The following part is only for diff views if ($('.repository.pull.diff').length === 0) { return; From f4da6065d28572fb9031e9caf1d6b4daf3d3b272 Mon Sep 17 00:00:00 2001 From: HesterG Date: Tue, 13 Jun 2023 15:23:54 +0800 Subject: [PATCH 17/24] comment --- web_src/js/features/common-global.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index a7af9dc9475ca..42666710f0b32 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -119,7 +119,7 @@ export function initGlobalCommon() { formEl.addEventListener('submit', async (e) => { e.preventDefault(); const submitter = e.submitter; - // Do not fetch repeatedly if post request has sent. + // Do not fetch repeatedly if request has sent. // Added to formEl to avoid sending several requests if there are several buttons inside the form if (formEl.classList.contains('isFetching')) return; formEl.classList.add('isFetching'); From b37a7db9347f56d14826e32abca0b8710762dff8 Mon Sep 17 00:00:00 2001 From: HesterG Date: Tue, 13 Jun 2023 15:29:14 +0800 Subject: [PATCH 18/24] comment --- web_src/js/features/common-global.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index 42666710f0b32..2cc532f527a97 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -140,7 +140,8 @@ export function initGlobalCommon() { req['body'] = formData; } const response = await fetch(formEl.getAttribute('action'), req); - // if page is redirected, no need to remove loading status + // if page is redirected, no need to remove isFetching + // because request may be sent again if isFetching is removed here when network is slow if (response.status === 200) { const {redirect} = await response.json(); window.location.href = redirect; From 74e330dbae6b0a404dd3edd79878d9a7f960eab8 Mon Sep 17 00:00:00 2001 From: HesterG Date: Tue, 13 Jun 2023 16:37:34 +0800 Subject: [PATCH 19/24] fix lint --- web_src/js/features/common-global.js | 76 ++++++++++++++-------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index 2cc532f527a97..a9d1403030ef0 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -60,6 +60,44 @@ export function initGlobalButtonClickOnEnter() { }); } +function prepareFetchForm(formEl) { + formEl.addEventListener('submit', async (e) => { + e.preventDefault(); + const submitter = e.submitter; + // Do not fetch repeatedly if request has sent. + // Added to formEl to avoid sending several requests if there are several buttons inside the form + if (formEl.classList.contains('isFetching')) return; + formEl.classList.add('isFetching'); + // if the submitter is a button, add loading class to it + if (submitter.tagName === 'BUTTON') submitter.classList.add('loading'); + const method = formEl.getAttribute('method').toUpperCase(); + const req = { + method, + headers: { + 'X-Csrf-Token': csrfToken, + } + }; + if (method !== 'GET') { + const formData = new FormData(formEl); + if (submitter.hasAttribute('name') && submitter.hasAttribute('value')) { + formData.append(submitter.getAttribute('name'), submitter.getAttribute('value')); + } + req['body'] = formData; + } + const response = await fetch(formEl.getAttribute('action'), req); + // if page is redirected, no need to remove isFetching + // because request may be sent again if isFetching is removed here when network is slow + if (response.status === 200) { + const {redirect} = await response.json(); + window.location.href = redirect; + // error occurs, still on the same page, remove loading status + } else { + formEl.classList.remove('isFetching'); + if (submitter.tagName === 'BUTTON') submitter.classList.remove('loading'); + } + }); +} + export function initGlobalCommon() { // Semantic UI modules. const $uiDropdowns = $('.ui.dropdown'); @@ -115,44 +153,6 @@ export function initGlobalCommon() { btn.classList.add('loading'); }); - function prepareFetchForm(formEl) { - formEl.addEventListener('submit', async (e) => { - e.preventDefault(); - const submitter = e.submitter; - // Do not fetch repeatedly if request has sent. - // Added to formEl to avoid sending several requests if there are several buttons inside the form - if (formEl.classList.contains('isFetching')) return; - formEl.classList.add('isFetching'); - // if the submitter is a button, add loading class to it - if (submitter.tagName === 'BUTTON') submitter.classList.add('loading'); - const method = formEl.getAttribute('method').toUpperCase(); - const req = { - method, - headers: { - 'X-Csrf-Token': csrfToken, - } - } - if (method !== 'GET') { - const formData = new FormData(formEl); - if (submitter.hasAttribute('name') && submitter.hasAttribute('value')) { - formData.append(submitter.getAttribute('name'), submitter.getAttribute('value')); - } - req['body'] = formData; - } - const response = await fetch(formEl.getAttribute('action'), req); - // if page is redirected, no need to remove isFetching - // because request may be sent again if isFetching is removed here when network is slow - if (response.status === 200) { - const {redirect} = await response.json(); - window.location.href = redirect; - // error occurs, still on the same page, remove loading status - } else { - formEl.classList.remove('isFetching'); - if (submitter.tagName === 'BUTTON') submitter.classList.remove('loading'); - } - }); - } - for (const el of document.querySelectorAll('.fetch-form')) { prepareFetchForm(el); } From c45a1e1ae11013471420de5263f47ddd0f44a105 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 13 Jun 2023 17:07:44 +0800 Subject: [PATCH 20/24] fine tune (wip) --- modules/context/base.go | 4 ++ routers/web/repo/pull_review.go | 16 ++---- templates/repo/diff/new_review.tmpl | 2 +- web_src/js/features/common-global.js | 83 ++++++++++++++++------------ 4 files changed, 56 insertions(+), 49 deletions(-) diff --git a/modules/context/base.go b/modules/context/base.go index ac9b52d51cf34..5ae5e65d3ed41 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/repo/pull_review.go b/routers/web/repo/pull_review.go index 8580578fc8370..69d36ff4a47ef 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -193,9 +193,7 @@ func SubmitReview(ctx *context.Context) { } if ctx.HasError() { ctx.Flash.Error(ctx.Data["ErrorMsg"].(string)) - ctx.JSON(http.StatusOK, map[string]interface{}{ - "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 } @@ -216,9 +214,7 @@ func SubmitReview(ctx *context.Context) { } ctx.Flash.Error(translated) - ctx.JSON(http.StatusOK, map[string]interface{}{ - "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 } } @@ -232,17 +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.JSON(http.StatusOK, map[string]interface{}{ - "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.JSON(http.StatusOK, map[string]interface{}{ - "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/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index 4bbb9961e57cb..c407064176da3 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/js/features/common-global.js b/web_src/js/features/common-global.js index a9d1403030ef0..a06e536c804a4 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -60,42 +60,55 @@ export function initGlobalButtonClickOnEnter() { }); } -function prepareFetchForm(formEl) { - formEl.addEventListener('submit', async (e) => { - e.preventDefault(); - const submitter = e.submitter; - // Do not fetch repeatedly if request has sent. - // Added to formEl to avoid sending several requests if there are several buttons inside the form - if (formEl.classList.contains('isFetching')) return; - formEl.classList.add('isFetching'); - // if the submitter is a button, add loading class to it - if (submitter.tagName === 'BUTTON') submitter.classList.add('loading'); - const method = formEl.getAttribute('method').toUpperCase(); - const req = { - method, - headers: { - 'X-Csrf-Token': csrfToken, - } - }; - if (method !== 'GET') { - const formData = new FormData(formEl); - if (submitter.hasAttribute('name') && submitter.hasAttribute('value')) { - formData.append(submitter.getAttribute('name'), submitter.getAttribute('value')); +async function fetchFormAction(e) { + if (!e.target.classList.contains('form-fetch-action')) return; + + e.preventDefault(); + const formEl = e.target; + if (formEl.classList.contains('loading')) return; + + // TODO: fine tune the UI feedback + formEl.classList.add('loading'); + e.submitter?.classList.add('loading'); + + const formMethod = formEl.getAttribute('method') || 'get'; + const formActionUrl = formEl.getAttribute('action'); + const formData = new FormData(formEl); + const [submitterName, submitterValue] = [e.submitter?.hasAttribute('name'), e.submitter?.hasAttribute('value')]; + if (submitterName) { + formData.append(submitterName, submitterValue); + } + + const req = {method: formMethod.toUpperCase(), headers: {'X-Csrf-Token': csrfToken}}; + if (formMethod === 'GET') { + // TODO: append to the action URL + } else { + req.body = formData; + } + + const onError = () => { + // TODO: show error to end users + formEl.classList.remove('loading'); + e.submitter?.classList.remove('loading'); + }; + + try { + const resp = await fetch(formActionUrl, req); + if (resp.status === 200) { + const {redirect} = await resp.json(); + if (redirect) { + window.location.href = redirect; + } else { + // TODO: remove areYouSure form check + window.reload(); } - req['body'] = formData; - } - const response = await fetch(formEl.getAttribute('action'), req); - // if page is redirected, no need to remove isFetching - // because request may be sent again if isFetching is removed here when network is slow - if (response.status === 200) { - const {redirect} = await response.json(); - window.location.href = redirect; - // error occurs, still on the same page, remove loading status } else { - formEl.classList.remove('isFetching'); - if (submitter.tagName === 'BUTTON') submitter.classList.remove('loading'); + // TODO: show error to end users + onError(); } - }); + } catch { + onError(); + } } export function initGlobalCommon() { @@ -153,9 +166,7 @@ export function initGlobalCommon() { btn.classList.add('loading'); }); - for (const el of document.querySelectorAll('.fetch-form')) { - prepareFetchForm(el); - } + document.addEventListener('submit', fetchFormAction); } export function initGlobalDropzone() { From 3b271d71794ad78731801b0a051fe47b28bdaed2 Mon Sep 17 00:00:00 2001 From: HesterG Date: Tue, 13 Jun 2023 17:40:36 +0800 Subject: [PATCH 21/24] quick fix for submitterName and value --- web_src/js/features/common-global.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index a06e536c804a4..a2617ea341483 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -74,7 +74,7 @@ async function fetchFormAction(e) { const formMethod = formEl.getAttribute('method') || 'get'; const formActionUrl = formEl.getAttribute('action'); const formData = new FormData(formEl); - const [submitterName, submitterValue] = [e.submitter?.hasAttribute('name'), e.submitter?.hasAttribute('value')]; + const [submitterName, submitterValue] = [e.submitter?.getAttribute('name'), e.submitter?.getAttribute('value')]; if (submitterName) { formData.append(submitterName, submitterValue); } From b8f64caa670f98869406f0f98939590979eda8a7 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 14 Jun 2023 02:47:29 +0800 Subject: [PATCH 22/24] improve --- templates/devtest/fetch-action.tmpl | 26 ++++++++++++ templates/devtest/list.tmpl | 17 ++++---- web_src/js/features/common-global.js | 59 +++++++++++++++++----------- 3 files changed, 72 insertions(+), 30 deletions(-) create mode 100644 templates/devtest/fetch-action.tmpl diff --git a/templates/devtest/fetch-action.tmpl b/templates/devtest/fetch-action.tmpl new file mode 100644 index 0000000000000..4f6017653c303 --- /dev/null +++ b/templates/devtest/fetch-action.tmpl @@ -0,0 +1,26 @@ +{{template "base/head" .}} +
+
+

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
+
+ + + +
+ +
+
+
+
+{{template "base/footer" .}} diff --git a/templates/devtest/list.tmpl b/templates/devtest/list.tmpl index 5044f2a501963..90b1fcc9d0411 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/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index 8bc570e313ce1..8c30eb5929bad 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -60,7 +60,7 @@ export function initGlobalButtonClickOnEnter() { }); } -async function fetchFormAction(e) { +async function formFetchAction(e) { if (!e.target.classList.contains('form-fetch-action')) return; e.preventDefault(); @@ -76,14 +76,23 @@ async function fetchFormAction(e) { const formData = new FormData(formEl); const [submitterName, submitterValue] = [e.submitter?.getAttribute('name'), e.submitter?.getAttribute('value')]; if (submitterName) { - formData.append(submitterName, submitterValue); + formData.append(submitterName, submitterValue || ''); } - const req = {method: formMethod.toUpperCase(), headers: {'X-Csrf-Token': csrfToken}}; - if (formMethod === 'GET') { - // TODO: append to the action URL + 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 { - req.body = formData; + reqOpt.body = formData; } const onError = () => { @@ -92,23 +101,27 @@ async function fetchFormAction(e) { e.submitter?.classList.remove('loading'); }; - try { - const resp = await fetch(formActionUrl, req); - if (resp.status === 200) { - const {redirect} = await resp.json(); - if (redirect) { - window.location.href = redirect; + const doRequest = async () => { + try { + const resp = await fetch(reqUrl, reqOpt); + if (resp.status === 200) { + const {redirect} = await resp.json(); + if (redirect) { + window.location.href = redirect; + } else { + // TODO: remove areYouSure form check + window.reload(); + } } else { - // TODO: remove areYouSure form check - window.reload(); + // TODO: show error to end users + onError(); } - } else { - // TODO: show error to end users + } catch { onError(); } - } catch { - onError(); - } + }; + + await doRequest(); } export function initGlobalCommon() { @@ -166,7 +179,7 @@ export function initGlobalCommon() { btn.classList.add('loading'); }); - document.addEventListener('submit', fetchFormAction); + document.addEventListener('submit', formFetchAction); } export function initGlobalDropzone() { @@ -235,7 +248,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 @@ -254,7 +267,7 @@ function linkAction(e) { const modalConfirmHtml = htmlEscape($this.attr('data-modal-confirm') || ''); if (!modalConfirmHtml) { - request(); + doRequest(); return; } @@ -273,7 +286,7 @@ function linkAction(e) { $modal.appendTo(document.body); $modal.modal({ onApprove() { - request(); + doRequest(); }, onHidden() { $modal.remove(); From 722d0eda3b861bb40125c10a4351d083021d6e74 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 14 Jun 2023 12:59:03 +0800 Subject: [PATCH 23/24] improve --- routers/web/devtest/devtest.go | 10 +++++++ routers/web/web.go | 1 + templates/devtest/fetch-action.tmpl | 24 ++++++++++++--- templates/devtest/gitea-ui.tmpl | 11 +++++++ web_src/css/modules/animations.css | 21 +++++++------ web_src/css/modules/tippy.css | 6 ++++ web_src/js/features/common-global.js | 40 ++++++++++++++++--------- web_src/js/features/comp/QuickSubmit.js | 21 ++++++++----- web_src/js/features/repo-code.js | 2 +- web_src/js/modules/tippy.js | 14 +++++++-- 10 files changed, 112 insertions(+), 38 deletions(-) diff --git a/routers/web/devtest/devtest.go b/routers/web/devtest/devtest.go index 48875e306d29e..64b732c035671 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/web.go b/routers/web/web.go index 1e235a3c3ce75..8683ef221dd97 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 index 4f6017653c303..2fb7289ebe9a7 100644 --- a/templates/devtest/fetch-action.tmpl +++ b/templates/devtest/fetch-action.tmpl @@ -1,5 +1,6 @@ {{template "base/head" .}}
+ {{template "base/alert" .}}

link-action

@@ -7,20 +8,35 @@ 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 824b7d0db6952..516b73cf0952e 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/web_src/css/modules/animations.css b/web_src/css/modules/animations.css index 3f5e8bd2676cd..dcd0e075376b2 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 bd55b9d6b9377..fe32597280331 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 8c30eb5929bad..0ac51d426542c 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; @@ -65,11 +66,12 @@ async function formFetchAction(e) { e.preventDefault(); const formEl = e.target; - if (formEl.classList.contains('loading')) return; + if (formEl.classList.contains('is-loading')) return; - // TODO: fine tune the UI feedback - formEl.classList.add('loading'); - e.submitter?.classList.add('loading'); + 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'); @@ -95,10 +97,20 @@ async function formFetchAction(e) { reqOpt.body = formData; } - const onError = () => { - // TODO: show error to end users - formEl.classList.remove('loading'); - e.submitter?.classList.remove('loading'); + 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 () => { @@ -109,18 +121,18 @@ async function formFetchAction(e) { if (redirect) { window.location.href = redirect; } else { - // TODO: remove areYouSure form check - window.reload(); + formEl.classList.remove('dirty'); // remove the areYouSure check before reloading + window.location.reload(); } } else { - // TODO: show error to end users - onError(); + onError(`server error: ${resp.status}`); } - } catch { - onError(); + } catch (e) { + onError(e.error); } }; + // TODO: add "confirm" support like "link-action" in the future await doRequest(); } diff --git a/web_src/js/features/comp/QuickSubmit.js b/web_src/js/features/comp/QuickSubmit.js index d598a59655308..2587375a717d6 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 6a01a8445b575..306f38829fad2 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 b424cdfd50c7f..3409e1c714232 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; From 9f428a3ed9f850b8f15304a2087941a511fb24e7 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 14 Jun 2023 13:23:18 +0800 Subject: [PATCH 24/24] improve --- web_src/js/features/common-global.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index 0ac51d426542c..c0e66be51c9f6 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -7,7 +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"; +import {createTippy} from '../modules/tippy.js'; const {appUrl, csrfToken, i18n} = window.config; @@ -118,10 +118,10 @@ async function formFetchAction(e) { 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 { - formEl.classList.remove('dirty'); // remove the areYouSure check before reloading window.location.reload(); } } else {