Skip to content

Commit f26f71f

Browse files
wxiaoguangwxiaoguang
authored
Refactor pull request view (5) (#37517)
Clean up templates, remove various CSS patches. By the way, fix incorrect NewRequest URLs in tests.
1 parent c4c50be commit f26f71f

23 files changed

Lines changed: 144 additions & 167 deletions

‎templates/base/footer.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
{{ctx.ScriptImport "js/index.js" "module"}}
1313
{{template "custom/footer" .}}
1414
<script nonce="{{ctx.CspScriptNonce}}" type="module">
15-
if (!window.config?.frontendInited) alert("Frontend is not initialized, check console errors or asset files.")
15+
if (!window.config?.frontendInited && window.config?.runModeIsProd) alert("Frontend is not initialized, check console errors or asset files.");
1616
</script>
1717
</body>
1818
</html>

‎templates/devtest/flex-list.tmpl‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,21 +101,30 @@
101101
<div class="item">item 2</div>
102102
</div>
103103
</div>
104-
<h3>Flex List (with "ui segment fitted")</h3>
105-
<div class="ui attached segment fitted">
106-
<div class="flex-divided-list">
104+
<h3>Flex List (with "ui segment fitted", items have their own padding)</h3>
105+
<div class="ui attached segment fitted container-divided">
106+
<div class="flex-divided-list container-divided">
107107
<div class="item">item 1</div>
108108
<div class="item">item 2</div>
109+
<div class="item flex-divided-list">
110+
<div class="item">item nested 1</div>
111+
<div class="item">item nested 2</div>
112+
</div>
113+
<div class="item">item 3</div>
109114
</div>
110115
</div>
111116

112-
<h3>If parent provides border or padding:</h3>
113-
<div class="container-segmented tw-border tw-border-secondary">
117+
<h3>If parent provides padding or items need their own flex and/or padding:</h3>
118+
<div class="container-divided tw-border tw-border-secondary">
114119
<div class="tw-m-3">before divider</div>
115120
<div class="divider"></div>
116-
<div class="flex-divided-list">
121+
<div class="flex-divided-list container-divided flex-items-block">
117122
<div class="item">item 1</div>
118123
<div class="item">item 2</div>
124+
<div class="item flex-divided-list">
125+
<div class="item">item nested 1</div>
126+
<div class="item">item nested 2</div>
127+
</div>
119128
</div>
120129
<div class="divider"></div>
121130
<div class="tw-m-3">after divider</div>

‎templates/repo/commit_statuses.tmpl‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99
</span>
1010
{{end}}
1111
<div class="tippy-target">
12-
{{template "repo/pulls/status" (dict "CommitStatuses" .Statuses "CommitStatus" .Status)}}
12+
<div class="ui segment">
13+
<div class="flex-divided-list">
14+
{{template "repo/pulls/status" (dict "CommitStatuses" .Statuses "CommitStatus" .Status)}}
15+
</div>
16+
</div>
1317
</div>
1418
{{end}}

‎templates/repo/issue/view_content/pull_merge_box.tmpl‎

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,18 @@
2727
{{- else if .Issue.PullRequest.IsStatusMergeable}}tw-text-green
2828
{{- else}}tw-text-red{{end}}">{{svg "octicon-git-merge" 40}}</div>
2929
<div class="content">
30-
{{if .LatestCommitStatus}}
31-
<div class="ui attached segment fitted">
32-
{{template "repo/pulls/status" (dict
33-
"CommitStatus" .LatestCommitStatus
34-
"CommitStatuses" .LatestCommitStatuses
35-
"ShowHideChecks" true
36-
"StatusCheckData" $statusCheckData
37-
)}}
38-
</div>
39-
{{end}}
40-
{{$showGeneralMergeForm := false}}
41-
<div class="ui attached segment merge-section {{if not $.LatestCommitStatus}}avatar-content-left-arrow{{end}} flex-items-block">
30+
<div class="ui segment fitted avatar-content-left-arrow container-divided">
31+
<div class="merge-section flex-divided-list flex-items-block container-divided">
32+
{{if .LatestCommitStatus}}
33+
{{template "repo/pulls/status" (dict
34+
"CommitStatus" .LatestCommitStatus
35+
"CommitStatuses" .LatestCommitStatuses
36+
"ShowHideChecks" true
37+
"StatusCheckData" $statusCheckData
38+
)}}
39+
{{end}}
40+
41+
{{$showGeneralMergeForm := false}}
4242
{{if .Issue.PullRequest.HasMerged}}
4343
{{if .IsPullBranchDeletable}}
4444
<div class="item item-section text tw-flex-1">
@@ -78,7 +78,7 @@
7878
{{svg "octicon-x"}}
7979
{{ctx.Locale.Tr "repo.pulls.files_conflicted"}}
8080
</div>
81-
<ul>
81+
<ul class="item">
8282
{{range .ConflictedFiles}}
8383
<li>{{.}}</li>
8484
{{else}}
@@ -143,7 +143,7 @@
143143
{{svg "octicon-x"}}
144144
{{ctx.Locale.TrN $.ChangedProtectedFilesNum "repo.pulls.blocked_by_changed_protected_files_1" "repo.pulls.blocked_by_changed_protected_files_n"}}
145145
</div>
146-
<ul>
146+
<ul class="item">
147147
{{range .ChangedProtectedFiles}}
148148
<li>{{.}}</li>
149149
{{end}}
@@ -200,7 +200,6 @@
200200
{{template "repo/issue/view_content/update_branch_by_merge" $}}
201201

202202
{{if .Issue.PullRequest.IsEmpty}}
203-
<div class="divider"></div>
204203
<div class="item">
205204
{{svg "octicon-alert"}}
206205
{{ctx.Locale.Tr "repo.pulls.is_empty"}}
@@ -209,13 +208,13 @@
209208

210209
{{if .AllowMerge}} {{/* user is allowed to merge */}}
211210
{{if $data.MergeFormProps}}
212-
<div class="divider"></div>
213211
{{$showGeneralMergeForm = true}}
214212
{{/* The merge form is a Vue component. After mounted, it has a button for choosing merge style, so make it have min-height to avoid layout shifting */}}
215-
<div id="pull-request-merge-form" class="tw-min-h-[40px]" data-merge-form-props="{{JsonUtils.EncodeToString $data.MergeFormProps}}"></div>
213+
<div class="item">
214+
<div id="pull-request-merge-form" class="tw-min-h-[40px]" data-merge-form-props="{{JsonUtils.EncodeToString $data.MergeFormProps}}"></div>
215+
</div>
216216
{{else}}
217217
{{/* no merge style was set in repo setting: not or ($prUnit.PullRequestsConfig.AllowMerge ...) */}}
218-
<div class="divider"></div>
219218
<div class="item tw-text-red">
220219
{{svg "octicon-x"}}
221220
{{ctx.Locale.Tr "repo.pulls.no_merge_desc"}}
@@ -227,7 +226,6 @@
227226
{{end}} {{/* end if the repo was set to use any merge style */}}
228227
{{else}}
229228
{{/* user is not allowed to merge */}}
230-
<div class="divider"></div>
231229
<div class="item">
232230
{{svg "octicon-info"}}
233231
{{ctx.Locale.Tr "repo.pulls.no_merge_access"}}
@@ -260,7 +258,7 @@
260258
{{svg "octicon-x"}}
261259
{{ctx.Locale.TrN $.ChangedProtectedFilesNum "repo.pulls.blocked_by_changed_protected_files_1" "repo.pulls.blocked_by_changed_protected_files_n"}}
262260
</div>
263-
<ul>
261+
<ul class="item">
264262
{{range .ChangedProtectedFiles}}
265263
<li>{{.}}</li>
266264
{{end}}
@@ -296,20 +294,24 @@
296294
* Then the Manually Merged form will be shown in the merge form
297295
*/}}
298296
{{if and $.StillCanManualMerge (not $showGeneralMergeForm)}}
299-
<div class="divider"></div>
300-
<form class="ui form form-fetch-action" action="{{.Issue.Link}}/merge" method="post">{{/* another similar form is in PullRequestMergeForm.vue*/}}
301-
<div class="field">
302-
<input type="text" name="merge_commit_id" placeholder="{{ctx.Locale.Tr "repo.pulls.merge_commit_id"}}">
303-
</div>
304-
<button class="ui red button" type="submit" name="do" value="manually-merged">
305-
{{ctx.Locale.Tr "repo.pulls.merge_manually"}}
306-
</button>
307-
</form>
297+
<div class="item">
298+
<form class="ui form form-fetch-action tw-flex-1" action="{{.Issue.Link}}/merge" method="post">{{/* another similar form is in PullRequestMergeForm.vue*/}}
299+
<div class="field">
300+
<input type="text" name="merge_commit_id" placeholder="{{ctx.Locale.Tr "repo.pulls.merge_commit_id"}}">
301+
</div>
302+
<button class="ui red button" type="submit" name="do" value="manually-merged">
303+
{{ctx.Locale.Tr "repo.pulls.merge_manually"}}
304+
</button>
305+
</form>
306+
</div>
308307
{{end}}
309308

310309
{{if $data.ShowPullCommands}}
311-
{{template "repo/issue/view_content/pull_merge_instruction" dict "PullRequest" .Issue.PullRequest "MergeBoxData" $data}}
310+
<div class="item">
311+
{{template "repo/issue/view_content/pull_merge_instruction" dict "PullRequest" .Issue.PullRequest "MergeBoxData" $data}}
312+
</div>
312313
{{end}}
314+
</div>
313315
</div>
314316
</div>
315317
</div>

‎templates/repo/issue/view_content/pull_merge_instruction.tmpl‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
{{$data := $.MergeBoxData}}
22
{{$pull := $.PullRequest}}
3-
<div class="divider"></div>
43
<details>
54
<summary>{{ctx.Locale.Tr "repo.pulls.cmd_instruction_hint"}}</summary>
65
<div class="tw-mt-2">

‎templates/repo/issue/view_content/update_branch_by_merge.tmpl‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
{{if and (gt $.Issue.PullRequest.CommitsBehind 0) (not $.Issue.IsClosed) (not $.Issue.PullRequest.IsChecking) (not $.IsPullFilesConflicted) (not $.IsPullRequestBroken)}}
2-
<div class="divider"></div>
32
<div class="item item-section">
43
<div class="item-section-left flex-text-inline">
54
{{svg "octicon-alert"}}

‎templates/repo/pulls/status.tmpl‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,19 @@
66
*/}}
77
{{$statusCheckData := .StatusCheckData}}
88
{{if .CommitStatus}}
9-
<div class="commit-status-panel">
10-
<div class="ui top attached header commit-status-header">
11-
{{$statusCheckData.CommitStatusCheckPrompt ctx.Locale}}
12-
9+
{{if $statusCheckData}}
10+
<div class="item flex-left-right commit-status-toggle">
11+
<div>{{$statusCheckData.CommitStatusCheckPrompt ctx.Locale}}</div>
1312
{{if .ShowHideChecks}}
14-
<div class="ui right">
15-
<button class="commit-status-hide-checks btn interact-fg"
13+
<button data-global-click="onCommitStatusChecksToggle" class="btn interact-fg"
1614
data-show-all="{{ctx.Locale.Tr "repo.pulls.status_checks_show_all"}}"
17-
data-hide-all="{{ctx.Locale.Tr "repo.pulls.status_checks_hide_all"}}">
18-
{{ctx.Locale.Tr "repo.pulls.status_checks_hide_all"}}</button>
19-
</div>
15+
data-hide-all="{{ctx.Locale.Tr "repo.pulls.status_checks_hide_all"}}"
16+
>{{ctx.Locale.Tr "repo.pulls.status_checks_hide_all"}}</button>
2017
{{end}}
2118
</div>
22-
19+
{{end}}
2320
{{if and $statusCheckData $statusCheckData.RequireApprovalRunCount}}
24-
<div class="ui attached segment flex-left-right" id="approve-status-checks">
21+
<div class="item flex-left-right" id="approve-status-checks">
2522
<div>
2623
<strong>
2724
{{ctx.Locale.Tr "repo.pulls.status_checks_need_approvals" $statusCheckData.RequireApprovalRunCount}}
@@ -36,28 +33,31 @@
3633
</div>
3734
{{end}}
3835

39-
<div class="commit-status-list">
36+
<div class="item flex-divided-list commit-status-list">
4037
{{range .CommitStatuses}}
41-
<div class="commit-status-item">
42-
{{template "repo/commit_status" .}}
43-
<div class="status-context gt-ellipsis">{{.Context}} <span class="tw-text-text-light-2">{{.Description}}</span></div>
44-
<div class="ui status-details">
38+
<div class="item commit-status-item">
39+
<div class="flex-text-block">
40+
{{template "repo/commit_status" .}}
41+
<div class="status-context gt-ellipsis">{{.Context}} <span class="tw-text-text-light-2">{{.Description}}</span></div>
42+
</div>
43+
<div class="status-details">
4544
{{if and $statusCheckData $statusCheckData.IsContextRequired}}
4645
{{if (call $statusCheckData.IsContextRequired .Context)}}<div class="ui label">{{ctx.Locale.Tr "repo.pulls.status_checks_requested"}}</div>{{end}}
4746
{{end}}
48-
<span>{{if .TargetURL}}<a href="{{.TargetURL}}">{{ctx.Locale.Tr "repo.pulls.status_checks_details"}}</a>{{end}}</span>
47+
{{if .TargetURL}}<a href="{{.TargetURL}}">{{ctx.Locale.Tr "repo.pulls.status_checks_details"}}</a>{{end}}
4948
</div>
5049
</div>
5150
{{end}}
5251
{{if $statusCheckData}}
5352
{{range $statusCheckData.MissingRequiredChecks}}
54-
<div class="commit-status-item">
55-
{{svg "octicon-dot-fill" 18 "commit-status icon tw-text-yellow"}}
56-
<div class="status-context gt-ellipsis">{{.}}</div>
53+
<div class="item commit-status-item">
54+
<div class="flex-text-block">
55+
{{svg "octicon-dot-fill" 16 "commit-status icon tw-text-yellow"}}
56+
<div class="status-context gt-ellipsis">{{.}}</div>
57+
</div>
5758
<div class="ui label">{{ctx.Locale.Tr "repo.pulls.status_checks_requested"}}</div>
5859
</div>
5960
{{end}}
6061
{{end}}
6162
</div>
62-
</div>
6363
{{end}}

‎tests/integration/api_actions_artifact_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func TestActionsArtifactDownload(t *testing.T) {
149149
assert.Contains(t, listResp.Value[artifactIdx].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts")
150150

151151
idx := strings.Index(listResp.Value[artifactIdx].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/")
152-
url := listResp.Value[artifactIdx].FileContainerResourceURL[idx+1:] + "?itemPath=artifact-download"
152+
url := listResp.Value[artifactIdx].FileContainerResourceURL[idx:] + "?itemPath=artifact-download"
153153
req = NewRequest(t, "GET", url).
154154
AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
155155
resp = MakeRequest(t, req, http.StatusOK)
@@ -245,7 +245,7 @@ func TestActionsArtifactDownloadMultiFiles(t *testing.T) {
245245
assert.Contains(t, fileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts")
246246

247247
idx := strings.Index(fileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/")
248-
url := fileContainerResourceURL[idx+1:] + "?itemPath=" + testArtifactName
248+
url := fileContainerResourceURL[idx:] + "?itemPath=" + testArtifactName
249249
req = NewRequest(t, "GET", url).
250250
AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
251251
resp = MakeRequest(t, req, http.StatusOK)
@@ -323,7 +323,7 @@ func TestActionsArtifactOverwrite(t *testing.T) {
323323
listResp := DecodeJSON(t, resp, &listArtifactsResponse{})
324324

325325
idx := strings.Index(listResp.Value[0].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/")
326-
url := listResp.Value[0].FileContainerResourceURL[idx+1:] + "?itemPath=artifact-download"
326+
url := listResp.Value[0].FileContainerResourceURL[idx:] + "?itemPath=artifact-download"
327327
req = NewRequest(t, "GET", url).
328328
AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
329329
resp = MakeRequest(t, req, http.StatusOK)
@@ -380,7 +380,7 @@ func TestActionsArtifactOverwrite(t *testing.T) {
380380
assert.Equal(t, "artifact-download", uploadedItem.Name)
381381

382382
idx := strings.Index(uploadedItem.FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/")
383-
url := uploadedItem.FileContainerResourceURL[idx+1:] + "?itemPath=artifact-download"
383+
url := uploadedItem.FileContainerResourceURL[idx:] + "?itemPath=artifact-download"
384384
req = NewRequest(t, "GET", url).
385385
AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
386386
resp = MakeRequest(t, req, http.StatusOK)

‎tests/integration/api_branch_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ func TestAPIUpdateBranchReference(t *testing.T) {
348348

349349
func testAPIRenameBranch(t *testing.T, doerName, ownerName, repoName, from, to string, expectedHTTPStatus int) *httptest.ResponseRecorder {
350350
token := getUserToken(t, doerName, auth_model.AccessTokenScopeWriteRepository)
351-
req := NewRequestWithJSON(t, "PATCH", "api/v1/repos/"+ownerName+"/"+repoName+"/branches/"+from, &api.RenameBranchRepoOption{
351+
req := NewRequestWithJSON(t, "PATCH", "/api/v1/repos/"+ownerName+"/"+repoName+"/branches/"+from, &api.RenameBranchRepoOption{
352352
Name: to,
353353
}).AddTokenAuth(token)
354354
return MakeRequest(t, req, expectedHTTPStatus)

‎tests/integration/attachment_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,18 +96,18 @@ func testUploadAttachmentDeleteTemp(t *testing.T) {
9696
defer web.RouteMock(route_web.RouterMockPointBeforeWebRoutes, func(resp http.ResponseWriter, req *http.Request) {
9797
tmpFileCountDuringUpload = countTmpFile()
9898
})()
99-
_ = testCreateIssueAttachment(t, session, "user2/repo1", "image.png", testGeneratePngBytes(), http.StatusOK)
99+
_ = testCreateIssueAttachment(t, session, "/user2/repo1", "image.png", testGeneratePngBytes(), http.StatusOK)
100100
assert.Equal(t, 1, tmpFileCountDuringUpload, "the temp file should exist when uploaded size exceeds the parse form's max memory")
101101
assert.Equal(t, 0, countTmpFile(), "the temp file should be deleted after upload")
102102
}
103103

104104
func testCreateAnonymousAttachment(t *testing.T) {
105105
session := emptyTestSession(t)
106-
testCreateIssueAttachment(t, session, "user2/repo1", "image.png", testGeneratePngBytes(), http.StatusSeeOther)
106+
testCreateIssueAttachment(t, session, "/user2/repo1", "image.png", testGeneratePngBytes(), http.StatusSeeOther)
107107
}
108108

109109
func testCreateUser2IssueAttachment(t *testing.T) {
110-
const repoURL = "user2/repo1"
110+
const repoURL = "/user2/repo1"
111111
session := loginUser(t, "user2")
112112
uuid := testCreateIssueAttachment(t, session, repoURL, "image.png", testGeneratePngBytes(), http.StatusOK)
113113

@@ -177,7 +177,7 @@ func testGetAttachment(t *testing.T) {
177177
}
178178

179179
func testDeleteAttachmentPermissions(t *testing.T) {
180-
const repoURL = "user2/repo1"
180+
const repoURL = "/user2/repo1"
181181

182182
ownerSession := loginUser(t, "user2")
183183
readonlySession := loginUser(t, "user5")
@@ -191,12 +191,12 @@ func testDeleteAttachmentPermissions(t *testing.T) {
191191
testCreateReleaseAttachment(t, readonlySession, repoURL, "reader-release.png", testGeneratePngBytes(), http.StatusNotFound)
192192

193193
crossRepoUUID := testCreateIssueAttachment(t, ownerSession, repoURL, "cross-repo.png", testGeneratePngBytes(), http.StatusOK)
194-
testDeleteIssueAttachment(t, ownerSession, "user2/repo2", crossRepoUUID, http.StatusBadRequest)
194+
testDeleteIssueAttachment(t, ownerSession, "/user2/repo2", crossRepoUUID, http.StatusBadRequest)
195195
testDeleteIssueAttachment(t, ownerSession, repoURL, crossRepoUUID, http.StatusOK)
196196

197197
releaseUUID := testCreateReleaseAttachment(t, ownerSession, repoURL, "reader-release.png", testGeneratePngBytes(), http.StatusOK)
198198
testDeleteReleaseAttachment(t, ownerSession, repoURL, releaseUUID, http.StatusOK)
199199

200200
// test deleting release attachment from another repo
201-
testDeleteReleaseAttachment(t, ownerSession, "user2/repo2", crossRepoUUID, http.StatusBadRequest)
201+
testDeleteReleaseAttachment(t, ownerSession, "/user2/repo2", crossRepoUUID, http.StatusBadRequest)
202202
}

0 commit comments

Comments
 (0)