From 8af9df00360a812d31281179f4c6e095959e487e Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 17 Dec 2020 16:52:58 +0100 Subject: [PATCH] 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> --- .../repo/issue/view_content/comments.tmpl | 11 +- web_src/less/_base.less | 53 +++++- web_src/less/_repository.less | 180 ++++++------------ web_src/less/_review.less | 2 +- web_src/less/themes/theme-arc-green.less | 156 ++------------- 5 files changed, 124 insertions(+), 278 deletions(-) diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 2c413b3c4..bc5d7dde1 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -398,10 +398,7 @@ {{avatar .Poster}} {{end}} - {{svg (printf "octicon-%s" .Review.Type.Icon)}} + {{svg (printf "octicon-%s" .Review.Type.Icon)}} {{if .OriginalAuthor }} {{ .OriginalAuthor }} {{if $.Repository.OriginalURL}}({{$.i18n.Tr "repo.migrated_from" $.Repository.OriginalURL $.Repository.GetOriginalURLHostname | Safe }}){{end}} @@ -452,7 +449,7 @@ {{ range $filename, $lines := .Review.CodeComments}} {{range $line, $comms := $lines}}
-
+
{{$invalid := (index $comms 0).Invalidated}} {{$resolved := (index $comms 0).IsResolved}} {{$resolveDoer := (index $comms 0).ResolveDoer}} @@ -497,8 +494,8 @@
{{end}} -
-
+
+
{{range $comms}} {{ $createdSubStr:= TimeSinceUnix .CreatedUnix $.Lang }}
diff --git a/web_src/less/_base.less b/web_src/less/_base.less index 45cd54932..9a7aa28d4 100644 --- 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 7cac37207..fe1c25881 100644 --- 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 2d0ac1b32..222d2f1e2 100644 --- 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 141b87068..871b5b00b 100644 --- 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 {