Skip to content

Refactor pull-request compare&create page #33071

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 1, 2025
Merged

Refactor pull-request compare&create page #33071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 45 additions & 46 deletions templates/repo/diff/compare.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{{template "base/head" .}}
<div role="main" aria-label="{{.Title}}" class="page-content repository diff {{if .PageIsComparePull}}compare pull{{end}}">
{{template "repo/header" .}}
{{$showDiffBox := false}}

<div class="ui container fluid padded">
<h2 class="ui header">
{{if and $.PageIsComparePull $.IsSigned (not .Repository.IsArchived)}}
Expand All @@ -27,9 +27,10 @@
{{- $HeadCompareName = printf "%s/%s" $.HeadRepo.OwnerName $.HeadRepo.Name -}}
{{- end -}}
{{- end -}}

<div class="ui segment choose branch">
<a class="tw-mr-2" href="{{$.HeadRepo.Link}}/compare/{{PathEscapeSegments $.HeadBranch}}{{$.CompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.BaseName}}/{{PathEscape $.Repository.Name}}:{{end}}{{PathEscapeSegments $.BaseBranch}}" title="{{ctx.Locale.Tr "repo.pulls.switch_head_and_base"}}">{{svg "octicon-git-compare"}}</a>
<div class="ui floating filter dropdown" data-no-results="{{ctx.Locale.Tr "no_results_found"}}">
<div class="ui dropdown jump select-branch">
<div class="ui basic small button">
<span class="text">{{if $.PageIsComparePull}}{{ctx.Locale.Tr "repo.pulls.compare_base"}}{{else}}{{ctx.Locale.Tr "repo.compare.compare_base"}}{{end}}: {{$BaseCompareName}}:{{$.BaseBranch}}</span>
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
Expand All @@ -44,61 +45,63 @@
<div class="two column row">
<a class="reference column" href="#" data-target=".base-branch-list">
<span class="text black">
{{svg "octicon-git-branch" 16 "tw-mr-1"}}{{ctx.Locale.Tr "repo.branches"}}
{{svg "octicon-git-branch"}} {{ctx.Locale.Tr "repo.branches"}}
</span>
</a>
<a class="reference column" href="#" data-target=".base-tag-list">
<span class="text black">
{{svg "octicon-tag" 16 "tw-mr-1"}}{{ctx.Locale.Tr "repo.tags"}}
{{svg "octicon-tag"}} {{ctx.Locale.Tr "repo.tags"}}
</span>
</a>
</div>
</div>
</div>
<div class="scrolling menu reference-list-menu base-branch-list">
{{range .Branches}}
<div class="item {{if eq $.BaseBranch .}}selected{{end}}" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{end}}{{PathEscapeSegments $.HeadBranch}}">{{$BaseCompareName}}:{{.}}</div>
<a class="item {{if eq $.BaseBranch .}}selected{{end}}" href="{{$.RepoLink}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{end}}{{PathEscapeSegments $.HeadBranch}}">{{$BaseCompareName}}:{{.}}</a>
{{end}}
{{if not .PullRequestCtx.SameRepo}}
{{range .HeadBranches}}
<div class="item" data-url="{{$.HeadRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$HeadCompareName}}:{{.}}</div>
<a class="item" href="{{$.HeadRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$HeadCompareName}}:{{.}}</a>
{{end}}
{{end}}
{{if .OwnForkRepo}}
{{range .OwnForkRepoBranches}}
<div class="item" data-url="{{$.OwnForkRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$OwnForkCompareName}}:{{.}}</div>
<a class="item" href="{{$.OwnForkRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$OwnForkCompareName}}:{{.}}</a>
{{end}}
{{end}}
{{if and .RootRepo (.RootRepo.AllowsPulls ctx)}}
{{range .RootRepoBranches}}
<div class="item" data-url="{{$.RootRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$RootRepoCompareName}}:{{.}}</div>
<a class="item" href="{{$.RootRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$RootRepoCompareName}}:{{.}}</a>
{{end}}
{{end}}
</div>
<div class="scrolling menu reference-list-menu base-tag-list tw-hidden">
{{range .Tags}}
<div class="item {{if eq $.BaseBranch .}}selected{{end}}" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{end}}{{PathEscapeSegments $.HeadBranch}}">{{$BaseCompareName}}:{{.}}</div>
<a class="item {{if eq $.BaseBranch .}}selected{{end}}" href="{{$.RepoLink}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{end}}{{PathEscapeSegments $.HeadBranch}}">{{$BaseCompareName}}:{{.}}</a>
{{end}}
{{if not .PullRequestCtx.SameRepo}}
{{range .HeadTags}}
<div class="item" data-url="{{$.HeadRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$HeadCompareName}}:{{.}}</div>
<a class="item" href="{{$.HeadRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$HeadCompareName}}:{{.}}</a>
{{end}}
{{end}}
{{if .OwnForkRepo}}
{{range .OwnForkRepoTags}}
<div class="item" data-url="{{$.OwnForkRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$OwnForkCompareName}}:{{.}}</div>
<a class="item" href="{{$.OwnForkRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$OwnForkCompareName}}:{{.}}</a>
{{end}}
{{end}}
{{if .RootRepo}}
{{range .RootRepoTags}}
<div class="item" data-url="{{$.RootRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$RootRepoCompareName}}:{{.}}</div>
<a class="item" href="{{$.RootRepo.Link}}/compare/{{PathEscapeSegments .}}{{$.CompareSeparator}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{PathEscapeSegments $.HeadBranch}}">{{$RootRepoCompareName}}:{{.}}</a>
{{end}}
{{end}}
</div>
</div>
</div>

<a href="{{.RepoLink}}/compare/{{PathEscapeSegments .BaseBranch}}{{.OtherCompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{end}}{{PathEscapeSegments $.HeadBranch}}" title="{{ctx.Locale.Tr "repo.pulls.switch_comparison_type"}}">{{svg "octicon-arrow-left" 16}}<div class="compare-separator">{{.CompareSeparator}}</div></a>
<div class="ui floating filter dropdown">

<div class="ui dropdown jump select-branch">
<div class="ui basic small button">
<span class="text">{{if $.PageIsComparePull}}{{ctx.Locale.Tr "repo.pulls.compare_compare"}}{{else}}{{ctx.Locale.Tr "repo.compare.compare_head"}}{{end}}: {{$HeadCompareName}}:{{$.HeadBranch}}</span>
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
Expand All @@ -113,76 +116,75 @@
<div class="two column row">
<a class="reference column" href="#" data-target=".head-branch-list">
<span class="text black">
{{svg "octicon-git-branch" 16 "tw-mr-1"}}{{ctx.Locale.Tr "repo.branches"}}
{{svg "octicon-git-branch"}} {{ctx.Locale.Tr "repo.branches"}}
</span>
</a>
<a class="reference column" href="#" data-target=".head-tag-list">
<span class="text black">
{{svg "octicon-tag" 16 "tw-mr-1"}}{{ctx.Locale.Tr "repo.tags"}}
{{svg "octicon-tag"}} {{ctx.Locale.Tr "repo.tags"}}
</span>
</a>
</div>
</div>
</div>
<div class="scrolling menu reference-list-menu head-branch-list">
{{range .HeadBranches}}
<div class="{{if eq $.HeadBranch .}}selected{{end}} item" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{end}}{{PathEscapeSegments .}}">{{$HeadCompareName}}:{{.}}</div>
<a class="{{if eq $.HeadBranch .}}selected{{end}} item" href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{end}}{{PathEscapeSegments .}}">{{$HeadCompareName}}:{{.}}</a>
{{end}}
{{if not .PullRequestCtx.SameRepo}}
{{range .Branches}}
<div class="item" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.BaseName}}/{{PathEscape $.Repository.Name}}:{{PathEscapeSegments .}}">{{$BaseCompareName}}:{{.}}</div>
<a class="item" href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.BaseName}}/{{PathEscape $.Repository.Name}}:{{PathEscapeSegments .}}">{{$BaseCompareName}}:{{.}}</a>
{{end}}
{{end}}
{{if .OwnForkRepo}}
{{range .OwnForkRepoBranches}}
<div class="item" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.OwnForkRepo.OwnerName}}/{{PathEscape $.OwnForkRepo.Name}}:{{PathEscapeSegments .}}">{{$OwnForkCompareName}}:{{.}}</div>
<a class="item" href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.OwnForkRepo.OwnerName}}/{{PathEscape $.OwnForkRepo.Name}}:{{PathEscapeSegments .}}">{{$OwnForkCompareName}}:{{.}}</a>
{{end}}
{{end}}
{{if .RootRepo}}
{{range .RootRepoBranches}}
<div class="item" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.RootRepo.OwnerName}}/{{PathEscape $.RootRepo.Name}}:{{PathEscapeSegments .}}">{{$RootRepoCompareName}}:{{.}}</div>
<a class="item" href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.RootRepo.OwnerName}}/{{PathEscape $.RootRepo.Name}}:{{PathEscapeSegments .}}">{{$RootRepoCompareName}}:{{.}}</a>
{{end}}
{{end}}
</div>
<div class="scrolling menu reference-list-menu head-tag-list tw-hidden">
{{range .HeadTags}}
<div class="{{if eq $.HeadBranch .}}selected{{end}} item" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{end}}{{PathEscapeSegments .}}">{{$HeadCompareName}}:{{.}}</div>
<a class="{{if eq $.HeadBranch .}}selected{{end}} item" href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{if not $.PullRequestCtx.SameRepo}}{{PathEscape $.HeadUser.Name}}/{{PathEscape $.HeadRepo.Name}}:{{end}}{{PathEscapeSegments .}}">{{$HeadCompareName}}:{{.}}</a>
{{end}}
{{if not .PullRequestCtx.SameRepo}}
{{range .Tags}}
<div class="item" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.BaseName}}/{{PathEscape $.Repository.Name}}:{{PathEscapeSegments .}}">{{$BaseCompareName}}:{{.}}</div>
<a class="item" href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.BaseName}}/{{PathEscape $.Repository.Name}}:{{PathEscapeSegments .}}">{{$BaseCompareName}}:{{.}}</a>
{{end}}
{{end}}
{{if .OwnForkRepo}}
{{range .OwnForkRepoTags}}
<div class="item" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.OwnForkRepo.OwnerName}}/{{PathEscape $.OwnForkRepo.Name}}:{{PathEscapeSegments .}}">{{$OwnForkCompareName}}:{{.}}</div>
<a class="item" href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.OwnForkRepo.OwnerName}}/{{PathEscape $.OwnForkRepo.Name}}:{{PathEscapeSegments .}}">{{$OwnForkCompareName}}:{{.}}</a>
{{end}}
{{end}}
{{if .RootRepo}}
{{range .RootRepoTags}}
<div class="item" data-url="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.RootRepo.OwnerName}}/{{PathEscape $.RootRepo.Name}}:{{PathEscapeSegments .}}">{{$RootRepoCompareName}}:{{.}}</div>
<a class="item" href="{{$.RepoLink}}/compare/{{PathEscapeSegments $.BaseBranch}}{{$.CompareSeparator}}{{PathEscape $.RootRepo.OwnerName}}/{{PathEscape $.RootRepo.Name}}:{{PathEscapeSegments .}}">{{$RootRepoCompareName}}:{{.}}</a>
{{end}}
{{end}}
</div>
</div>
</div>
</div>

{{$showDiffBox := and .CommitCount (not .IsNothingToCompare)}}
{{if and .IsSigned .PageIsComparePull}}
{{$allowCreatePR := or $.AllowEmptyPr (not .IsNothingToCompare)}}
{{if .IsNothingToCompare}}
{{if and $.IsSigned $.AllowEmptyPr (not .Repository.IsArchived) .PageIsComparePull}}
<div class="ui segment">{{ctx.Locale.Tr "repo.pulls.nothing_to_compare_and_allow_empty_pr"}}</div>
<div class="ui info message show-form-container {{if .Flash}}tw-hidden{{end}}">
<button class="ui button primary show-form">{{ctx.Locale.Tr "repo.pulls.new"}}</button>
</div>
<div class="pullrequest-form {{if not .Flash}}tw-hidden{{end}}">
{{template "repo/issue/new_form" .}}
</div>
<div class="ui segment">
{{if $allowCreatePR}}
{{ctx.Locale.Tr "repo.pulls.nothing_to_compare_and_allow_empty_pr"}}
{{else if and .HeadIsBranch .BaseIsBranch}}
<div class="ui segment">{{ctx.Locale.Tr "repo.pulls.nothing_to_compare"}}</div>
{{ctx.Locale.Tr "repo.pulls.nothing_to_compare"}}
{{else}}
<div class="ui segment">{{ctx.Locale.Tr "repo.pulls.nothing_to_compare_have_tag"}}</div>
{{ctx.Locale.Tr "repo.pulls.nothing_to_compare_have_tag"}}
{{end}}
</div>
{{end}}
{{else if and .PageIsComparePull (gt .CommitCount 0)}}
{{if .HasPullRequest}}
<div class="ui segment flex-text-block tw-gap-4">
{{template "shared/issueicon" .}}
Expand All @@ -194,34 +196,31 @@
{{ctx.Locale.Tr "repo.pulls.view"}}
</a>
</div>
{{else}}
{{if and $.IsSigned (not .Repository.IsArchived)}}
<div class="ui info message show-form-container {{if .Flash}}tw-hidden{{end}}">
<button class="ui button primary show-form">{{ctx.Locale.Tr "repo.pulls.new"}}</button>
</div>
{{else if .Repository.IsArchived}}
<div class="ui warning message tw-text-center">
<div class="ui warning message">
{{if .Repository.ArchivedUnix.IsZero}}
{{ctx.Locale.Tr "repo.archive.title"}}
{{else}}
{{ctx.Locale.Tr "repo.archive.title_date" (DateUtils.AbsoluteLong .Repository.ArchivedUnix)}}
{{end}}
</div>
{{end}}
{{if $.IsSigned}}
{{else if $allowCreatePR}}
<div class="ui info message pullrequest-form-toggle {{if .Flash}}tw-hidden{{end}}">
<button class="ui button primary show-panel toggle" data-panel=".pullrequest-form-toggle, .pullrequest-form">{{ctx.Locale.Tr "repo.pulls.new"}}</button>
</div>
<div class="pullrequest-form {{if not .Flash}}tw-hidden{{end}}">
{{template "repo/issue/new_form" .}}
</div>
{{end}}
{{$showDiffBox = true}}
{{else}}{{/* not singed-in or not for pull-request */}}
{{if not .CommitCount}}
<div class="ui segment">{{ctx.Locale.Tr "repo.commits.nothing_to_compare"}}</div>
{{end}}
{{else if not .IsNothingToCompare}}
{{$showDiffBox = true}}
{{end}}
</div>

{{if $showDiffBox}}
<div class="ui container fluid padded">
<div class="ui container fluid padded tw-my-4">
{{template "repo/commits_table" .}}
{{template "repo/diff/box" .}}
</div>
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/compare_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestCompareTag(t *testing.T) {
req := NewRequest(t, "GET", "/user2/repo1/compare/v1.1...master")
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
selection := htmlDoc.doc.Find(".choose.branch .filter.dropdown")
selection := htmlDoc.doc.Find(".ui.dropdown.select-branch")
// A dropdown for both base and head.
assert.Lenf(t, selection.Nodes, 2, "The template has changed")

Expand All @@ -44,7 +44,7 @@ func TestCompareDefault(t *testing.T) {
req := NewRequest(t, "GET", "/user2/repo1/compare/v1.1")
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
selection := htmlDoc.doc.Find(".choose.branch .filter.dropdown")
selection := htmlDoc.doc.Find(".ui.dropdown.select-branch")
assert.Lenf(t, selection.Nodes, 2, "The template has changed")
}

Expand Down
9 changes: 0 additions & 9 deletions web_src/css/repo.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,10 +825,6 @@ td .commit-summary {
height: 10px;
}

.repository.compare.pull .show-form-container {
text-align: left;
}

.repository .choose.branch {
display: flex;
align-items: center;
Expand Down Expand Up @@ -866,11 +862,6 @@ td .commit-summary {
margin-top: -8px;
}

.repository.compare.pull .pullrequest-form {
margin-top: 16px;
margin-bottom: 16px;
}

.repository.compare.pull .markup {
font-size: 14px;
}
Expand Down
2 changes: 1 addition & 1 deletion web_src/js/features/repo-issue-sidebar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {initIssueSidebarComboList} from './repo-issue-sidebar-combolist.ts';

function initBranchSelector() {
// TODO: RemoveIssueRef: see "repo/issue/branch_selector_field.tmpl"
const elSelectBranch = document.querySelector('.ui.dropdown.select-branch');
const elSelectBranch = document.querySelector('.ui.dropdown.select-branch.branch-selector-dropdown');
if (!elSelectBranch) return;

const urlUpdateIssueRef = elSelectBranch.getAttribute('data-url-update-issueref');
Expand Down
Loading
Loading