Skip to content

Commit e50473e

Browse files
delvhwxiaoguang
andauthored
Prevent invalid behavior for file reviewing when loading more files (#21230) (#21234)
Backport of #21230 The problem was that many PR review components loaded by `Show more` received the same ID as previous batches, which confuses browsers (when clicked). All such occurrences should now be fixed. Additionally improved the background of the `viewed` checkbox. Fixes #21228. Fixes #20681. Co-authored-by: wxiaoguang <[email protected]> Co-authored-by: wxiaoguang <[email protected]>
1 parent 20c135c commit e50473e

File tree

2 files changed

+22
-13
lines changed

2 files changed

+22
-13
lines changed

templates/repo/diff/box.tmpl

+9-9
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@
5757
{{end}}
5858
</ol>
5959
<div id="diff-file-boxes">
60-
{{range $i, $file := .Diff.Files}}
60+
{{range $file := .Diff.Files}}
61+
{{/*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*/}}
6162
{{$blobBase := call $.GetBlobByPathForCommit $.BaseCommit $file.OldName}}
6263
{{$blobHead := call $.GetBlobByPathForCommit $.HeadCommit $file.Name}}
6364
{{$isImage := or (call $.IsBlobAnImage $blobBase) (call $.IsBlobAnImage $blobHead)}}
@@ -93,8 +94,8 @@
9394
<div class="diff-file-header-actions df ac">
9495
{{if $showFileViewToggle}}
9596
<div class="ui compact icon buttons">
96-
<span class="ui tiny basic button tooltip file-view-toggle" data-toggle-selector="#diff-source-{{$i}}" data-content="{{$.i18n.Tr "repo.file_view_source"}}" data-position="bottom center">{{svg "octicon-code"}}</span>
97-
<span class="ui tiny basic button tooltip file-view-toggle active" data-toggle-selector="#diff-rendered-{{$i}}" data-content="{{$.i18n.Tr "repo.file_view_rendered"}}" data-position="bottom center">{{svg "octicon-file"}}</span>
97+
<span class="ui tiny basic button tooltip file-view-toggle" data-toggle-selector="#diff-source-{{$file.NameHash}}" data-content="{{$.i18n.Tr "repo.file_view_source"}}" data-position="bottom center">{{svg "octicon-code"}}</span>
98+
<span class="ui tiny basic button tooltip file-view-toggle active" data-toggle-selector="#diff-rendered-{{$file.NameHash}}" data-content="{{$.i18n.Tr "repo.file_view_rendered"}}" data-position="bottom center">{{svg "octicon-file"}}</span>
9899
</div>
99100
{{end}}
100101
{{if $file.IsProtected}}
@@ -115,15 +116,14 @@
115116
{{if $file.HasChangedSinceLastReview}}
116117
<span class="changed-since-last-review unselectable">{{$.i18n.Tr "repo.pulls.has_changed_since_last_review"}}</span>
117118
{{end}}
118-
<div data-link="{{$.Issue.Link}}/viewed-files" data-headcommit="{{$.PullHeadCommitID}}" class="viewed-file-form unselectable{{if $file.IsViewed}} viewed-file-checked-form{{end}}">
119-
<input type="checkbox" name="{{$file.GetDiffFileName}}" id="viewed-file-checkbox-{{$i}}" autocomplete="off" {{if $file.IsViewed}}checked{{end}}></input>
120-
<label for="viewed-file-checkbox-{{$i}}">{{$.i18n.Tr "repo.pulls.has_viewed_file"}}</label>
121-
</div>
119+
<label data-link="{{$.Issue.Link}}/viewed-files" data-headcommit="{{$.PullHeadCommitID}}" class="viewed-file-form unselectable{{if $file.IsViewed}} viewed-file-checked-form{{end}}">
120+
<input type="checkbox" name="{{$file.GetDiffFileName}}" autocomplete="off"{{if $file.IsViewed}} checked{{end}}> {{$.i18n.Tr "repo.pulls.has_viewed_file"}}
121+
</label>
122122
{{end}}
123123
</div>
124124
</h4>
125125
<div class="diff-file-body ui attached unstackable table segment" {{if $file.IsViewed}}data-folded="true"{{end}}>
126-
<div id="diff-source-{{$i}}" class="file-body file-code unicode-escaped code-diff{{if $.IsSplitStyle}} code-diff-split{{else}} code-diff-unified{{end}}{{if $showFileViewToggle}} hide{{end}}">
126+
<div id="diff-source-{{$file.NameHash}}" class="file-body file-code unicode-escaped code-diff{{if $.IsSplitStyle}} code-diff-split{{else}} code-diff-unified{{end}}{{if $showFileViewToggle}} hide{{end}}">
127127
{{if or $file.IsIncomplete $file.IsBin}}
128128
<div class="diff-file-body binary" style="padding: 5px 10px;">
129129
{{if $file.IsIncomplete}}
@@ -148,7 +148,7 @@
148148
{{end}}
149149
</div>
150150
{{if $showFileViewToggle}}
151-
<div id="diff-rendered-{{$i}}" class="file-body file-code {{if $.IsSplitStyle}} code-diff-split{{else}} code-diff-unified{{end}}">
151+
<div id="diff-rendered-{{$file.NameHash}}" class="file-body file-code {{if $.IsSplitStyle}} code-diff-split{{else}} code-diff-unified{{end}}">
152152
<table class="chroma w-100">
153153
{{if $isImage}}
154154
{{template "repo/diff/image_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead}}

web_src/less/_review.less

+13-4
Original file line numberDiff line numberDiff line change
@@ -272,13 +272,22 @@ a.blob-excerpt:hover {
272272
}
273273

274274
.viewed-file-form {
275-
margin: 0 3px;
276-
padding: 0 3px;
277-
border-radius: 3px;
275+
display: flex;
276+
align-items: center;
277+
border: 1px none;
278+
padding: 4px 8px;
279+
margin: -8px 0; // just like other buttons in the diff box header
280+
border-radius: .285rem; // just like .ui.tiny.button
281+
font-size: .857rem; // just like .ui.tiny.button
282+
}
283+
284+
.viewed-file-form input {
285+
margin-right: 4px;
278286
}
279287

280288
.viewed-file-checked-form {
281-
background-color: var(--color-primary-light-4);
289+
background-color: var(--color-primary-light-6);
290+
border: 1px solid var(--color-primary-light-4);
282291
}
283292

284293
#viewed-files-summary {

0 commit comments

Comments
 (0)