From 29c00eb1ed13fbcd2a5e493d075c2bbd0e2a26f7 Mon Sep 17 00:00:00 2001
From: Gusted <williamzijl7@hotmail.com>
Date: Tue, 25 Oct 2022 11:11:49 +0000
Subject: [PATCH] Improve code comment review on mobile (#21461)

- Fix placement of avatar image, this was not placed in the
`comment-header-left` and add CSS to cover the limiting of width+height
of avatar for code-review comment on "Files changed" page. This fixes
the big noticeable avatar issue.
- Apply `margin-bottom` to the "next" button, so it's consistent with
the "previous" button.
- Make sure the "next"/"previous" start at `flex-start` on mobile and
not off-screen at `flex-end`. As well force them to have `flex: 1` so
they won't overflow on x-asis. This also requires the `width: 100%` for
the `.ui.buttons` div.
- Resolves #20074


### Before
<details><img width="512"
src="https://user-images.githubusercontent.com/25481501/195952930-09560cad-419f-43a3-a8a4-a4166c117994.jpg"></details>

### After
<details><img width="512"
src="https://user-images.githubusercontent.com/25481501/197340081-0365dfa8-4344-46b4-8702-a40c778c073f.jpg"></details>

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>
---
 templates/repo/diff/conversation.tmpl    |  2 +-
 templates/repo/diff/section_split.tmpl   |  8 +++---
 templates/repo/diff/section_unified.tmpl |  2 +-
 web_src/js/features/repo-issue.js        |  6 ++--
 web_src/less/_repository.less            |  6 +++-
 web_src/less/_review.less                | 36 +++++++++++++++++++-----
 web_src/less/helpers.less                |  1 +
 7 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/templates/repo/diff/conversation.tmpl b/templates/repo/diff/conversation.tmpl
index 781e1f681..204a2146f 100644
--- a/templates/repo/diff/conversation.tmpl
+++ b/templates/repo/diff/conversation.tmpl
@@ -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"}}
diff --git a/templates/repo/diff/section_split.tmpl b/templates/repo/diff/section_split.tmpl
index 6143a4715..24302e536 100644
--- a/templates/repo/diff/section_split.tmpl
+++ b/templates/repo/diff/section_split.tmpl
@@ -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}}
@@ -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}}
@@ -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}}
@@ -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}}
diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl
index 373747002..e83b1f779 100644
--- a/templates/repo/diff/section_unified.tmpl
+++ b/templates/repo/diff/section_unified.tmpl
@@ -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>
diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js
index 9dbe78edf..54cdeb383 100644
--- a/web_src/js/features/repo-issue.js
+++ b/web_src/js/features/repo-issue.js
@@ -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);
diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less
index bd007196d..039c47130 100644
--- a/web_src/less/_repository.less
+++ b/web_src/less/_repository.less
@@ -1126,7 +1126,10 @@
         .comment-content {
           margin-left: 36px;
         }
+      }
 
+      .code-comment,
+      .comment {
         img.avatar {
           width: 28px;
           height: 28px;
@@ -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);
 }
 
diff --git a/web_src/less/_review.less b/web_src/less/_review.less
index 013d6d5aa..4dd232945 100644
--- a/web_src/less/_review.less
+++ b/web_src/less/_review.less
@@ -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);
@@ -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 {
@@ -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;
@@ -90,6 +107,10 @@
         float: none;
         margin: 0 .5rem 0 0;
         flex-shrink: 0;
+
+        ~ .content {
+          margin-left: 1em;
+        }
       }
 
       img.avatar {
@@ -100,6 +121,10 @@
         margin-left: 0 !important;
       }
 
+      .comment-container {
+        width: 100%;
+      }
+
       &.code-comment {
         padding: 0 0 .5rem !important;
       }
@@ -166,10 +191,7 @@
     .button {
       width: 100%;
       margin: 0 !important;
-
-      &:not(:last-child) {
-        margin-bottom: .75rem !important;
-      }
+      margin-bottom: .75rem !important;
     }
   }
 }
diff --git a/web_src/less/helpers.less b/web_src/less/helpers.less
index 19477f7ec..f47569392 100644
--- a/web_src/less/helpers.less
+++ b/web_src/less/helpers.less
@@ -175,4 +175,5 @@
 @media @mediaSm {
   .db-small { display: block !important; }
   .w-100-small { width: 100% !important; }
+  .js-small { justify-content: flex-start !important; }
 }