Skip to content

Commit

Permalink
Fix PR comments UI (#18323)
Browse files Browse the repository at this point in the history
Closes:
* Review comment cannot be edited #17768
* Changing PR Comment Resolved State Disables Further Changes #18315
  • Loading branch information
wxiaoguang authored Jan 18, 2022
1 parent 11b4827 commit 84f8ef3
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 27 deletions.
2 changes: 1 addition & 1 deletion models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion routers/api/v1/repo/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
18 changes: 10 additions & 8 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
61 changes: 46 additions & 15 deletions templates/repo/issue/view_content/comments.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@
</div>
{{else if eq .Type 22}}
<div class="timeline-item-group">
<div class="timeline-item event" id="{{.HashTag}}">
<div class="timeline-item event">
{{if .OriginalAuthor }}
{{else}}
<a class="timeline-avatar"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>
Expand Down Expand Up @@ -434,23 +434,46 @@
</span>
</div>
{{if .Content}}
<div class="timeline-item comment">
<div class="timeline-item comment" id="{{.HashTag}}">
<div class="content comment-container">
<div class="ui top attached header comment-header df ac sb">
<span class="text grey">
{{if .OriginalAuthor }}
<span class="text black">
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
{{ .OriginalAuthor }}
</span>
<span class="text grey"> {{if $.Repository.OriginalURL}}</span>
<span class="text migrate">({{$.i18n.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe }}){{end}}</span>
{{else}}
<a class="author"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>{{.Poster.GetDisplayName}}</a>
{{end}}
<div class="comment-header-left df ac">
<span class="text grey">
{{if .OriginalAuthor }}
<span class="text black">
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
{{ .OriginalAuthor }}
</span>
<span class="text grey"> {{if $.Repository.OriginalURL}}</span>
<span class="text migrate">({{$.i18n.Tr "repo.migrated_from" ($.Repository.OriginalURL|Escape) ($.Repository.GetOriginalURLHostname|Escape) | Safe }}){{end}}</span>
{{else}}
<a class="author"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>{{.Poster.GetDisplayName}}</a>
{{end}}

{{$.i18n.Tr "repo.issues.review.left_comment" | Safe}}
</span>
{{$.i18n.Tr "repo.issues.review.left_comment" | Safe}}
</span>
</div>
<div class="comment-header-right actions df ac">
{{if (.ShowRole.HasRole "Poster")}}
<div class="ui basic label">
{{$.i18n.Tr "repo.issues.poster"}}
</div>
{{end}}
{{if (.ShowRole.HasRole "Writer")}}
<div class="ui basic label">
{{$.i18n.Tr "repo.issues.collaborator"}}
</div>
{{end}}
{{if (.ShowRole.HasRole "Owner")}}
<div class="ui basic label">
{{$.i18n.Tr "repo.issues.owner"}}
</div>
{{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}}
</div>
</div>
<div class="ui attached segment comment-body">
<div class="render-content markup">
Expand All @@ -460,10 +483,18 @@
<span class="no-content">{{$.i18n.Tr "repo.issues.no_content"}}</span>
{{end}}
</div>
<div id="comment-{{.ID}}" class="raw-content hide">{{.Content}}</div>
<div class="edit-content-zone hide" data-write="issuecomment-{{.ID}}-write" data-preview="issuecomment-{{.ID}}-preview" data-update-url="{{$.RepoLink}}/comments/{{.ID}}" data-context="{{$.RepoLink}}" data-attachment-url="{{$.RepoLink}}/comments/{{.ID}}/attachments"></div>
{{if .Attachments}}
{{template "repo/issue/view_content/attachments" Dict "ctx" $ "Attachments" .Attachments "Content" .RenderedContent}}
{{end}}
</div>
{{$reactions := .Reactions.GroupByType}}
{{if $reactions}}
<div class="ui attached segment reactions">
{{template "repo/issue/view_content/reactions" Dict "ctx" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}}
</div>
{{end}}
</div>
</div>
{{end}}
Expand Down
2 changes: 1 addition & 1 deletion web_src/js/features/repo-diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion web_src/js/features/repo-issue-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions web_src/less/_repository.less
Original file line number Diff line number Diff line change
Expand Up @@ -2673,10 +2673,12 @@

a {
color: var(--color-text);
text-decoration: none;
}

a:hover {
color: var(--color-primary);
text-decoration: none;
}
}

Expand Down

0 comments on commit 84f8ef3

Please sign in to comment.