Improve the list header in milestone page #27302

yp05327 · 2023-09-27T05:00:19Z

The ui of list header in milestone page is not same as issue and pr list page.
And they are using different template codes which can be merged into one.

Before:

After:

lunny · 2023-09-27T05:17:36Z

Why should we move the search bar ?

yp05327 · 2023-09-27T07:39:38Z

Why should the search bar in different places?

delvh · 2023-09-30T08:39:22Z

templates/repo/issue/filters.tmpl

+			<!-- 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>


I recommend extracting all this into an extra subtemplate, simply to keep things readable.
At the moment, we use far too few subtemplates.

Also, the indentation of this block seems off. it's using 6 spaces, then 4 spaces. Should just be 2 spaces and no new indentation level after the comment.

Edit: actually lint fails on it as well.

silverwind · 2023-10-03T21:05:49Z

Change is good for consistency.

yp05327 · 2023-10-04T08:37:52Z

And in the projects list page, the search bar is also in the below.
I'm a little busy these days. I will take some times to improve this PR later.

templates/repo/issue/milestone/filter_list.tmpl

templates/repo/issue/search.tmpl

yp05327 · 2023-10-05T01:04:03Z

@puni9869
It seems that there's a bug from #27115.
When an archived label is selected, if you disabled showing archived labels, the selected archived labels will keep showing in the page.

Is this by design?

yp05327 · 2023-10-05T01:05:33Z

routers/web/repo/issue.go

@@ -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)


Should I move this into the template? This solution seems not good enough. 🤔

Should I move this into the template? This solution seems not good enough. 🤔

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.

I notice that there are too many similar links in different places of these templates.
If they can be a shared one, it can be more maintainable.

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)

Will make a proof of concept of this helper soon.

Co-authored-by: puni9869 <80308335+puni9869@users.noreply.github.com>

puni9869 · 2023-10-05T01:06:42Z

@yp05327 I appreciate this change 💯 , give me 1 hr or so for deep reivew.

@puni9869 It seems that there's a bug from #27115. When an archived label is selected, if you disabled showing archived labels, the selected archived labels will keep showing in the page. Is this by design?

Yes, its by design because the label is selected, if the checkbox is unchecked then the label info will be incomplete,

we want to show all labels which are selected.

puni9869 · 2023-10-05T01:11:38Z

@puni9869 It seems that there's a bug from #27115. When an archived label is selected, if you disabled showing archived labels, the selected archived labels will keep showing in the page. Is this by design?

@yp05327
// @delvh

I will blog post about the full functionality on the coming weekend.

There is currently work going on this functionality here #27451

I have one more enhancement in TODO: When we have some label selected after page reload we want to show all the selected labels in the top of the list, like github

All the selected labels are first and then sorted way listing of label in the selector drop down.

This will completes the archived label feature base functionality.

puni9869 · 2023-10-05T01:22:10Z

routers/web/repo/issue.go

+	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)
+	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)


same as above

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);

wxiaoguang · 2023-10-06T08:07:13Z

routers/web/repo/issue.go

+	ctx.Data["OpenCount"] = issueStats.OpenCount
+	ctx.Data["ClosedCount"] = issueStats.ClosedCount
+	linkStr := "%s?q=%s&type=%s&sort=%s&state=%s&labels=%s&milestone=%d&project=%d&assignee=%d&poster=%d&archived=%t"
+	ctx.Data["OpenLink"] = fmt.Sprintf(linkStr, ctx.Link, keyword, viewType, sortType, "open", selectLabels, mentionedID, projectID, assigneeID, posterID, archived)


It's not right to put a string into query parameter. You need to encode(escape) it.

Yes, needs queryEscape for each of these params. I still suggest we make a helper like #27192 (comment) instead of this cumbersome and error-prone string concatenation.

wxiaoguang

Please always encode parameters correctly.

puni9869 · 2023-10-06T08:23:16Z

Please always encode parameters correctly.

sry, we missed that.

puni9869 · 2023-10-10T06:09:15Z

routers/web/repo/issue.go

+	ctx.Data["OpenLink"] = fmt.Sprintf(linkStr, ctx.Link,
+		url.QueryEscape(keyword), url.QueryEscape(viewType), url.QueryEscape(sortType), "open", url.QueryEscape(selectLabels),
+		mentionedID, projectID, assigneeID, posterID, archived)
+	ctx.Data["ClosedLink"] = fmt.Sprintf(linkStr, ctx.Link,
+		url.QueryEscape(keyword), url.QueryEscape(viewType), url.QueryEscape(sortType), "closed", url.QueryEscape(selectLabels),
+		mentionedID, projectID, assigneeID, posterID, archived)


wxiaoguang · 2023-10-11T10:15:33Z

routers/web/repo/milestone.go

@@ -72,6 +73,9 @@ func Milestones(ctx *context.Context) {
 	}
 	ctx.Data["OpenCount"] = stats.OpenCount
 	ctx.Data["ClosedCount"] = stats.ClosedCount
+	linkStr := "%s/milestones?state=%s&q=%s&sort=%s"
+	ctx.Data["OpenLink"] = fmt.Sprintf(linkStr, ctx.Repo.RepoLink, "open", keyword, sortType)


It still needs correct escaping.

dismiss change request

silverwind

Escaping looks good. While I'm not a fan of the method, I guess we can always improve later with the proposed helper.

* giteaofficial/main: Improve the list header in milestone page (go-gitea#27302) Fix poster is not loaded in get default merge message (go-gitea#27657) Hide archived labels by default from the suggestions when assigning labels for an issue (go-gitea#27451) actions/setup-go use go-version-file (go-gitea#27651) Update agit-support.en-us.md (go-gitea#27652)

improve

7a171ec

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 27, 2023

pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 27, 2023

yp05327 added 2 commits September 27, 2023 16:43

Merge branch 'main' into improve-milestone-issue-header

1807546

fix locale usages

c138aeb

delvh approved these changes Sep 30, 2023

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 30, 2023

lunny added the topic/ui Change the appearance of the Gitea UI label Sep 30, 2023

lunny added this to the 1.22.0 milestone Sep 30, 2023

improve

a2e56a6

pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 5, 2023

github-actions bot removed the topic/ui Change the appearance of the Gitea UI label Oct 5, 2023

yp05327 added 2 commits October 5, 2023 09:38

Merge branch 'main' into improve-milestone-issue-header

3471fd4

apply changes in 27115

bc9d014

pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 5, 2023

yp05327 requested a review from puni9869 October 5, 2023 00:53

puni9869 reviewed Oct 5, 2023

View reviewed changes

templates/repo/issue/milestone/filter_list.tmpl Outdated Show resolved Hide resolved

puni9869 reviewed Oct 5, 2023

View reviewed changes

templates/repo/issue/search.tmpl Outdated Show resolved Hide resolved

yp05327 commented Oct 5, 2023

View reviewed changes

Update templates/repo/issue/milestone/filter_list.tmpl

4ba4346

Co-authored-by: puni9869 <80308335+puni9869@users.noreply.github.com>

puni9869 reviewed Oct 5, 2023

View reviewed changes

yp05327 added 2 commits October 5, 2023 01:35

fix lint

3977b02

improve

d7416ed

yp05327 force-pushed the improve-milestone-issue-header branch from 50a0ee9 to d7416ed Compare October 6, 2023 00:23

yp05327 added 2 commits October 6, 2023 00:26

improve

e4d02b9

Merge branch 'main' into improve-milestone-issue-header

29ac78d

puni9869 approved these changes Oct 6, 2023

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 6, 2023

wxiaoguang reviewed Oct 6, 2023

View reviewed changes

wxiaoguang previously requested changes Oct 6, 2023

View reviewed changes

GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Oct 6, 2023

add escape

df69f0f

puni9869 reviewed Oct 10, 2023

View reviewed changes

wxiaoguang reviewed Oct 11, 2023

View reviewed changes

add escape in milestone

9cacb68

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Oct 12, 2023

silverwind approved these changes Oct 17, 2023

View reviewed changes

silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 17, 2023

silverwind enabled auto-merge (squash) October 17, 2023 23:32

Merge branch 'main' into improve-milestone-issue-header

3ccb388

silverwind merged commit 8abc1aa into go-gitea:main Oct 18, 2023
25 checks passed

GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 18, 2023

yp05327 deleted the improve-milestone-issue-header branch October 18, 2023 06:03

go-gitea locked as resolved and limited conversation to collaborators Jan 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve the list header in milestone page #27302

Improve the list header in milestone page #27302

yp05327 commented Sep 27, 2023

lunny commented Sep 27, 2023

yp05327 commented Sep 27, 2023

delvh Sep 30, 2023

silverwind Oct 3, 2023 •

edited

Loading

silverwind commented Oct 3, 2023

yp05327 commented Oct 4, 2023

yp05327 commented Oct 5, 2023

yp05327 Oct 5, 2023

puni9869 Oct 5, 2023

yp05327 Oct 5, 2023

silverwind Oct 5, 2023 •

edited

Loading

silverwind Oct 6, 2023

puni9869 commented Oct 5, 2023 •

edited

Loading

puni9869 commented Oct 5, 2023

puni9869 Oct 5, 2023

puni9869 Oct 5, 2023 •

edited

Loading

yp05327 Oct 6, 2023

wxiaoguang Oct 6, 2023

silverwind Oct 6, 2023

wxiaoguang left a comment

puni9869 commented Oct 6, 2023

puni9869 Oct 10, 2023

wxiaoguang Oct 11, 2023

silverwind left a comment

Improve the list header in milestone page #27302

Improve the list header in milestone page #27302

Conversation

yp05327 commented Sep 27, 2023

lunny commented Sep 27, 2023

yp05327 commented Sep 27, 2023

Choose a reason for hiding this comment

silverwind Oct 3, 2023 • edited Loading

Choose a reason for hiding this comment

silverwind commented Oct 3, 2023

yp05327 commented Oct 4, 2023

yp05327 commented Oct 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

silverwind Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

puni9869 commented Oct 5, 2023 • edited Loading

puni9869 commented Oct 5, 2023

Choose a reason for hiding this comment

puni9869 Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wxiaoguang left a comment

Choose a reason for hiding this comment

puni9869 commented Oct 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

silverwind left a comment

Choose a reason for hiding this comment

silverwind Oct 3, 2023 •

edited

Loading

silverwind Oct 5, 2023 •

edited

Loading

puni9869 commented Oct 5, 2023 •

edited

Loading

puni9869 Oct 5, 2023 •

edited

Loading