Diff and code review refactors and improvements (#13922)

* Diff CSS refactors and misc tweaks

- Simplify Diff CSS styling
- Add color variables for diff
- Fix vertical centering of inline comment button
- Slightly adjust text colors, e.g. in comment header

* Code review improvments

* selector tweak

* fix diff issues, add inactive bg color

Co-authored-by: 6543 <6543@obermui.de>

diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl
index 2c413b3c46d9de692e9be48db3bfca22515460b2..bc5d7dde12406bc467d005424cbbc420e2d8c32a 100644 (file)
--- a/templates/repo/issue/view_content/comments.tmpl
+++ b/templates/repo/issue/view_content/comments.tmpl
@@ -398,10 +398,7 @@
                                        {{avatar .Poster}}
                                </a>
                                {{end}}
-                               <span class="badge {{if eq .Review.Type 1}}bg-green
-                               {{- else if eq .Review.Type 2}}bg-grey
-                               {{- else if eq .Review.Type 3}}bg-red
-                               {{- else}}bg-grey{{end}} text-white">{{svg (printf "octicon-%s" .Review.Type.Icon)}}</span>
+                               <span class="badge{{if eq .Review.Type 1}} bg-green text-white{{else if eq .Review.Type 3}} bg-red text-white{{end}}">{{svg (printf "octicon-%s" .Review.Type.Icon)}}</span>
                                <span class="text grey">
                                        {{if .OriginalAuthor }}
                                                <span class="text black"><i class="fa {{MigrationIcon $.Repository.GetOriginalURLHostname}}" aria-hidden="true"></i> {{ .OriginalAuthor }}</span><span class="text grey"> {{if $.Repository.OriginalURL}}</span><span class="text migrate">({{$.i18n.Tr "repo.migrated_from" $.Repository.OriginalURL $.Repository.GetOriginalURLHostname | Safe }}){{end}}</span>
@@ -452,7 +449,7 @@
                                {{ range $filename, $lines := .Review.CodeComments}}
                                        {{range $line, $comms := $lines}}
                                                        <div class="ui segments">
-                                                               <div class="ui segment">
+                                                               <div class="ui segment py-3">
                                                                        {{$invalid := (index $comms 0).Invalidated}}
                                                                        {{$resolved := (index $comms 0).IsResolved}}
                                                                        {{$resolveDoer := (index $comms 0).ResolveDoer}}
@@ -497,8 +494,8 @@
                                                                                </div>
                                                                        </div>
                                                                {{end}}
-                                                               <div id="code-comments-{{(index $comms 0).ID}}" class="ui segment{{if $resolved}} hide{{end}}">
-                                                                       <div class="ui comments">
+                                                               <div id="code-comments-{{(index $comms 0).ID}}" class="ui segment{{if $resolved}} hide{{end}} py-3">
+                                                                       <div class="ui comments mb-0">
                                                                                {{range $comms}}
                                                                                        {{ $createdSubStr:= TimeSinceUnix .CreatedUnix $.Lang }}
                                                                                        <div class="comment code-comment" id="{{.HashTag}}">

diff --git a/web_src/less/_base.less b/web_src/less/_base.less
index 45cd549320d40b35e0e34d40b1bca2001ca18c96..9a7aa28d4b7fe7650efca5535ea2e5f866f78953 100644 (file)
--- a/web_src/less/_base.less
+++ b/web_src/less/_base.less
@@ -73,8 +73,16 @@
   --color-black: #1b1c1d;
   --color-gold: #a1882b;
   --color-white: #ffffff;
+  --color-diff-removed-word-bg: #fdb8c0;
+  --color-diff-added-word-bg: #acf2bd;
+  --color-diff-removed-row-bg: #ffeef0;
+  --color-diff-added-row-bg: #e6ffed;
+  --color-diff-removed-row-border: #f1c0c0;
+  --color-diff-added-row-border: #e6ffed;
+  --color-diff-inactive: #f2f2f2;
   /* target-based colors */
   --color-body: #ffffff;
+  --color-text-dark: #080808;
   --color-text: #212121;
   --color-text-light: #555555;
   --color-text-light-2: #888888;
@@ -267,6 +275,10 @@ a.muted:hover,
   color: var(--color-text);
 }
 
+.ui.menu .item > .label {
+  background: var(--color-grey);
+}
+
 .ui.link.menu .item:hover,
 .ui.menu .dropdown.item:hover,
 .ui.menu .link.item:hover,
@@ -436,6 +448,28 @@ a.ui.card:hover,
   background: var(--color-card);
 }
 
+.ui.comments .comment .text,
+.ui.comments .comment .author {
+  color: var(--color-text);
+}
+
+.ui.comments .comment a.author:hover {
+  color: var(--color-primary);
+}
+
+.ui.comments .comment .metadata {
+  color: var(--color-text-light-2);
+}
+
+.ui.comments .comment .actions a {
+  color: var(--color-text-light);
+}
+
+.ui.comments .comment .actions a.active,
+.ui.comments .comment .actions a:hover {
+  color: var(--color-primary);
+}
+
 .ui.progress[data-percent="0"] .bar .progress {
   color: var(--color-text);
 }
@@ -714,7 +748,7 @@ a.ui.card:hover,
       color: var(--color-text);
 
       &:hover {
-        color: #000000;
+        color: var(--color-text-dark);
       }
     }
 
@@ -722,7 +756,7 @@ a.ui.card:hover,
       color: var(--color-text-light) !important;
 
       a {
-        color: inherit !important;
+        color: var(--color-text) !important;
 
         &:hover {
           color: var(--color-primary) !important;
@@ -731,7 +765,7 @@ a.ui.card:hover,
     }
 
     &.light.grey {
-      color: var(--color-grey) !important;
+      color: var(--color-text-light-2) !important;
     }
 
     &.green {
@@ -1691,6 +1725,10 @@ a.ui.basic.label:hover {
   border-color: var(--color-secondary);
 }
 
+.ui.segments > .segment {
+  border-color: var(--color-secondary);
+}
+
 .ui.secondary.segment {
   background: var(--color-secondary-bg);
   color: var(--color-text-light);
@@ -1863,7 +1901,7 @@ table th[data-sortt-desc] {
 
 .ui.tabular.menu .item {
   padding: 11px 12px;
-  color: var(--color-secondary-dark-6);
+  color: var(--color-text-light-2);
 }
 
 .ui.tabular.menu .item:hover {
@@ -1878,12 +1916,17 @@ table th[data-sortt-desc] {
   margin-top: 1px; /* offset fomantic's margin-bottom: -1px */
 }
 
+.ui.segment .ui.tabular.menu .active.item,
+.ui.segment .ui.tabular.menu .active.item:hover {
+  background: var(--color-box-body);
+}
+
 .ui.secondary.pointing.menu {
   border-color: var(--color-secondary);
 }
 
 .ui.secondary.pointing.menu .item {
-  color: var(--color-secondary-dark-6);
+  color: var(--color-text-light-2);
 }
 
 .ui.secondary.pointing.menu .active.item,

diff --git a/web_src/less/_repository.less b/web_src/less/_repository.less
index 7cac37207ecc33e47ebc55635e3e8902a1e3f048..fe1c258810f2a6250e49102e0e17e907fbd56860 100644 (file)
--- a/web_src/less/_repository.less
+++ b/web_src/less/_repository.less
@@ -835,7 +835,7 @@
           float: left;
           margin-left: -33px;
           margin-right: 8px;
-          color: var(--color-text-light-2);
+          color: var(--color-text);
           align-items: center;
           justify-content: center;
 
@@ -1016,7 +1016,7 @@
           }
 
           .no-content {
-            color: #767676;
+            color: var(--color-text-light-2);
             font-style: italic;
           }
 
@@ -1054,55 +1054,22 @@
 
       .code-comment {
         border: 1px solid transparent;
-        padding: 6px 6px 3px;
+        padding: .5rem 0;
+        margin: 0;
 
         .content {
           border: none !important;
         }
+
+        .avatar.image {
+          width: 28px;
+          height: 28px;
+        }
       }
 
       .event {
         padding-left: 15px;
 
-        & > .svg:not(.issue-symbol) {
-          text-shadow: -2px 0 #fff, 0 2px #fff, 2px 0 #fff, 0 -2px #fff;
-        }
-
-        & > .svg.issue-symbol {
-          font-size: 20px;
-          margin-left: -35px;
-          margin-right: -1px;
-          margin-top: 0 !important;
-          height: 28px;
-          width: 28px;
-          border-radius: 50%;
-          text-align: center;
-          line-height: 28px;
-          background: #eee;
-
-          &::before {
-            width: 15px;
-            display: inline-block;
-          }
-
-          &.octicon-key::before {
-            width: 18px;
-          }
-
-          &.octicon-circle-slash::before {
-            width: 17px;
-          }
-
-          &.octicon-comment {
-            font-size: 21px;
-            line-height: 33px;
-
-            &::before {
-              width: 20px;
-            }
-          }
-        }
-
         .detail {
           font-size: .9rem;
           margin-top: 5px;
@@ -1533,7 +1500,6 @@
     }
 
     .button {
-      margin: -5px 0 -5px 12px;
       padding: 8px 10px;
       flex: 0 0 auto;
     }
@@ -1545,9 +1511,11 @@
     }
 
     .file-body.file-code {
+      background: var(--color-code-bg);
+
       .lines-num {
         text-align: right;
-        color: #a6a6a6;
+        color: var(--color-text-light);
         width: 1%;
         min-width: 50px;
         user-select: none;
@@ -1607,74 +1575,11 @@
       }
     }
 
-    .code-diff-unified tbody tr {
-      &.del-code td {
-        background-color: #ffeef0;
-        border-color: #f1c0c0;
-      }
-
-      &.add-code td {
-        background-color: #e6ffed;
-        border-color: #bef5cb;
-      }
-
-      &.del-code td.lines-num {
-        background-color: #ffe5e4;
-      }
-
-      &.add-code td.lines-num {
-        background-color: #cdffd8;
-      }
-
-    }
-
     .code-diff-split {
-
       table,
       tbody {
         width: 100%;
       }
-
-      tbody tr {
-
-        // light gray for empty lines before / after commit
-        &.add-code td:nth-child(1),
-        &.add-code td:nth-child(2),
-        &.add-code td:nth-child(3),
-        &.del-code td:nth-child(4),
-        &.del-code td:nth-child(5),
-        &.del-code td:nth-child(6) {
-          background-color: #fafbfc;
-          border-right-color: #eaecef;
-        }
-
-        &.del-code {
-          background-color: #ffeef0;
-        }
-
-        &.del-code td.add-code {
-          background-color: #e6ffed;
-        }
-        &.del-code td.lines-num-new.add-code {
-          background-color: #cdffd8;
-          border-color: #bef5cb;
-        }
-
-        &.add-code {
-          background-color: #e6ffed;
-          border-color: #bef5cb;
-        }
-
-        &.add-code td.lines-num-new {
-          background-color: #cdffd8;
-        }
-
-        td:nth-child(4) {
-          border-left-width: 1px;
-          border-left-style: solid;
-          border-left-color: #f6f8fa;
-        }
-      }
     }
 
     &.file-content {
@@ -1708,7 +1613,7 @@
       list-style: none;
       padding-bottom: 4px;
       margin-bottom: 4px;
-      border-bottom: 1px dashed #dddddd;
+      border-bottom: 1px dashed var(--color-secondary);
       padding-left: 6px;
     }
   }
@@ -2563,12 +2468,13 @@
 .comment-header {
   #avatar-arrow;
   border: none !important;
+  background: var(--color-box-header);
   border-bottom: 1px solid var(--color-secondary) !important;
   font-weight: normal !important;
   padding: .5rem 1rem !important;
   margin: 0 !important;
   position: relative;
-  color: #767676;
+  color: var(--color-text-light-2);
   min-height: 41px;
   background-color: var(--color-box-header);
   display: flex;
@@ -2589,6 +2495,14 @@
     top: -8px;
     left: 7px;
   }
+
+  a {
+    color: var(--color-text);
+  }
+
+  a:hover {
+    color: var(--color-primary);
+  }
 }
 
 .comment-header .actions a {
@@ -2596,21 +2510,13 @@
   padding: .5rem !important;
 }
 
-.comment-header a {
-  color: rgba(0, 0, 0, .4) !important;
-}
-
-.comment-header .actions a:hover,
-.comment-header .actions a.active {
-  color: rgba(0, 0, 0, .8) !important;
-}
-
 .comment-header-left > * + *,
 .comment-header-right > * + * {
   margin-left: .25rem;
 }
 
 .comment-body {
+  background: var(--color-box-body);
   border: none !important;
   width: 100% !important;
   max-width: 100% !important;
@@ -3105,11 +3011,45 @@ td.blob-excerpt {
 }
 
 .removed-code {
-  background-color: #fdb8c0;
+  background: var(--color-diff-removed-word-bg);
 }
 
 .added-code {
-  background-color: #acf2bd;
+  background: var(--color-diff-added-word-bg);
+}
+
+.code-diff-unified .del-code,
+.code-diff-unified .del-code td,
+.code-diff-split .del-code .lines-num-old,
+.code-diff-split .del-code .lines-type-marker-old,
+.code-diff-split .del-code .lines-code-old {
+  background: var(--color-diff-removed-row-bg);
+  border-color: var(--color-diff-removed-row-border);
+}
+
+.code-diff-unified .add-code,
+.code-diff-unified .add-code td,
+.code-diff-split .add-code .lines-num-new,
+.code-diff-split .add-code .lines-type-marker-new,
+.code-diff-split .add-code .lines-code-new,
+.code-diff-split .del-code .add-code.lines-num-new,
+.code-diff-split .del-code .add-code.lines-type-marker-new,
+.code-diff-split .del-code .add-code.lines-code-new {
+  background: var(--color-diff-added-row-bg);
+  border-color: var(--color-diff-added-row-border);
+}
+
+.code-diff-split .del-code .lines-num-new,
+.code-diff-split .del-code .lines-type-marker-new,
+.code-diff-split .del-code .lines-code-new,
+.code-diff-split .add-code .lines-num-old,
+.code-diff-split .add-code .lines-type-marker-old,
+.code-diff-split .add-code .lines-code-old {
+  background: var(--color-diff-inactive);
+}
+
+.code-diff-split tbody tr td:nth-child(4) {
+  border-left: 1px solid var(--color-secondary);
 }
 
 .repository .ui.menu.new-menu {

diff --git a/web_src/less/_review.less b/web_src/less/_review.less
index 2d0ac1b324d2920563521ffcba7880ae007f262c..222d2f1e23d8d08ad05557a7bfcf8a1f84b78b4f 100644 (file)
--- a/web_src/less/_review.less
+++ b/web_src/less/_review.less
@@ -119,7 +119,7 @@
 }
 
 .file-comment {
-  color: rgba(0, 0, 0, .87);
+  color: var(--color-text);
 }
 
 a.fold-file {

diff --git a/web_src/less/themes/theme-arc-green.less b/web_src/less/themes/theme-arc-green.less
index 141b87068cab7c6bdf86084d8ef5aa1149f6b2f5..871b5b00b81e1112df75e9b910e1e77c08c949d0 100644 (file)
--- a/web_src/less/themes/theme-arc-green.less
+++ b/web_src/less/themes/theme-arc-green.less
@@ -68,15 +68,23 @@
   --color-black: #1e222e;
   --color-gold: #a1882b;
   --color-white: #ffffff;
+  --color-diff-removed-word-bg: #6f3333;
+  --color-diff-added-word-bg: #3c653c;
+  --color-diff-removed-row-bg: #3c2626;
+  --color-diff-added-row-bg: #283e2d;
+  --color-diff-removed-row-border: #634343;
+  --color-diff-added-row-border: #314a37;
+  --color-diff-inactive: #353846;
   /* target-based colors */
   --color-body: #383c4a;
-  --color-box-header: #454a57;
-  --color-box-body: #353945;
+  --color-box-header: #404652;
+  --color-box-body: #303440;
+  --color-text-dark: #dbe0ea;
   --color-text: #bbc0ca;
   --color-text-light: #a6aab5;
   --color-text-light-2: #868a95;
   --color-footer: #2e323e;
-  --color-timeline: #4a505c;
+  --color-timeline: #4c525e;
   --color-input-text: #d5dbe6;
   --color-input-background: #2e323e;
   --color-input-border: #454a57;
@@ -318,10 +326,6 @@ a.ui.basic.green.label:hover {
   background-color: #393d4a !important;
 }
 
-.ui .text.grey {
-  color: var(--color-secondary-dark-6) !important;
-}
-
 .repository.file.editor.edit,
 .repository.wiki.new .CodeMirror {
   .editor-preview,
@@ -360,32 +364,6 @@ a.ui.basic.green.label:hover {
   color: #dbdbdb;
 }
 
-.repository.view.issue .comment-list .event > .svg.issue-symbol {
-  background: #3b4954;
-}
-
-.repository.view.issue .comment-list .event > .svg:not(.issue-symbol) {
-  text-shadow: -2px 0 #383c4a, 0 2px #383c4a, 2px 0 #383c4a, 0 -2px #383c4a;
-}
-
-.repository.view.issue .comment-list .comment .content .header {
-  color: #dbdbdb;
-  background-color: var(--color-secondary);
-  border-color: var(--color-secondary);
-}
-
-.repository.view.issue .comment-list .timeline-item .badge {
-  color: #ccc;
-}
-
-.comment-header-right a {
-  color: var(--color-secondary-dark-6);
-}
-
-.comment-header-right a:hover {
-  color: #dedede;
-}
-
 .repository .navbar .active.item,
 .repository .navbar .active.item:hover {
   border-color: transparent !important;
@@ -395,36 +373,10 @@ a.ui.basic.green.label:hover {
   background-color: var(--color-secondary) !important;
 }
 
-.repository .diff-file-box .code-diff-unified tbody tr.del-code td {
-  background-color: #3c2626 !important;
-  border-color: #634343 !important;
-}
-
-.repository .diff-file-box .code-diff-unified tbody tr.del-code td.lines-num {
-  background-color: #4e2c2c !important;
-}
-
-.repository .diff-file-box .code-diff-unified tbody tr.add-code td {
-  background-color: #283e2d !important;
-  border-color: #314a37 !important;
-}
-
-.repository .diff-file-box .code-diff-unified tbody tr.add-code td.lines-num {
-  background-color: #2c4632 !important;
-}
-
 .repository .diff-stats li {
   border-color: var(--color-secondary);
 }
 
-.removed-code {
-  background-color: #5f3737;
-}
-
-.added-code {
-  background-color: #3a523a;
-}
-
 .tag-code,
 .tag-code td {
   background: #353945 !important;
@@ -439,22 +391,6 @@ td.blob-hunk {
   color: #dbdbdb !important;
 }
 
-.lines-type-marker {
-  background: #2a2e3a;
-}
-
-.code-diff-split .same-code .lines-type-marker {
-  background: #353945;
-}
-
-.ui .text.black {
-  color: var(--color-secondary-dark-6);
-}
-
-.ui .text.black:hover {
-  color: #dbdbdb;
-}
-
 .ui.attached.info.message,
 .ui.info.message {
   box-shadow: 0 0 0 1px #4b5e71 inset, 0 0 0 0 transparent;
@@ -666,36 +602,6 @@ a.blob-excerpt:hover {
   }
 }
 
-.repository .label.list .item {
-  border-bottom: 1px dashed var(--color-secondary);
-}
-
-.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(1),
-.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(2),
-.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(3),
-.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(4),
-.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(5),
-.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(6) {
-  background-color: #2a2e3a;
-}
-
-.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(4),
-.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(5),
-.repository .diff-file-box .code-diff-split tbody tr.add-code td:nth-child(6),
-.repository .diff-file-box .code-diff-split tbody tr td.add-code,
-.repository .diff-file-box .code-diff-split tbody tr td.lines-num-new.add-code {
-  background-color: #283e2d !important;
-  border-color: #314a37 !important;
-}
-
-.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(1),
-.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(2),
-.repository .diff-file-box .code-diff-split tbody tr.del-code td:nth-child(3),
-.repository .diff-file-box .code-diff-split tbody tr td.del-code {
-  background-color: #3c2626 !important;
-  border-color: #634343 !important;
-}
-
 .ui.header .sub.header {
   color: var(--color-secondary-dark-6);
 }
@@ -764,46 +670,6 @@ a.blob-excerpt:hover {
   }
 }
 
-.comment-code-cloud {
-  border-color: transparent;
-
-  .ui.attached.top.header {
-    background: none transparent;
-    border: 0;
-  }
-
-  .footer .markdown-info {
-    color: inherit;
-  }
-}
-
-.file-comment {
-  color: var(--color-secondary-dark-6);
-}
-
-.ui.comments .comment {
-  .author {
-    color: #dbdbdb;
-  }
-
-  .metadata {
-    color: #808084;
-  }
-
-  .text {
-    color: var(--color-secondary-dark-6);
-  }
-}
-
-.comment-header a {
-  color: var(--color-secondary-dark-6) !important;
-}
-
-.comment-header .actions a:hover,
-.comment-header .actions a.active {
-  color: #dedede !important;
-}
-
 /* code mirror dark theme */
 
 .CodeMirror {