From aa4d1d94f79e8edd9fa9ff2813fea12b085b2cae Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 30 Mar 2023 14:06:10 +0200 Subject: [PATCH] Diff improvements (#23553) - Avoid flash of wrong tree toggle icon on page load by setting icon based on sync state - Avoid "pop-in" of tree on page load by leaving space based on sync state - Use the same border/box-shadow combo used on comment `:target` also for file `:target`. - Refactor `DiffFileTree.vue` to use `toggleElem` instead of hardcoded class name. - Left-align inline comment boxes and make them fit the same amount of markup content on a line as GitHub. - Fix height of `diff-file-list` Fixes: https://github.com/go-gitea/gitea/issues/23593 Screenshot 2023-03-18 at 00 52 04 Screenshot 2023-03-18 at 00 59 43 Screenshot 2023-03-18 at 01 27 14 Screenshot 2023-03-19 at 21 44 16 (Yes I'm aware the border-radius in bottom corners is suboptimal, but this would be notorously hard to fix without relying on `overflow: hidden`). --- options/locale/locale_en-US.ini | 2 + templates/repo/diff/box.tmpl | 18 +++++++-- web_src/css/base.css | 4 ++ web_src/css/repository.css | 43 +++++++++++++++++++--- web_src/css/review.css | 11 +----- web_src/js/components/DiffFileTree.vue | 21 ++++++----- web_src/js/components/DiffFileTreeItem.vue | 22 ++++++----- 7 files changed, 81 insertions(+), 40 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 3f47af826e..0d5b2c9a44 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2280,6 +2280,8 @@ diff.image.side_by_side = Side by Side diff.image.swipe = Swipe diff.image.overlay = Overlay diff.has_escaped = This line has hidden Unicode characters +diff.show_file_tree = Show file tree +diff.hide_file_tree = Hide file tree releases.desc = Track project versions and downloads. release.releases = Releases diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 4a5fc4b7cf..36e669276e 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -15,11 +15,18 @@
- + +
{{svg "octicon-diff" 16 "gt-mr-2"}}{{.locale.Tr "repo.diff.stats_desc" .Diff.NumFiles .Diff.TotalAddition .Diff.TotalDeletion | Str2html}}
@@ -68,6 +75,9 @@
+
{{range $i, $file := .Diff.Files}} {{/*notice: the index of Diff.Files should not be used for element ID, because the index will be restarted from 0 when doing load-more for PRs with a lot of files*/}} diff --git a/web_src/css/base.css b/web_src/css/base.css index 3e25a47a01..9dc1f59dd1 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -238,6 +238,10 @@ table { border-collapse: collapse; } +button { + cursor: pointer; +} + details summary { cursor: pointer; } diff --git a/web_src/css/repository.css b/web_src/css/repository.css index e2f2075946..27d6a51cdd 100644 --- a/web_src/css/repository.css +++ b/web_src/css/repository.css @@ -1616,14 +1616,12 @@ padding: 7px 0; background: var(--color-body); line-height: 30px; - height: 47px; /* match .ui.attached.header.diff-file-header.sticky-2nd-row */ } @media (max-width: 991px) { .repository .diff-detail-box { flex-direction: column; align-items: flex-start; - height: 77px; /* match .ui.attached.header.diff-file-header.sticky-2nd-row */ } } @@ -1679,6 +1677,13 @@ } } +.diff-detail-actions { + /* prevent font-size from increasing element height so that .diff-detail-box comes + out with height of 47px (one line) and 77px (two lines), which is important for + position: sticky */ + height: 33px; +} + .repository .diff-detail-box .diff-detail-actions > * { margin-right: 0; } @@ -1853,10 +1858,24 @@ padding-bottom: 5px; } +.diff-file-box { + border: 1px solid transparent; + border-radius: var(--border-radius); +} + +/* TODO: this can potentially be made "global" by removing the class prefix */ +.diff-file-box .ui.attached.header, +.diff-file-box .ui.attached.table { + margin: 0; /* remove fomantic negative margins */; + width: initial; /* remove fomantic over 100% width */; + max-width: initial; /* remove fomantic over 100% width */; +} + .repository .diff-stats { clear: both; margin-bottom: 5px; - max-height: 400px; + max-height: 200px; + height: fit-content; overflow: auto; padding-left: 0; } @@ -2652,7 +2671,8 @@ filter: drop-shadow(-3px 0 0 var(--color-primary-alpha-30)) !important; } -.code-comment:target { +.code-comment:target, +.diff-file-box:target { border-color: var(--color-primary) !important; border-radius: var(--border-radius) !important; box-shadow: 0 0 0 3px var(--color-primary-alpha-30) !important; @@ -3226,17 +3246,28 @@ td.blob-excerpt { } #diff-file-tree { - width: 20%; + flex: 0 0 20%; max-width: 380px; line-height: inherit; position: sticky; padding-top: 0; top: 47px; - max-height: calc(100vh - 50px); + max-height: calc(100vh - 47px); height: 100%; overflow-y: auto; } +.diff-toggle-file-tree-button { + background: none; + border: none; + user-select: none; + color: inherit; +} + +.diff-toggle-file-tree-button:hover { + color: var(--color-primary); +} + @media (max-width: 991px) { #diff-file-tree { display: none !important; diff --git a/web_src/css/review.css b/web_src/css/review.css index 42267b4d2a..3deb2192fc 100644 --- a/web_src/css/review.css +++ b/web_src/css/review.css @@ -67,8 +67,7 @@ .comment-code-cloud { padding: 0.5rem 1rem !important; position: relative; - margin: 0 auto; - max-width: 1000px; + max-width: 820px; } @media (max-width: 767px) { @@ -308,11 +307,3 @@ a.blob-excerpt:hover { width: 72px; height: 10px; } - -.diff-file-box { - border-radius: 0.285rem; /* Just like ui.top.attached.header */ -} - -.diff-file-box:target { - box-shadow: 0 0 0 3px var(--color-accent); -} diff --git a/web_src/js/components/DiffFileTree.vue b/web_src/js/components/DiffFileTree.vue index fa59768ee5..2c2fabfed7 100644 --- a/web_src/js/components/DiffFileTree.vue +++ b/web_src/js/components/DiffFileTree.vue @@ -16,6 +16,7 @@