Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review fixes and enhancements #24526

Merged
merged 22 commits into from
May 10, 2023
Merged

Review fixes and enhancements #24526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
268f3ed
Review fixes and enhancements
silverwind May 4, 2023
7c7aa59
add avatars to review and review dismiss events
silverwind May 4, 2023
5438a5b
add missing inline avatars, fix avatar offset, icon size tweaks
silverwind May 4, 2023
043b6f8
Update web_src/css/repository.css
silverwind May 4, 2023
7686722
move to .Review.HTMLTypeColorName
silverwind May 8, 2023
e4c8f4a
Merge branch 'main' into fix-margin
silverwind May 8, 2023
293d98c
misc tweaks, decrease review list avatar size
silverwind May 8, 2023
9a72ff8
make fmt
silverwind May 8, 2023
ebbfa0e
final avatar fixes for review timeline
silverwind May 8, 2023
396b9d1
restore inline avatar
silverwind May 8, 2023
ccaa8b0
Merge branch 'main' into fix-margin
silverwind May 9, 2023
a963dd2
use constants
silverwind May 9, 2023
dbea7c3
add comment
silverwind May 9, 2023
38ee4da
use 16px everywhere except dismiss
silverwind May 9, 2023
dde032c
remove empty
silverwind May 9, 2023
ffa0443
Merge branch 'main' into fix-margin
GiteaBot May 10, 2023
d86e371
Merge branch 'main' into fix-margin
GiteaBot May 10, 2023
26d2a13
fix lint
silverwind May 10, 2023
b6cea6c
Merge branch 'main' into fix-margin
GiteaBot May 10, 2023
bcfc9b3
Merge branch 'main' into fix-margin
GiteaBot May 10, 2023
0724613
Merge branch 'main' into fix-margin
GiteaBot May 10, 2023
0cb87a8
Merge branch 'main' into fix-margin
GiteaBot May 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions models/issues/review.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,20 @@ func (r *Review) LoadAttributes(ctx context.Context) (err error) {
return err
}

func (r *Review) HTMLTypeColorName() string {
switch r.Type {
case ReviewTypeApprove:
return "green"
case ReviewTypeComment:
return "grey"
case ReviewTypeReject:
return "red"
case ReviewTypeRequest:
return "yellow"
}
return "grey"
}

// GetReviewByID returns the review by the given ID
func GetReviewByID(ctx context.Context, id int64) (*Review, error) {
review := new(Review)
Expand Down
18 changes: 16 additions & 2 deletions templates/repo/issue/view_content/comments.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,10 @@
<div class="timeline-item event">
{{if .OriginalAuthor}}
{{else}}
<a class="timeline-avatar"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>
{{/* Some timeline avatars need a offset to correctly allign with their speech
bubble. The condition depends on review type and for positive reviews whether
there is a comment element or not */}}
<a class="timeline-avatar{{if or (and (eq .Review.Type 1) (or .Content .Attachments)) (eq .Review.Type 2) (eq .Review.Type 3)}} timeline-avatar-offset{{end}}"{{if gt .Poster.ID 0}} href="{{.Poster.HomeLink}}"{{end}}>
silverwind marked this conversation as resolved.
Show resolved Hide resolved
{{avatar $.Context .Poster}}
</a>
{{end}}
Expand Down Expand Up @@ -408,6 +411,11 @@
<div class="content comment-container">
<div class="ui top attached header comment-header gt-df gt-ac gt-sb">
<div class="comment-header-left gt-df gt-ac">
{{if gt .Poster.ID 0}}
<a class="inline-timeline-avatar" href="{{.Poster.HomeLink}}">
{{avatar $.Context .Poster}}
</a>
{{end}}
<span class="text grey muted-links">
{{if .OriginalAuthor}}
<span class="text black gt-bold">
Expand Down Expand Up @@ -763,6 +771,7 @@
<img src="{{.Poster.AvatarLink $.Context}}">
</a>
<span class="badge grey">{{svg "octicon-x" 16}}</span>
{{template "shared/user/avatarlink" dict "Context" $.Context "user" .Poster}}
<span class="text grey muted-links">
{{template "shared/user/authorlink" .Poster}}
{{$reviewerName := ""}}
Expand All @@ -777,7 +786,12 @@
{{if .Content}}
<div class="timeline-item comment">
<div class="content">
<div class="ui top attached header arrow-top">
<div class="ui top attached header comment-header-left gt-df gt-ac arrow-top">
{{if gt .Poster.ID 0}}
<a class="inline-timeline-avatar" href="{{.Poster.HomeLink}}">
{{avatar $.Context .Poster}}
</a>
{{end}}
<span class="text grey muted-links">
{{$.locale.Tr "action.review_dismissed_reason"}}
</span>
Expand Down
51 changes: 19 additions & 32 deletions templates/repo/issue/view_content/pull.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{{if or .PullReviewers .OriginalReviews}}
<div class="comment box">
<div class="comment box timeline-item gt-pt-3 gt-pb-0">
<div class="content">
<div class="ui segment">
<h4>{{$.locale.Tr "repo.issues.review.reviewers"}}</h4>
Expand All @@ -8,9 +8,12 @@
<div class="ui divider"></div>
<div class="review-item">
<div class="review-item-left">
<span class="gt-mr-3 text {{.Review.HTMLTypeColorName}}">
{{svg (printf "octicon-%s" .Review.Type.Icon)}}
</span>
{{if .User}}
<a href="{{.User.HomeLink}}">
{{avatar $.Context .User}}
{{avatar $.Context .User 20}}
</a>
{{end}}
<span class="gt-ml-2">
Expand All @@ -34,13 +37,13 @@
</div>
<div class="review-item-right">
{{if .Review.Stale}}
<span class="ui type-icon text grey" data-tooltip-content="{{$.locale.Tr "repo.issues.is_stale"}}">
<span class="ui text grey" data-tooltip-content="{{$.locale.Tr "repo.issues.is_stale"}}">
{{svg "octicon-hourglass" 16 "icon"}}
</span>
{{end}}
{{if (and $.Permission.IsAdmin (or (eq .Review.Type 1) (eq .Review.Type 3)) (not $.Issue.IsClosed))}}
<a href="#" class="ui muted icon dismiss-review-btn" data-review-id="dismiss-review-{{.Review.ID}}" data-tooltip-content="{{$.locale.Tr "repo.issues.dismiss_review"}}">
{{svg "octicon-x" 16}}
<a href="#" class="ui muted icon gt-df gt-ac dismiss-review-btn" data-review-id="dismiss-review-{{.Review.ID}}" data-tooltip-content="{{$.locale.Tr "repo.issues.dismiss_review"}}">
{{svg "octicon-x" 20}}
</a>
<div class="ui small modal" id="dismiss-review-modal">
<div class="header">
Expand All @@ -65,19 +68,9 @@
</div>
</div>
{{end}}
<span class="type-icon text {{if eq .Review.Type 1}}green
{{- else if eq .Review.Type 2}}grey
{{- else if eq .Review.Type 3}}red
{{- else if eq .Review.Type 4}}yellow
{{else}}grey{{end}}">

{{if .CanChange}}
<a href="#" class="ui icon re-request-review {{if .Checked}}checked{{end}}" data-issue-id="{{$.Issue.ID}}" data-tooltip-content="{{if .Checked}} {{$.locale.Tr "repo.issues.remove_request_review"}} {{else}} {{$.locale.Tr "repo.issues.re_request_review"}} {{end}}" data-id="{{.ItemID}}" data-update-url="{{$.RepoLink}}/issues/request_review">
{{if .Checked}} {{svg "octicon-trash"}} {{else}} {{svg "octicon-sync"}} {{end}}
</a>
{{end}}
{{svg (printf "octicon-%s" .Review.Type.Icon)}}
</span>
{{if .CanChange}}
<a href="#" class="ui icon muted gt-df gt-ac re-request-review{{if .Checked}} checked{{end}}" data-issue-id="{{$.Issue.ID}}" data-tooltip-content="{{if .Checked}}{{$.locale.Tr "repo.issues.remove_request_review"}}{{else}}{{$.locale.Tr "repo.issues.re_request_review"}}{{end}}" data-id="{{.ItemID}}" data-update-url="{{$.RepoLink}}/issues/request_review">{{if .Checked}}{{svg "octicon-trash"}}{{else}}{{svg "octicon-sync"}}{{end}}</a>
{{end}}
</div>
</div>
{{end}}
Expand All @@ -86,22 +79,16 @@
<div class="ui divider"></div>
<div class="review-item">
<div class="review-item-left">
<span class="gt-mr-3 text {{.Review.HTMLTypeColorName}}">
{{svg (printf "octicon-%s" .Type.Icon)}}
</span>
<a href="{{$.Repository.OriginalURL}}" data-tooltip-content="{{$.locale.Tr "repo.migrated_from_fake" ($.Repository.GetOriginalURLHostname|Escape) | Safe}}">
<span class="text black ">
<span class="text black">
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
{{.OriginalAuthor}}
</span>
</a>
</div>
<div class="review-item-right">
<span class="type-icon text {{if eq .Type 1}}green
{{- else if eq .Type 2}}grey
{{- else if eq .Type 3}}red
{{- else if eq .Type 4}}yellow
{{else}}grey{{end}}">
{{svg (printf "octicon-%s" .Type.Icon)}}
</span>
</div>
</div>
{{end}}
</div>
Expand Down Expand Up @@ -204,7 +191,7 @@
</div>
{{else if .Issue.PullRequest.IsAncestor}}
<div class="item">
<i class="icon icon-octicon">{{svg "octicon-alert" 16}}</i>
<i class="icon icon-octicon">{{svg "octicon-alert"}}</i>
{{$.locale.Tr "repo.pulls.is_ancestor"}}
</div>
{{else if or .Issue.PullRequest.CanAutoMerge .Issue.PullRequest.IsEmpty}}
Expand All @@ -230,7 +217,7 @@
</div>
{{else if .IsBlockedByChangedProtectedFiles}}
<div class="item">
<i class="icon icon-octicon">{{svg "octicon-x" 16}}</i>
<i class="icon icon-octicon">{{svg "octicon-x"}}</i>
{{$.locale.TrN $.ChangedProtectedFilesNum "repo.pulls.blocked_by_changed_protected_files_1" "repo.pulls.blocked_by_changed_protected_files_n" | Safe}}
<ul>
{{range .ChangedProtectedFiles}}
Expand Down Expand Up @@ -294,7 +281,7 @@
<div class="ui divider"></div>

<div class="item">
<i class="icon icon-octicon">{{svg "octicon-alert" 16}}</i>
<i class="icon icon-octicon">{{svg "octicon-alert"}}</i>
{{$.locale.Tr "repo.pulls.is_empty"}}
</div>
{{end}}
Expand Down Expand Up @@ -430,7 +417,7 @@
</div>
{{else if .IsBlockedByChangedProtectedFiles}}
<div class="item text red">
<i class="icon icon-octicon">{{svg "octicon-x" 16}}</i>
<i class="icon icon-octicon">{{svg "octicon-x"}}</i>
{{$.locale.TrN $.ChangedProtectedFilesNum "repo.pulls.blocked_by_changed_protected_files_1" "repo.pulls.blocked_by_changed_protected_files_n" | Safe}}
<ul>
{{range .ChangedProtectedFiles}}
Expand Down
22 changes: 6 additions & 16 deletions templates/repo/issue/view_content/sidebar.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,37 +54,27 @@
{{range .PullReviewers}}
<div class="item gt-mb-2">
{{if .User}}
<a class="muted sidebar-item-link" href="{{.User.HomeLink}}">
{{avatar $.Context .User 28 "gt-mr-3"}}
{{.User.GetDisplayName}}
</a>
<a class="muted sidebar-item-link" href="{{.User.HomeLink}}">{{avatar $.Context .User 28 "gt-mr-3"}}{{.User.GetDisplayName}}</a>
{{else if .Team}}
<span class="text">{{svg "octicon-people" 16 "teamavatar"}}{{$.Issue.Repo.OwnerName}}/{{.Team.Name}}</span>
{{end}}
<span class="ui right type-icon text {{if eq .Review.Type 1}}green
{{- else if eq .Review.Type 2}}grey
{{- else if eq .Review.Type 3}}red
{{- else if eq .Review.Type 4}}yellow
{{- else}}grey{{end}} right ">

<span class="ui right gt-df gt-ac gt-gap-3">
{{if .CanChange}}
<a href="#" class="ui icon re-request-review {{if .Checked}}checked{{end}}" data-tooltip-content="{{if .Checked}} {{$.locale.Tr "repo.issues.remove_request_review"}} {{else}} {{$.locale.Tr "repo.issues.re_request_review"}} {{end}}" data-issue-id="{{$.Issue.ID}}" data-id="{{.ItemID}}" data-update-url="{{$.RepoLink}}/issues/request_review">
{{if .Checked}} {{svg "octicon-trash"}} {{else}} {{svg "octicon-sync"}} {{end}}
</a>
<a href="#" class="ui muted icon re-request-review{{if .Checked}} checked{{end}}" data-tooltip-content="{{if .Checked}}{{$.locale.Tr "repo.issues.remove_request_review"}}{{else}}{{$.locale.Tr "repo.issues.re_request_review"}}{{end}}" data-issue-id="{{$.Issue.ID}}" data-id="{{.ItemID}}" data-update-url="{{$.RepoLink}}/issues/request_review">{{if .Checked}}{{svg "octicon-trash"}}{{else}}{{svg "octicon-sync"}}{{end}}</a>
{{end}}
{{svg (printf "octicon-%s" .Review.Type.Icon)}}
{{svg (printf "octicon-%s" .Review.Type.Icon) 16 (printf "text %s" (.Review.HTMLTypeColorName))}}
</span>
</div>
{{end}}
{{range .OriginalReviews}}
<div class="item" style="margin-bottom: 10px;">
<div class="item gt-mb-3">
<a href="{{$.Repository.OriginalURL}}" data-tooltip-content="{{$.locale.Tr "repo.migrated_from_fake" ($.Repository.GetOriginalURLHostname|Escape) | Safe}}">
<span class="text black">
{{svg (MigrationIcon $.Repository.GetOriginalURLHostname)}}
{{.OriginalAuthor}}
</span>
</a>
<span class="ui right type-icon text {{if eq .Type 1}}green
<span class="ui right text {{if eq .Type 1}}green
{{- else if eq .Type 2}}grey
{{- else if eq .Type 3}}red
{{- else if eq .Type 4}}yellow
Expand Down
40 changes: 19 additions & 21 deletions web_src/css/repository.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,49 +691,41 @@
margin-right: 5px;
}

.repository.view.issue .pull .merge.box .timeline-avatar {
.repository.view.issue .merge.box .timeline-avatar {
margin-top: 3px;
margin-left: 4px;
}

.repository.view.issue .pull .merge.box .branch-update.grid .row {
.repository.view.issue .merge.box .branch-update.grid .row {
padding-bottom: 1rem;
}

.repository.view.issue .pull .merge.box .branch-update.grid .row .icon {
.repository.view.issue .merge.box .branch-update.grid .row .icon {
margin-top: 1.1rem;
}

.repository.view.issue .pull .review-item {
.repository.view.issue .review-item {
display: flex;
justify-content: space-between;
align-items: center;
}

.repository.view.issue .pull .review-item .review-item-left,
.repository.view.issue .pull .review-item .review-item-right {
.review-item-left,
.review-item-right {
display: flex;
align-items: center;
}

.repository.view.issue .pull .review-item .text {
margin: 0.3em 0 0.5em 0.5em;
}

.repository.view.issue .pull .review-item .type-icon {
align-self: flex-start;
margin-right: 1em;
}

.repository.view.issue .pull .review-item .type-icon i {
line-height: 1.8em;
.review-item-right {
gap: 8px;
margin-left: 4px;
}

.repository.view.issue .pull .review-item .divider {
.repository.view.issue .review-item .divider {
margin: 0.5rem 0;
}

.repository.view.issue .pull .review-item .review-content {
.repository.view.issue .review-item .review-content {
padding: 1em 0 1em 3.8em;
}

Expand Down Expand Up @@ -782,6 +774,10 @@
padding-bottom: 8px;
}

.repository.view.issue .comment-list .timeline-avatar-offset {
margin-top: 48px;
}

.repository.view.issue .comment-list .timeline-item {
margin-left: 16px;
position: relative;
Expand Down Expand Up @@ -1098,8 +1094,8 @@
margin-left: 3em;
}

.repository.view.issue .comment-list .code-comment img.avatar,
.repository.view.issue .comment-list .comment img.avatar {
.repository.view.issue .comment-code-cloud .comment-list .code-comment img.avatar,
.repository.view.issue .comment-code-cloud .comment-list .comment img.avatar {
width: 28px;
height: 28px;
}
Expand Down Expand Up @@ -3261,6 +3257,8 @@ td.blob-excerpt {
.repository.view.issue .comment-list .timeline .inline-timeline-avatar {
display: flex;
margin-bottom: auto;
margin-left: 6px;
margin-right: 2px;
}
.repository.view.issue .comment-list .timeline .inline-timeline-avatar img.avatar {
height: 24px;
Expand Down