-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Improve the list header in milestone page #27302
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.
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
Changes from 8 commits
7a171ec
1807546
c138aeb
a2e56a6
3471fd4
bc9d014
4ba4346
e8e37c8
8508daf
3977b02
d7416ed
e4d02b9
29ac78d
df69f0f
9cacb68
3ccb388
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -246,6 +246,12 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti | |
isShowClosed = true | ||
} | ||
|
||
archived := ctx.FormBool("archived") | ||
showArchivedLabels := "false" | ||
if archived { | ||
showArchivedLabels = "true" | ||
} | ||
|
||
page := ctx.FormInt("page") | ||
if page <= 1 { | ||
page = 1 | ||
|
@@ -417,6 +423,10 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti | |
ctx.Data["PinnedIssues"] = pinned | ||
ctx.Data["IsRepoAdmin"] = ctx.IsSigned && (ctx.Repo.IsAdmin() || ctx.Doer.IsAdmin) | ||
ctx.Data["IssueStats"] = issueStats | ||
ctx.Data["OpenCount"] = issueStats.OpenCount | ||
ctx.Data["ClosedCount"] = issueStats.ClosedCount | ||
ctx.Data["OpenLink"] = fmt.Sprintf("%s?q=%s&type=%s&sort=%s&state=open&labels=%s&milestone=%d&project=%d&assignee=%d&poster=%d&archived=%s", ctx.Link, keyword, viewType, sortType, selectLabels, mentionedID, projectID, assigneeID, posterID, showArchivedLabels) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I move this into the template? This solution seems not good enough. 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Lets move this in separat pr because there are many places where we can reduce and refactor this dedicately. I don't want to over burder this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I notice that there are too many similar links in different places of these templates. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of endlessly extending these query param lists, we should instead add a template helper that just merges the specified pairs into the current query params that the backend sees: #27192 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will make a proof of concept of this helper soon. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a blocker. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for keeping you reiterating |
||
ctx.Data["ClosedLink"] = fmt.Sprintf("%s?q=%s&type=%s&sort=%s&state=closed&labels=%s&milestone=%d&project=%d&assignee=%d&poster=%d&archived=%s", ctx.Link, keyword, viewType, sortType, selectLabels, mentionedID, projectID, assigneeID, posterID, showArchivedLabels) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. may be use can use queryStr := "%s?q=%s&type=%s&sort=%s&state=%s&labels=%s&milestone=%d&project=%d&assignee=%d&poster=%d&archived=%s";
// take state in expression "close/open"
ctx.Data["OpenLink"] = fmt.Sprintf(queryStr, "open",all the args here);
ctx.Data["CloseLink"] = fmt.Sprintf(queryStr, "close", all the args here); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
ctx.Data["SelLabelIDs"] = labelIDs | ||
ctx.Data["SelectLabels"] = selectLabels | ||
ctx.Data["ViewType"] = viewType | ||
|
@@ -432,6 +442,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti | |
} else { | ||
ctx.Data["State"] = "open" | ||
} | ||
ctx.Data["ShowArchivedLabels"] = archived | ||
|
||
pager.AddParam(ctx, "q", "Keyword") | ||
pager.AddParam(ctx, "type", "ViewType") | ||
|
@@ -442,11 +453,8 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti | |
pager.AddParam(ctx, "project", "ProjectID") | ||
pager.AddParam(ctx, "assignee", "AssigneeID") | ||
pager.AddParam(ctx, "poster", "PosterID") | ||
pager.AddParam(ctx, "archived", "ShowArchivedLabels") | ||
|
||
if ctx.FormBool("archived") { | ||
ctx.Data["ShowArchivedLabels"] = true | ||
pager.AddParam(ctx, "archived", "ShowArchivedLabels") | ||
} | ||
ctx.Data["Page"] = pager | ||
} | ||
|
||
|
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<!-- Sort --> | ||
<div class="list-header-sort ui small dropdown type jump item"> | ||
<span class="text"> | ||
{{ctx.Locale.Tr "repo.issues.filter_sort"}} | ||
</span> | ||
{{svg "octicon-triangle-down" 14 "dropdown icon"}} | ||
<div class="menu"> | ||
<a class="{{if or (eq .SortType "closestduedate") (not .SortType)}}active {{end}}item" href="{{$.Link}}?sort=closestduedate&state={{$.State}}&q={{$.Keyword}}">{{ctx.Locale.Tr "repo.milestones.filter_sort.earliest_due_data"}}</a> | ||
<a class="{{if eq .SortType "furthestduedate"}}active {{end}}item" href="{{$.Link}}?sort=furthestduedate&state={{$.State}}&q={{$.Keyword}}">{{ctx.Locale.Tr "repo.milestones.filter_sort.latest_due_date"}}</a> | ||
<a class="{{if eq .SortType "leastcomplete"}}active {{end}}item" href="{{$.Link}}?sort=leastcomplete&state={{$.State}}&q={{$.Keyword}}">{{ctx.Locale.Tr "repo.milestones.filter_sort.least_complete"}}</a> | ||
<a class="{{if eq .SortType "mostcomplete"}}active {{end}}item" href="{{$.Link}}?sort=mostcomplete&state={{$.State}}&q={{$.Keyword}}">{{ctx.Locale.Tr "repo.milestones.filter_sort.most_complete"}}</a> | ||
<a class="{{if eq .SortType "mostissues"}}active {{end}}item" href="{{$.Link}}?sort=mostissues&state={{$.State}}&q={{$.Keyword}}">{{ctx.Locale.Tr "repo.milestones.filter_sort.most_issues"}}</a> | ||
<a class="{{if eq .SortType "leastissues"}}active {{end}}item" href="{{$.Link}}?sort=leastissues&state={{$.State}}&q={{$.Keyword}}">{{ctx.Locale.Tr "repo.milestones.filter_sort.least_issues"}}</a> | ||
</div> | ||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,16 @@ | ||
<div class="small-menu-items ui compact tiny menu"> | ||
<a class="{{if not .IsShowClosed}}active {{end}}item" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state=open&labels={{.SelectLabels}}&milestone={{.MilestoneID}}&project={{.ProjectID}}&assignee={{.AssigneeID}}&poster={{.PosterID}}{{if $.ShowArchivedLabels}}&archived=true{{end}}"> | ||
{{if .PageIsPullList}} | ||
<a class="{{if not .IsShowClosed}}active {{end}}item" href="{{.OpenLink}}"> | ||
{{if .PageIsMilestones}} | ||
{{svg "octicon-milestone" 16 "gt-mr-3"}} | ||
{{else if .PageIsPullList}} | ||
{{svg "octicon-git-pull-request" 16 "gt-mr-3"}} | ||
{{else}} | ||
{{svg "octicon-issue-opened" 16 "gt-mr-3"}} | ||
{{end}} | ||
{{ctx.Locale.PrettyNumber .IssueStats.OpenCount}} {{ctx.Locale.Tr "repo.issues.open_title"}} | ||
{{ctx.Locale.PrettyNumber .OpenCount}} {{ctx.Locale.Tr "repo.issues.open_title"}} | ||
</a> | ||
<a class="{{if .IsShowClosed}}active {{end}}item" href="{{$.Link}}?q={{$.Keyword}}&type={{.ViewType}}&sort={{$.SortType}}&state=closed&labels={{.SelectLabels}}&milestone={{.MilestoneID}}&project={{.ProjectID}}&assignee={{.AssigneeID}}&poster={{.PosterID}}{{if $.ShowArchivedLabels}}&archived=true{{end}}"> | ||
<a class="{{if .IsShowClosed}}active {{end}}item" href="{{.ClosedLink}}"> | ||
{{svg "octicon-check" 16 "gt-mr-3"}} | ||
{{ctx.Locale.PrettyNumber .IssueStats.ClosedCount}} {{ctx.Locale.Tr "repo.issues.closed_title"}} | ||
{{ctx.Locale.PrettyNumber .ClosedCount}} {{ctx.Locale.Tr "repo.issues.closed_title"}} | ||
</a> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One little optimization.
Could you check what will be the value of
ctx.FormBool("archived")
if you don't pass into query params,I guess you don't need
showArchivedLabels
varable extra to save the state . You can usearchived
variable. directlyThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is "false", type is bool.
improved in 8508daf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use.strings
package .strings.FormatBool
This change was already done.