From e50473e6bb0b7ca5d61b8d60ce3668fccbcaea96 Mon Sep 17 00:00:00 2001
From: delvh <leon@kske.dev>
Date: Fri, 23 Sep 2022 02:45:51 +0200
Subject: [PATCH] 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 <wxiaoguang@gmail.com>

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
---
 templates/repo/diff/box.tmpl | 18 +++++++++---------
 web_src/less/_review.less    | 17 +++++++++++++----
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl
index 98669ce0cb..15f0203efe 100644
--- a/templates/repo/diff/box.tmpl
+++ b/templates/repo/diff/box.tmpl
@@ -57,7 +57,8 @@
 			{{end}}
 		</ol>
 		<div id="diff-file-boxes">
-			{{range $i, $file := .Diff.Files}}
+			{{range $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*/}}
 				{{$blobBase := call $.GetBlobByPathForCommit $.BaseCommit $file.OldName}}
 				{{$blobHead := call $.GetBlobByPathForCommit $.HeadCommit $file.Name}}
 				{{$isImage := or (call $.IsBlobAnImage $blobBase) (call $.IsBlobAnImage $blobHead)}}
@@ -93,8 +94,8 @@
 						<div class="diff-file-header-actions df ac">
 							{{if $showFileViewToggle}}
 								<div class="ui compact icon buttons">
-									<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>
-									<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>
+									<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>
+									<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>
 								</div>
 							{{end}}
 							{{if $file.IsProtected}}
@@ -115,15 +116,14 @@
 								{{if $file.HasChangedSinceLastReview}}
 									<span class="changed-since-last-review unselectable">{{$.i18n.Tr "repo.pulls.has_changed_since_last_review"}}</span>
 								{{end}}
-								<div data-link="{{$.Issue.Link}}/viewed-files" data-headcommit="{{$.PullHeadCommitID}}" class="viewed-file-form unselectable{{if $file.IsViewed}} viewed-file-checked-form{{end}}">
-									<input type="checkbox" name="{{$file.GetDiffFileName}}" id="viewed-file-checkbox-{{$i}}" autocomplete="off" {{if $file.IsViewed}}checked{{end}}></input>
-									<label for="viewed-file-checkbox-{{$i}}">{{$.i18n.Tr "repo.pulls.has_viewed_file"}}</label>
-								</div>
+								<label data-link="{{$.Issue.Link}}/viewed-files" data-headcommit="{{$.PullHeadCommitID}}" class="viewed-file-form unselectable{{if $file.IsViewed}} viewed-file-checked-form{{end}}">
+									<input type="checkbox" name="{{$file.GetDiffFileName}}" autocomplete="off"{{if $file.IsViewed}} checked{{end}}> {{$.i18n.Tr "repo.pulls.has_viewed_file"}}
+								</label>
 							{{end}}
 						</div>
 					</h4>
 					<div class="diff-file-body ui attached unstackable table segment" {{if $file.IsViewed}}data-folded="true"{{end}}>
-						<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}}">
+						<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}}">
 							{{if or $file.IsIncomplete $file.IsBin}}
 								<div class="diff-file-body binary" style="padding: 5px 10px;">
 									{{if $file.IsIncomplete}}
@@ -148,7 +148,7 @@
 							{{end}}
 						</div>
 						{{if $showFileViewToggle}}
-							<div id="diff-rendered-{{$i}}" class="file-body file-code {{if $.IsSplitStyle}} code-diff-split{{else}} code-diff-unified{{end}}">
+							<div id="diff-rendered-{{$file.NameHash}}" class="file-body file-code {{if $.IsSplitStyle}} code-diff-split{{else}} code-diff-unified{{end}}">
 								<table class="chroma w-100">
 									{{if $isImage}}
 										{{template "repo/diff/image_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead}}
diff --git a/web_src/less/_review.less b/web_src/less/_review.less
index 2fa8efd67e..7aed7d857f 100644
--- a/web_src/less/_review.less
+++ b/web_src/less/_review.less
@@ -272,13 +272,22 @@ a.blob-excerpt:hover {
 }
 
 .viewed-file-form {
-  margin: 0 3px;
-  padding: 0 3px;
-  border-radius: 3px;
+  display: flex;
+  align-items: center;
+  border: 1px none;
+  padding: 4px 8px;
+  margin: -8px 0; // just like other buttons in the diff box header
+  border-radius: .285rem; // just like .ui.tiny.button
+  font-size: .857rem; // just like .ui.tiny.button
+}
+
+.viewed-file-form input {
+  margin-right: 4px;
 }
 
 .viewed-file-checked-form {
-  background-color: var(--color-primary-light-4);
+  background-color: var(--color-primary-light-6);
+  border: 1px solid var(--color-primary-light-4);
 }
 
 #viewed-files-summary {