Prevent invalid behavior for file reviewing when loading more files (#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. Lastly, the `go-licenses.json` was automatically updated. Fixes #21228. Fixes #20681. Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
		
							parent
							
								
									0a9a86b943
								
							
						
					
					
						commit
						acee32ca09
					
				
					 4 changed files with 29 additions and 15 deletions
				
			
		
										
											
												File diff suppressed because one or more lines are too long
											
										
									
								
							| 
						 | 
					@ -57,7 +57,8 @@
 | 
				
			||||||
			{{end}}
 | 
								{{end}}
 | 
				
			||||||
		</ol>
 | 
							</ol>
 | 
				
			||||||
		<div id="diff-file-boxes">
 | 
							<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}}
 | 
									{{$blobBase := call $.GetBlobByPathForCommit $.BaseCommit $file.OldName}}
 | 
				
			||||||
				{{$blobHead := call $.GetBlobByPathForCommit $.HeadCommit $file.Name}}
 | 
									{{$blobHead := call $.GetBlobByPathForCommit $.HeadCommit $file.Name}}
 | 
				
			||||||
				{{$isImage := or (call $.IsBlobAnImage $blobBase) (call $.IsBlobAnImage $blobHead)}}
 | 
									{{$isImage := or (call $.IsBlobAnImage $blobBase) (call $.IsBlobAnImage $blobHead)}}
 | 
				
			||||||
| 
						 | 
					@ -93,8 +94,8 @@
 | 
				
			||||||
						<div class="diff-file-header-actions df ac">
 | 
											<div class="diff-file-header-actions df ac">
 | 
				
			||||||
							{{if $showFileViewToggle}}
 | 
												{{if $showFileViewToggle}}
 | 
				
			||||||
								<div class="ui compact icon buttons">
 | 
													<div class="ui compact icon buttons">
 | 
				
			||||||
									<span class="ui tiny basic button tooltip file-view-toggle" data-toggle-selector="#diff-source-{{$i}}" data-content="{{$.locale.Tr "repo.file_view_source"}}" data-position="bottom center">{{svg "octicon-code"}}</span>
 | 
														<span class="ui tiny basic button tooltip file-view-toggle" data-toggle-selector="#diff-source-{{$file.NameHash}}" data-content="{{$.locale.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="{{$.locale.Tr "repo.file_view_rendered"}}" data-position="bottom center">{{svg "octicon-file"}}</span>
 | 
														<span class="ui tiny basic button tooltip file-view-toggle active" data-toggle-selector="#diff-rendered-{{$file.NameHash}}" data-content="{{$.locale.Tr "repo.file_view_rendered"}}" data-position="bottom center">{{svg "octicon-file"}}</span>
 | 
				
			||||||
								</div>
 | 
													</div>
 | 
				
			||||||
							{{end}}
 | 
												{{end}}
 | 
				
			||||||
							{{if $file.IsProtected}}
 | 
												{{if $file.IsProtected}}
 | 
				
			||||||
| 
						 | 
					@ -115,15 +116,14 @@
 | 
				
			||||||
								{{if $file.HasChangedSinceLastReview}}
 | 
													{{if $file.HasChangedSinceLastReview}}
 | 
				
			||||||
									<span class="changed-since-last-review unselectable">{{$.locale.Tr "repo.pulls.has_changed_since_last_review"}}</span>
 | 
														<span class="changed-since-last-review unselectable">{{$.locale.Tr "repo.pulls.has_changed_since_last_review"}}</span>
 | 
				
			||||||
								{{end}}
 | 
													{{end}}
 | 
				
			||||||
								<div data-link="{{$.Issue.Link}}/viewed-files" data-headcommit="{{$.PullHeadCommitID}}" class="viewed-file-form unselectable{{if $file.IsViewed}} viewed-file-checked-form{{end}}">
 | 
													<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}}" id="viewed-file-checkbox-{{$i}}" autocomplete="off" {{if $file.IsViewed}}checked{{end}}></input>
 | 
														<input type="checkbox" name="{{$file.GetDiffFileName}}" autocomplete="off"{{if $file.IsViewed}} checked{{end}}> {{$.locale.Tr "repo.pulls.has_viewed_file"}}
 | 
				
			||||||
									<label for="viewed-file-checkbox-{{$i}}">{{$.locale.Tr "repo.pulls.has_viewed_file"}}</label>
 | 
													</label>
 | 
				
			||||||
								</div>
 | 
					 | 
				
			||||||
							{{end}}
 | 
												{{end}}
 | 
				
			||||||
						</div>
 | 
											</div>
 | 
				
			||||||
					</h4>
 | 
										</h4>
 | 
				
			||||||
					<div class="diff-file-body ui attached unstackable table segment" {{if $file.IsViewed}}data-folded="true"{{end}}>
 | 
										<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}}
 | 
												{{if or $file.IsIncomplete $file.IsBin}}
 | 
				
			||||||
								<div class="diff-file-body binary" style="padding: 5px 10px;">
 | 
													<div class="diff-file-body binary" style="padding: 5px 10px;">
 | 
				
			||||||
									{{if $file.IsIncomplete}}
 | 
														{{if $file.IsIncomplete}}
 | 
				
			||||||
| 
						 | 
					@ -148,7 +148,7 @@
 | 
				
			||||||
							{{end}}
 | 
												{{end}}
 | 
				
			||||||
						</div>
 | 
											</div>
 | 
				
			||||||
						{{if $showFileViewToggle}}
 | 
											{{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">
 | 
													<table class="chroma w-100">
 | 
				
			||||||
									{{if $isImage}}
 | 
														{{if $isImage}}
 | 
				
			||||||
										{{template "repo/diff/image_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead}}
 | 
															{{template "repo/diff/image_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead}}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -72,8 +72,8 @@ func TestMain(m *testing.M) {
 | 
				
			||||||
	os.Exit(exitVal)
 | 
						os.Exit(exitVal)
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
// This should be the only test e2e necessary. It will collect all "*.test.e2e.js"
 | 
					// TestE2e should be the only test e2e necessary. It will collect all "*.test.e2e.js"
 | 
				
			||||||
//   files in this directory and build a test for each.
 | 
					// files in this directory and build a test for each.
 | 
				
			||||||
func TestE2e(t *testing.T) {
 | 
					func TestE2e(t *testing.T) {
 | 
				
			||||||
	// Find the paths of all e2e test files in test test directory.
 | 
						// Find the paths of all e2e test files in test test directory.
 | 
				
			||||||
	searchGlob := filepath.Join(filepath.Dir(setting.AppPath), "tests", "e2e", "*.test.e2e.js")
 | 
						searchGlob := filepath.Join(filepath.Dir(setting.AppPath), "tests", "e2e", "*.test.e2e.js")
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -272,13 +272,22 @@ a.blob-excerpt:hover {
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
.viewed-file-form {
 | 
					.viewed-file-form {
 | 
				
			||||||
  margin: 0 3px;
 | 
					  display: flex;
 | 
				
			||||||
  padding: 0 3px;
 | 
					  align-items: center;
 | 
				
			||||||
  border-radius: 3px;
 | 
					  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 {
 | 
					.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 {
 | 
					#viewed-files-summary {
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue