loading up the forgejo repo on tangled to test page performance
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
<img width="1250" alt="Screenshot 2023-03-18 at 00 52 04"
src="https://user-images.githubusercontent.com/115237/226071392-6789a644-aead-4756-a77e-aba3642150a0.png">
<img width="1246" alt="Screenshot 2023-03-18 at 00 59 43"
src="https://user-images.githubusercontent.com/115237/226071443-8bcba924-458b-48bd-b2f0-0de59cb180ac.png">
<img width="1250" alt="Screenshot 2023-03-18 at 01 27 14"
src="https://user-images.githubusercontent.com/115237/226073121-ccb99f9a-d3ac-40b7-9589-43580c4a01c9.png">
<img width="1231" alt="Screenshot 2023-03-19 at 21 44 16"
src="https://user-images.githubusercontent.com/115237/226207951-81bcae1b-6b41-4e39-83a7-0f37951df6be.png">
(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`).
authored by
silverwind
and committed by
GitHub
aa4d1d94
35cb786c
+81
-40
+2
options/locale/locale_en-US.ini
+2
options/locale/locale_en-US.ini
···
2280
2280
diff.image.swipe = Swipe
2281
2281
diff.image.overlay = Overlay
2282
2282
diff.has_escaped = This line has hidden Unicode characters
2283
+
diff.show_file_tree = Show file tree
2284
+
diff.hide_file_tree = Hide file tree
2283
2285
2284
2286
releases.desc = Track project versions and downloads.
2285
2287
release.releases = Releases
+14
-4
templates/repo/diff/box.tmpl
+14
-4
templates/repo/diff/box.tmpl
···
15
15
<div>
16
16
<div class="diff-detail-box diff-box sticky gt-df gt-sb gt-ac gt-fw">
17
17
<div class="gt-df gt-ac gt-fw">
18
-
<a class="diff-toggle-file-tree-button muted gt-df gt-ac">
18
+
<button class="diff-toggle-file-tree-button gt-df gt-ac" data-show-text="{{.locale.Tr "repo.diff.show_file_tree"}}" data-hide-text="{{.locale.Tr "repo.diff.hide_file_tree"}}">
19
19
{{/* the icon meaning is reversed here, "octicon-sidebar-collapse" means show the file tree */}}
20
-
{{svg "octicon-sidebar-collapse" 20 "icon hide"}}
21
-
{{svg "octicon-sidebar-expand" 20 "icon"}}
22
-
</a>
20
+
{{svg "octicon-sidebar-collapse" 20 "icon gt-hidden"}}
21
+
{{svg "octicon-sidebar-expand" 20 "icon gt-hidden"}}
22
+
</button>
23
+
<script>
24
+
const diffTreeVisible = localStorage?.getItem('diff_file_tree_visible') === 'true';
25
+
const diffTreeBtn = document.querySelector('.diff-toggle-file-tree-button');
26
+
const diffTreeIcon = `.octicon-sidebar-${diffTreeVisible ? 'expand' : 'collapse'}`;
27
+
diffTreeBtn.querySelector(diffTreeIcon).classList.remove('gt-hidden');
28
+
diffTreeBtn.setAttribute('data-tooltip-content', diffTreeBtn.getAttribute(diffTreeVisible ? 'data-hide-text' : 'data-show-text'));
29
+
</script>
23
30
<div class="diff-detail-stats gt-df gt-ac gt-ml-3">
24
31
{{svg "octicon-diff" 16 "gt-mr-2"}}{{.locale.Tr "repo.diff.stats_desc" .Diff.NumFiles .Diff.TotalAddition .Diff.TotalDeletion | Str2html}}
25
32
</div>
···
68
75
<div id="diff-file-list"></div>
69
76
<div id="diff-container">
70
77
<div id="diff-file-tree" class="gt-hidden"></div>
78
+
<script>
79
+
if (diffTreeVisible) document.getElementById('diff-file-tree').classList.remove('gt-hidden');
80
+
</script>
71
81
<div id="diff-file-boxes" class="sixteen wide column">
72
82
{{range $i, $file := .Diff.Files}}
73
83
{{/*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*/}}
+4
web_src/css/base.css
+4
web_src/css/base.css
+37
-6
web_src/css/repository.css
+37
-6
web_src/css/repository.css
···
1616
1616
padding: 7px 0;
1617
1617
background: var(--color-body);
1618
1618
line-height: 30px;
1619
-
height: 47px; /* match .ui.attached.header.diff-file-header.sticky-2nd-row */
1620
1619
}
1621
1620
1622
1621
@media (max-width: 991px) {
1623
1622
.repository .diff-detail-box {
1624
1623
flex-direction: column;
1625
1624
align-items: flex-start;
1626
-
height: 77px; /* match .ui.attached.header.diff-file-header.sticky-2nd-row */
1627
1625
}
1628
1626
}
1629
1627
···
1677
1675
.repository .diff-detail-box .diff-detail-stats strong {
1678
1676
font-size: 1rem;
1679
1677
}
1678
+
}
1679
+
1680
+
.diff-detail-actions {
1681
+
/* prevent font-size from increasing element height so that .diff-detail-box comes
1682
+
out with height of 47px (one line) and 77px (two lines), which is important for
1683
+
position: sticky */
1684
+
height: 33px;
1680
1685
}
1681
1686
1682
1687
.repository .diff-detail-box .diff-detail-actions > * {
···
1851
1856
.repository .diff-file-box .ui.bottom.attached.table.segment {
1852
1857
padding-top: 5px;
1853
1858
padding-bottom: 5px;
1859
+
}
1860
+
1861
+
.diff-file-box {
1862
+
border: 1px solid transparent;
1863
+
border-radius: var(--border-radius);
1864
+
}
1865
+
1866
+
/* TODO: this can potentially be made "global" by removing the class prefix */
1867
+
.diff-file-box .ui.attached.header,
1868
+
.diff-file-box .ui.attached.table {
1869
+
margin: 0; /* remove fomantic negative margins */;
1870
+
width: initial; /* remove fomantic over 100% width */;
1871
+
max-width: initial; /* remove fomantic over 100% width */;
1854
1872
}
1855
1873
1856
1874
.repository .diff-stats {
1857
1875
clear: both;
1858
1876
margin-bottom: 5px;
1859
-
max-height: 400px;
1877
+
max-height: 200px;
1878
+
height: fit-content;
1860
1879
overflow: auto;
1861
1880
padding-left: 0;
1862
1881
}
···
2652
2671
filter: drop-shadow(-3px 0 0 var(--color-primary-alpha-30)) !important;
2653
2672
}
2654
2673
2655
-
.code-comment:target {
2674
+
.code-comment:target,
2675
+
.diff-file-box:target {
2656
2676
border-color: var(--color-primary) !important;
2657
2677
border-radius: var(--border-radius) !important;
2658
2678
box-shadow: 0 0 0 3px var(--color-primary-alpha-30) !important;
···
3226
3246
}
3227
3247
3228
3248
#diff-file-tree {
3229
-
width: 20%;
3249
+
flex: 0 0 20%;
3230
3250
max-width: 380px;
3231
3251
line-height: inherit;
3232
3252
position: sticky;
3233
3253
padding-top: 0;
3234
3254
top: 47px;
3235
-
max-height: calc(100vh - 50px);
3255
+
max-height: calc(100vh - 47px);
3236
3256
height: 100%;
3237
3257
overflow-y: auto;
3258
+
}
3259
+
3260
+
.diff-toggle-file-tree-button {
3261
+
background: none;
3262
+
border: none;
3263
+
user-select: none;
3264
+
color: inherit;
3265
+
}
3266
+
3267
+
.diff-toggle-file-tree-button:hover {
3268
+
color: var(--color-primary);
3238
3269
}
3239
3270
3240
3271
@media (max-width: 991px) {
+1
-10
web_src/css/review.css
+1
-10
web_src/css/review.css
···
67
67
.comment-code-cloud {
68
68
padding: 0.5rem 1rem !important;
69
69
position: relative;
70
-
margin: 0 auto;
71
-
max-width: 1000px;
70
+
max-width: 820px;
72
71
}
73
72
74
73
@media (max-width: 767px) {
···
308
307
width: 72px;
309
308
height: 10px;
310
309
}
311
-
312
-
.diff-file-box {
313
-
border-radius: 0.285rem; /* Just like ui.top.attached.header */
314
-
}
315
-
316
-
.diff-file-box:target {
317
-
box-shadow: 0 0 0 3px var(--color-accent);
318
-
}
+11
-10
web_src/js/components/DiffFileTree.vue
+11
-10
web_src/js/components/DiffFileTree.vue
···
16
16
<script>
17
17
import DiffFileTreeItem from './DiffFileTreeItem.vue';
18
18
import {doLoadMoreFiles} from '../features/repo-diff.js';
19
+
import {toggleElem} from '../utils/dom.js';
19
20
20
21
const {pageData} = window.config;
21
22
const LOCAL_STORAGE_KEY = 'diff_file_tree_visible';
···
92
93
}
93
94
},
94
95
mounted() {
95
-
// ensure correct buttons when we are mounted to the dom
96
-
this.adjustToggleButton(this.fileTreeIsVisible);
97
96
// replace the pageData.diffFileInfo.files with our watched data so we get updates
98
97
pageData.diffFileInfo.files = this.files;
99
98
···
109
108
updateVisibility(visible) {
110
109
this.fileTreeIsVisible = visible;
111
110
localStorage.setItem(LOCAL_STORAGE_KEY, this.fileTreeIsVisible);
112
-
this.adjustToggleButton(this.fileTreeIsVisible);
111
+
this.updateState(this.fileTreeIsVisible);
113
112
},
114
-
adjustToggleButton(visible) {
115
-
const [toShow, toHide] = document.querySelectorAll('.diff-toggle-file-tree-button .icon');
116
-
toShow.classList.toggle('gt-hidden', visible); // hide the toShow icon if the tree is visible
117
-
toHide.classList.toggle('gt-hidden', !visible); // similarly
118
-
119
-
const diffTree = document.getElementById('diff-file-tree');
120
-
diffTree.classList.toggle('gt-hidden', !visible);
113
+
updateState(visible) {
114
+
const btn = document.querySelector('.diff-toggle-file-tree-button');
115
+
const [toShow, toHide] = btn.querySelectorAll('.icon');
116
+
const tree = document.getElementById('diff-file-tree');
117
+
const newTooltip = btn.getAttribute(visible ? 'data-hide-text' : 'data-show-text');
118
+
btn.setAttribute('data-tooltip-content', newTooltip);
119
+
toggleElem(tree, visible);
120
+
toggleElem(toShow, !visible);
121
+
toggleElem(toHide, visible);
121
122
},
122
123
loadMoreData() {
123
124
this.isLoadingNewData = true;
+12
-10
web_src/js/components/DiffFileTreeItem.vue
+12
-10
web_src/js/components/DiffFileTreeItem.vue
···
1
1
<template>
2
2
<div v-show="show" :title="item.name">
3
3
<!--title instead of tooltip above as the tooltip needs too much work with the current methods, i.e. not being loaded or staying open for "too long"-->
4
-
<div class="item" :class="item.isFile ? 'filewrapper gt-p-1' : ''">
4
+
<div class="item" :class="item.isFile ? 'filewrapper gt-p-1 gt-ac' : ''">
5
5
<!-- Files -->
6
6
<SvgIcon
7
7
v-if="item.isFile"
···
10
10
/>
11
11
<a
12
12
v-if="item.isFile"
13
-
class="file gt-ellipsis muted"
13
+
class="file gt-ellipsis"
14
14
:href="item.isFile ? '#diff-' + item.file.NameHash : ''"
15
15
>{{ item.name }}</a>
16
16
<SvgIcon
···
20
20
/>
21
21
22
22
<!-- Directories -->
23
-
<div v-if="!item.isFile" class="directory gt-p-1" @click.stop="handleClick(item.isFile)">
23
+
<div v-if="!item.isFile" class="directory gt-p-1 gt-ac" @click.stop="handleClick(item.isFile)">
24
24
<SvgIcon
25
25
class="svg-icon"
26
26
:name="collapsed ? 'octicon-chevron-right' : 'octicon-chevron-down'"
···
79
79
</script>
80
80
81
81
<style scoped>
82
-
span.svg-icon.status {
82
+
.svg-icon.status {
83
83
float: right;
84
84
}
85
85
86
-
span.svg-icon.file {
86
+
.svg-icon.file {
87
87
color: var(--color-secondary-dark-7);
88
88
}
89
89
90
-
span.svg-icon.directory {
90
+
.svg-icon.directory {
91
91
color: var(--color-primary);
92
92
}
93
93
94
-
span.svg-icon.octicon-diff-modified {
94
+
.svg-icon.octicon-diff-modified {
95
95
color: var(--color-yellow);
96
96
}
97
97
98
-
span.svg-icon.octicon-diff-added {
98
+
.svg-icon.octicon-diff-added {
99
99
color: var(--color-green);
100
100
}
101
101
102
-
span.svg-icon.octicon-diff-removed {
102
+
.svg-icon.octicon-diff-removed {
103
103
color: var(--color-red);
104
104
}
105
105
106
-
span.svg-icon.octicon-diff-renamed {
106
+
.svg-icon.octicon-diff-renamed {
107
107
color: var(--color-teal);
108
108
}
109
109
···
139
139
140
140
a {
141
141
text-decoration: none;
142
+
color: var(--color-text);
142
143
}
143
144
144
145
a:hover {
145
146
text-decoration: none;
147
+
color: var(--color-text);
146
148
}
147
149
</style>