Skip to content

Improve code comment review on mobile #21461

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

Merged
merged 13 commits into from
Oct 25, 2022
Merged

Improve code comment review on mobile #21461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6d90d8f
Improve code comment review on mobile
Oct 14, 2022
cc037ac
Merge branch 'main' into improve-code-review-comment
Oct 15, 2022
d489ecc
Merge branch 'main' into improve-code-review-comment
lunny Oct 17, 2022
7cfa8bf
Merge branch 'main' into improve-code-review-comment
Oct 18, 2022
9b247fa
Merge branch 'main' into improve-code-review-comment
Oct 22, 2022
af142d3
Keep "speech bubble" effect
Oct 22, 2022
81e710d
Merge branch 'main' into improve-code-review-comment
Oct 23, 2022
d18a8ee
Merge branch 'main' into improve-code-review-comment
Oct 24, 2022
938de62
margin and padding tweaks
silverwind Oct 24, 2022
912a196
use colspan and display: none to hide tds
silverwind Oct 24, 2022
9a8240d
Merge branch 'main' into improve-code-review-comment
lunny Oct 24, 2022
be916b9
Merge branch 'main' into improve-code-review-comment
lunny Oct 25, 2022
d6f1026
Merge branch 'main' into improve-code-review-comment
lunny Oct 25, 2022
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
2 changes: 1 addition & 1 deletion templates/repo/diff/conversation.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
{{template "repo/diff/comments" dict "root" $ "comments" .comments}}
</ui>
</div>
<div class="df je ac fw mt-3">
<div class="df je js-small ac fw mt-3">
<div class="ui buttons mr-2">
<button class="ui icon tiny basic button previous-conversation">
{{svg "octicon-arrow-up" 12 "icon"}} {{$.locale.Tr "repo.issues.previous"}}
Expand Down
8 changes: 4 additions & 4 deletions templates/repo/diff/section_split.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
<td class="lines-num"></td>
<td class="lines-escape"></td>
<td class="lines-type-marker"></td>
<td class="add-comment-left">
<td class="add-comment-left" colspan="4">
{{if gt (len $line.Comments) 0}}
{{if eq $line.GetCommentSide "previous"}}
{{template "repo/diff/conversation" mergeinto $.root "comments" $line.Comments}}
Expand All @@ -116,7 +116,7 @@
<td class="lines-num"></td>
<td class="lines-escape"></td>
<td class="lines-type-marker"></td>
<td class="add-comment-right">
<td class="add-comment-right" colspan="4">
{{if eq $line.GetCommentSide "proposed"}}
{{template "repo/diff/conversation" mergeinto $.root "comments" $line.Comments}}
{{end}}
Expand All @@ -133,7 +133,7 @@
<td class="lines-num"></td>
<td class="lines-escape"></td>
<td class="lines-type-marker"></td>
<td class="add-comment-left">
<td class="add-comment-left" colspan="4">
{{if gt (len $line.Comments) 0}}
{{if eq $line.GetCommentSide "previous"}}
{{template "repo/diff/conversation" mergeinto $.root "comments" $line.Comments}}
Expand All @@ -143,7 +143,7 @@
<td class="lines-num"></td>
<td class="lines-escape"></td>
<td class="lines-type-marker"></td>
<td class="add-comment-right">
<td class="add-comment-right" colspan="4">
{{if eq $line.GetCommentSide "proposed"}}
{{template "repo/diff/conversation" mergeinto $.root "comments" $line.Comments}}
{{end}}
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/diff/section_unified.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
{{if gt (len $line.Comments) 0}}
<tr class="add-comment" data-line-type="{{DiffLineTypeToStr .GetType}}">
<td colspan="3" class="lines-num"></td>
<td class="add-comment-left add-comment-right" colspan="2">
<td class="add-comment-left add-comment-right" colspan="5">
{{template "repo/diff/conversation" mergeinto $.root "comments" $line.Comments}}
</td>
</tr>
Expand Down
6 changes: 3 additions & 3 deletions web_src/js/features/repo-issue.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,16 +513,16 @@ export function initRepoPullRequestReview() {
<td class="lines-num"></td>
<td class="lines-escape"></td>
<td class="lines-type-marker"></td>
<td class="add-comment-left"></td>
<td class="add-comment-left" colspan="4"></td>
<td class="lines-num"></td>
<td class="lines-escape"></td>
<td class="lines-type-marker"></td>
<td class="add-comment-right"></td>
<td class="add-comment-right" colspan="4"></td>
` : `
<td class="lines-num"></td>
<td class="lines-num"></td>
<td class="lines-escape"></td>
<td class="add-comment-left add-comment-right" colspan="2"></td>
<td class="add-comment-left add-comment-right" colspan="5"></td>
`}
</tr>`);
tr.after(ntr);
Expand Down
6 changes: 5 additions & 1 deletion web_src/less/_repository.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,10 @@
.comment-content {
margin-left: 36px;
}
}

.code-comment,
.comment {
img.avatar {
width: 28px;
height: 28px;
Expand Down Expand Up @@ -3266,7 +3269,8 @@ td.blob-excerpt {
background: var(--color-diff-inactive);
}

.code-diff-split tbody tr td:nth-child(5) {
.code-diff-split tbody tr td:nth-child(5),
.code-diff-split tbody tr td.add-comment-right {
border-left: 1px solid var(--color-secondary);
}

Expand Down
36 changes: 29 additions & 7 deletions web_src/less/_review.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@
}
}

.add-comment .lines-num,
.add-comment .lines-escape,
.add-comment .lines-type-marker {
display: none;
}

.show-outdated,
.hide-outdated {
&:extend(.unselectable);
Expand All @@ -59,11 +65,13 @@
}

.comment-code-cloud {
padding: .5rem !important;
padding: .5rem 1rem !important;
position: relative;
margin: 0 auto;
max-width: 1000px;

@media @mediaSm {
max-width: auto;
padding: .75rem !important;

.code-comment-buttons {
Expand All @@ -73,13 +81,22 @@
width: 100%;
}
}

.ui.buttons {
width: 100%;
margin: 0 !important;

.button {
flex: 1;
}
}
}

.comments .comment {
margin: 0;
padding: 0;

@media @mediaSm {
padding: 0;
display: flex;

.comment-header-right.actions .ui.basic.label {
display: none;
Expand All @@ -90,6 +107,10 @@
float: none;
margin: 0 .5rem 0 0;
flex-shrink: 0;

~ .content {
margin-left: 1em;
}
}

img.avatar {
Expand All @@ -100,6 +121,10 @@
margin-left: 0 !important;
}

.comment-container {
width: 100%;
}

&.code-comment {
padding: 0 0 .5rem !important;
}
Expand Down Expand Up @@ -166,10 +191,7 @@
.button {
width: 100%;
margin: 0 !important;

&:not(:last-child) {
margin-bottom: .75rem !important;
}
margin-bottom: .75rem !important;
}
}
}
Expand Down
1 change: 1 addition & 0 deletions web_src/less/helpers.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,5 @@
@media @mediaSm {
.db-small { display: block !important; }
.w-100-small { width: 100% !important; }
.js-small { justify-content: flex-start !important; }
}