From 84f8ef3df6316cb8fe8048556288452c07da4d7b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 19 Jan 2022 01:28:38 +0800 Subject: [PATCH] Fix PR comments UI (#18323) Closes: * Review comment cannot be edited #17768 * Changing PR Comment Resolved State Disables Further Changes #18315 --- models/issue_comment.go | 2 +- routers/api/v1/repo/issue_comment.go | 4 +- routers/web/repo/issue.go | 18 +++--- .../repo/issue/view_content/comments.tmpl | 61 ++++++++++++++----- web_src/js/features/repo-diff.js | 2 +- web_src/js/features/repo-issue-content.js | 2 +- web_src/less/_repository.less | 2 + 7 files changed, 64 insertions(+), 27 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index 99c38bcf5..f4a6b3ce1 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -519,7 +519,7 @@ func (c *Comment) LoadPoster() error { return c.loadPoster(db.GetEngine(db.DefaultContext)) } -// LoadAttachments loads attachments +// LoadAttachments loads attachments (it never returns error, the error during `GetAttachmentsByCommentIDCtx` is ignored) func (c *Comment) LoadAttachments() error { if len(c.Attachments) > 0 { return nil diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index b929cec37..f90028a0a 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -537,7 +537,9 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption) if !ctx.IsSigned || (ctx.User.ID != comment.PosterID && !ctx.Repo.IsAdmin()) { ctx.Status(http.StatusForbidden) return - } else if comment.Type != models.CommentTypeComment { + } + + if comment.Type != models.CommentTypeComment && comment.Type != models.CommentTypeReview && comment.Type != models.CommentTypeCode { ctx.Status(http.StatusNoContent) return } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index ee94e0e6d..9dee47753 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1333,7 +1333,7 @@ func ViewIssue(ctx *context.Context) { return } - if comment.Type == models.CommentTypeComment { + if comment.Type == models.CommentTypeComment || comment.Type == models.CommentTypeReview { if err := comment.LoadAttachments(); err != nil { ctx.ServerError("LoadAttachments", err) return @@ -2194,7 +2194,9 @@ func UpdateCommentContent(ctx *context.Context) { if !ctx.IsSigned || (ctx.User.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) { ctx.Error(http.StatusForbidden) return - } else if comment.Type != models.CommentTypeComment && comment.Type != models.CommentTypeCode { + } + + if comment.Type != models.CommentTypeComment && comment.Type != models.CommentTypeReview && comment.Type != models.CommentTypeCode { ctx.Error(http.StatusNoContent) return } @@ -2212,11 +2214,9 @@ func UpdateCommentContent(ctx *context.Context) { return } - if comment.Type == models.CommentTypeComment { - if err := comment.LoadAttachments(); err != nil { - ctx.ServerError("LoadAttachments", err) - return - } + if err := comment.LoadAttachments(); err != nil { + ctx.ServerError("LoadAttachments", err) + return } // when the update request doesn't intend to update attachments (eg: change checkbox state), ignore attachment updates @@ -2404,7 +2404,9 @@ func ChangeCommentReaction(ctx *context.Context) { ctx.Error(http.StatusForbidden) return - } else if comment.Type != models.CommentTypeComment && comment.Type != models.CommentTypeCode { + } + + if comment.Type != models.CommentTypeComment && comment.Type != models.CommentTypeCode && comment.Type != models.CommentTypeReview { ctx.Error(http.StatusNoContent) return } diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 3242a5b3e..73057248c 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -399,7 +399,7 @@ {{else if eq .Type 22}}
-
+
{{if .OriginalAuthor }} {{else}} @@ -434,23 +434,46 @@
{{if .Content}} -
+
- - {{if .OriginalAuthor }} - - {{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}} - {{ .OriginalAuthor }} - - {{if $.Repository.OriginalURL}} - ({{$.i18n.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe }}){{end}} - {{else}} - {{.Poster.GetDisplayName}} - {{end}} +
+ + {{if .OriginalAuthor }} + + {{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}} + {{ .OriginalAuthor }} + + {{if $.Repository.OriginalURL}} + ({{$.i18n.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe }}){{end}} + {{else}} + {{.Poster.GetDisplayName}} + {{end}} - {{$.i18n.Tr "repo.issues.review.left_comment" | Safe}} - + {{$.i18n.Tr "repo.issues.review.left_comment" | Safe}} + +
+
+ {{if (.ShowRole.HasRole "Poster")}} +
+ {{$.i18n.Tr "repo.issues.poster"}} +
+ {{end}} + {{if (.ShowRole.HasRole "Writer")}} +
+ {{$.i18n.Tr "repo.issues.collaborator"}} +
+ {{end}} + {{if (.ShowRole.HasRole "Owner")}} +
+ {{$.i18n.Tr "repo.issues.owner"}} +
+ {{end}} + {{if not $.Repository.IsArchived}} + {{template "repo/issue/view_content/add_reaction" Dict "ctx" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID)}} + {{template "repo/issue/view_content/context_menu" Dict "ctx" $ "item" . "delete" true "issue" true "diff" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} + {{end}} +
@@ -460,10 +483,18 @@ {{$.i18n.Tr "repo.issues.no_content"}} {{end}}
+
{{.Content}}
+
{{if .Attachments}} {{template "repo/issue/view_content/attachments" Dict "ctx" $ "Attachments" .Attachments "Content" .RenderedContent}} {{end}}
+ {{$reactions := .Reactions.GroupByType}} + {{if $reactions}} +
+ {{template "repo/issue/view_content/reactions" Dict "ctx" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} +
+ {{end}}
{{end}} diff --git a/web_src/js/features/repo-diff.js b/web_src/js/features/repo-diff.js index 3d937bbdb..f877af721 100644 --- a/web_src/js/features/repo-diff.js +++ b/web_src/js/features/repo-diff.js @@ -45,7 +45,7 @@ export function initRepoDiffConversationForm() { }); - $('.resolve-conversation').on('click', async function (e) { + $(document).on('click', '.resolve-conversation', async function (e) { e.preventDefault(); const comment_id = $(this).data('comment-id'); const origin = $(this).data('origin'); diff --git a/web_src/js/features/repo-issue-content.js b/web_src/js/features/repo-issue-content.js index 602523f89..40e88fb1f 100644 --- a/web_src/js/features/repo-issue-content.js +++ b/web_src/js/features/repo-issue-content.js @@ -109,7 +109,7 @@ export function initRepoIssueContentHistory() { if (!issueIndex) return; const $itemIssue = $('.repository.issue .timeline-item.comment.first'); // issue(PR) main content - const $comments = $('.repository.issue .comment-list .comment'); // includes: issue(PR) comments, code rerview comments + const $comments = $('.repository.issue .comment-list .comment'); // includes: issue(PR) comments, review comments, code comments if (!$itemIssue.length && !$comments.length) return; const repoLink = $('#repolink').val(); diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less index 5e7fdc420..75ccc8d25 100644 --- a/web_src/less/_repository.less +++ b/web_src/less/_repository.less @@ -2673,10 +2673,12 @@ a { color: var(--color-text); + text-decoration: none; } a:hover { color: var(--color-primary); + text-decoration: none; } }