From 58c634b8549fb279aec72cecd6a48511803db067 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 10 Nov 2024 16:26:42 +0800 Subject: [PATCH] Refactor sidebar label selector (#32460) Introduce `issueSidebarLabelsData` to handle all sidebar labels related data. --- routers/web/repo/compare.go | 15 +- routers/web/repo/issue.go | 181 ++++++++++-------- routers/web/repo/issue_label.go | 6 +- routers/web/repo/issue_label_test.go | 2 +- routers/web/web.go | 2 +- templates/repo/issue/labels/label.tmpl | 7 - .../issue/labels/labels_selector_field.tmpl | 46 ----- .../repo/issue/labels/labels_sidebar.tmpl | 11 -- templates/repo/issue/new_form.tmpl | 16 +- templates/repo/issue/sidebar/label_list.tmpl | 51 +++++ .../repo/issue/sidebar/label_list_item.tmpl | 11 ++ .../repo/issue/sidebar/reviewer_list.tmpl | 16 +- .../repo/issue/view_content/sidebar.tmpl | 5 +- web_src/css/repo.css | 8 +- web_src/css/repo/issue-label.css | 5 +- web_src/js/features/common-page.ts | 4 +- .../features/repo-issue-sidebar-combolist.ts | 46 +++-- web_src/js/features/repo-issue-sidebar.md | 27 +++ web_src/js/features/repo-issue-sidebar.ts | 20 +- web_src/js/features/repo-issue.ts | 15 +- web_src/js/index.ts | 3 +- web_src/js/utils/dom.ts | 10 +- 22 files changed, 275 insertions(+), 232 deletions(-) delete mode 100644 templates/repo/issue/labels/label.tmpl delete mode 100644 templates/repo/issue/labels/labels_selector_field.tmpl delete mode 100644 templates/repo/issue/labels/labels_sidebar.tmpl create mode 100644 templates/repo/issue/sidebar/label_list.tmpl create mode 100644 templates/repo/issue/sidebar/label_list_item.tmpl create mode 100644 web_src/js/features/repo-issue-sidebar.md diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 3477ba36e80..9a7d3dfbf65 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -788,7 +788,11 @@ func CompareDiff(ctx *context.Context) { if !nothingToCompare { // Setup information for new form. - RetrieveRepoMetas(ctx, ctx.Repo.Repository, true) + retrieveRepoMetasForIssueWriter(ctx, ctx.Repo.Repository, true) + if ctx.Written() { + return + } + labelsData := retrieveRepoLabels(ctx, ctx.Repo.Repository, 0, true) if ctx.Written() { return } @@ -796,6 +800,10 @@ func CompareDiff(ctx *context.Context) { if ctx.Written() { return } + _, templateErrs := setTemplateIfExists(ctx, pullRequestTemplateKey, pullRequestTemplateCandidates, labelsData) + if len(templateErrs) > 0 { + ctx.Flash.Warning(renderErrorOfTemplates(ctx, templateErrs), true) + } } } beforeCommitID := ctx.Data["BeforeCommitID"].(string) @@ -808,11 +816,6 @@ func CompareDiff(ctx *context.Context) { ctx.Data["Title"] = "Comparing " + base.ShortSha(beforeCommitID) + separator + base.ShortSha(afterCommitID) ctx.Data["IsDiffCompare"] = true - _, templateErrs := setTemplateIfExists(ctx, pullRequestTemplateKey, pullRequestTemplateCandidates) - - if len(templateErrs) > 0 { - ctx.Flash.Warning(renderErrorOfTemplates(ctx, templateErrs), true) - } if content, ok := ctx.Data["content"].(string); ok && content != "" { // If a template content is set, prepend the "content". In this case that's only diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 7fa8d428d36..a4e2fd8cea3 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -870,51 +870,112 @@ func RetrieveRepoReviewers(ctx *context.Context, repo *repo_model.Repository, is ctx.Data["IssueSidebarReviewersData"] = data } -// RetrieveRepoMetas find all the meta information of a repository -func RetrieveRepoMetas(ctx *context.Context, repo *repo_model.Repository, isPull bool) []*issues_model.Label { - if !ctx.Repo.CanWriteIssuesOrPulls(isPull) { - return nil +type issueSidebarLabelsData struct { + Repository *repo_model.Repository + RepoLink string + IssueID int64 + IsPullRequest bool + AllLabels []*issues_model.Label + RepoLabels []*issues_model.Label + OrgLabels []*issues_model.Label + SelectedLabelIDs string +} + +func makeSelectedStringIDs[KeyType, ItemType comparable]( + allLabels []*issues_model.Label, candidateKey func(candidate *issues_model.Label) KeyType, + selectedItems []ItemType, selectedKey func(selected ItemType) KeyType, +) string { + selectedIDSet := make(container.Set[string]) + allLabelMap := map[KeyType]*issues_model.Label{} + for _, label := range allLabels { + allLabelMap[candidateKey(label)] = label + } + for _, item := range selectedItems { + if label, ok := allLabelMap[selectedKey(item)]; ok { + label.IsChecked = true + selectedIDSet.Add(strconv.FormatInt(label.ID, 10)) + } + } + ids := selectedIDSet.Values() + sort.Strings(ids) + return strings.Join(ids, ",") +} + +func (d *issueSidebarLabelsData) SetSelectedLabels(labels []*issues_model.Label) { + d.SelectedLabelIDs = makeSelectedStringIDs( + d.AllLabels, func(label *issues_model.Label) int64 { return label.ID }, + labels, func(label *issues_model.Label) int64 { return label.ID }, + ) +} + +func (d *issueSidebarLabelsData) SetSelectedLabelNames(labelNames []string) { + d.SelectedLabelIDs = makeSelectedStringIDs( + d.AllLabels, func(label *issues_model.Label) string { return strings.ToLower(label.Name) }, + labelNames, strings.ToLower, + ) +} + +func (d *issueSidebarLabelsData) SetSelectedLabelIDs(labelIDs []int64) { + d.SelectedLabelIDs = makeSelectedStringIDs( + d.AllLabels, func(label *issues_model.Label) int64 { return label.ID }, + labelIDs, func(labelID int64) int64 { return labelID }, + ) +} + +func retrieveRepoLabels(ctx *context.Context, repo *repo_model.Repository, issueID int64, isPull bool) *issueSidebarLabelsData { + labelsData := &issueSidebarLabelsData{ + Repository: repo, + RepoLink: ctx.Repo.RepoLink, + IssueID: issueID, + IsPullRequest: isPull, } + ctx.Data["IssueSidebarLabelsData"] = labelsData labels, err := issues_model.GetLabelsByRepoID(ctx, repo.ID, "", db.ListOptions{}) if err != nil { ctx.ServerError("GetLabelsByRepoID", err) return nil } - ctx.Data["Labels"] = labels + labelsData.RepoLabels = labels + if repo.Owner.IsOrganization() { orgLabels, err := issues_model.GetLabelsByOrgID(ctx, repo.Owner.ID, ctx.FormString("sort"), db.ListOptions{}) if err != nil { return nil } + labelsData.OrgLabels = orgLabels + } + labelsData.AllLabels = append(labelsData.AllLabels, labelsData.RepoLabels...) + labelsData.AllLabels = append(labelsData.AllLabels, labelsData.OrgLabels...) + return labelsData +} - ctx.Data["OrgLabels"] = orgLabels - labels = append(labels, orgLabels...) +// retrieveRepoMetasForIssueWriter finds some the meta information of a repository for an issue/pr writer +func retrieveRepoMetasForIssueWriter(ctx *context.Context, repo *repo_model.Repository, isPull bool) { + if !ctx.Repo.CanWriteIssuesOrPulls(isPull) { + return } RetrieveRepoMilestonesAndAssignees(ctx, repo) if ctx.Written() { - return nil + return } retrieveProjects(ctx, repo) if ctx.Written() { - return nil + return } PrepareBranchList(ctx) if ctx.Written() { - return nil + return } - // Contains true if the user can create issue dependencies ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx, ctx.Doer, isPull) - - return labels } // Tries to load and set an issue template. The first return value indicates if a template was loaded. -func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles []string) (bool, map[string]error) { +func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles []string, labelsData *issueSidebarLabelsData) (bool, map[string]error) { commit, err := ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch) if err != nil { return false, nil @@ -951,26 +1012,9 @@ func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles ctx.Data["Fields"] = template.Fields ctx.Data["TemplateFile"] = template.FileName } - labelIDs := make([]string, 0, len(template.Labels)) - if repoLabels, err := issues_model.GetLabelsByRepoID(ctx, ctx.Repo.Repository.ID, "", db.ListOptions{}); err == nil { - ctx.Data["Labels"] = repoLabels - if ctx.Repo.Owner.IsOrganization() { - if orgLabels, err := issues_model.GetLabelsByOrgID(ctx, ctx.Repo.Owner.ID, ctx.FormString("sort"), db.ListOptions{}); err == nil { - ctx.Data["OrgLabels"] = orgLabels - repoLabels = append(repoLabels, orgLabels...) - } - } - for _, metaLabel := range template.Labels { - for _, repoLabel := range repoLabels { - if strings.EqualFold(repoLabel.Name, metaLabel) { - repoLabel.IsChecked = true - labelIDs = append(labelIDs, strconv.FormatInt(repoLabel.ID, 10)) - break - } - } - } - } + labelsData.SetSelectedLabelNames(template.Labels) + selectedAssigneeIDs := make([]int64, 0, len(template.Assignees)) selectedAssigneeIDStrings := make([]string, 0, len(template.Assignees)) if userIDs, err := user_model.GetUserIDsByNames(ctx, template.Assignees, false); err == nil { @@ -983,8 +1027,7 @@ func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles if template.Ref != "" && !strings.HasPrefix(template.Ref, "refs/") { // Assume that the ref intended is always a branch - for tags users should use refs/tags/ template.Ref = git.BranchPrefix + template.Ref } - ctx.Data["HasSelectedLabel"] = len(labelIDs) > 0 - ctx.Data["label_ids"] = strings.Join(labelIDs, ",") + ctx.Data["HasSelectedAssignee"] = len(selectedAssigneeIDs) > 0 ctx.Data["assignee_ids"] = strings.Join(selectedAssigneeIDStrings, ",") ctx.Data["SelectedAssigneeIDs"] = selectedAssigneeIDs @@ -1042,8 +1085,14 @@ func NewIssue(ctx *context.Context) { } } - RetrieveRepoMetas(ctx, ctx.Repo.Repository, false) - + retrieveRepoMetasForIssueWriter(ctx, ctx.Repo.Repository, false) + if ctx.Written() { + return + } + labelsData := retrieveRepoLabels(ctx, ctx.Repo.Repository, 0, false) + if ctx.Written() { + return + } tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID) if err != nil { ctx.ServerError("GetTagNamesByRepoID", err) @@ -1052,7 +1101,7 @@ func NewIssue(ctx *context.Context) { ctx.Data["Tags"] = tags ret := issue_service.ParseTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo) - templateLoaded, errs := setTemplateIfExists(ctx, issueTemplateKey, IssueTemplateCandidates) + templateLoaded, errs := setTemplateIfExists(ctx, issueTemplateKey, IssueTemplateCandidates, labelsData) for k, v := range errs { ret.TemplateErrors[k] = v } @@ -1161,34 +1210,25 @@ func ValidateRepoMetas(ctx *context.Context, form forms.CreateIssueForm, isPull err error ) - labels := RetrieveRepoMetas(ctx, ctx.Repo.Repository, isPull) + retrieveRepoMetasForIssueWriter(ctx, ctx.Repo.Repository, isPull) + if ctx.Written() { + return ret + } + labelsData := retrieveRepoLabels(ctx, ctx.Repo.Repository, 0, isPull) if ctx.Written() { return ret } var labelIDs []int64 - hasSelected := false // Check labels. if len(form.LabelIDs) > 0 { labelIDs, err = base.StringsToInt64s(strings.Split(form.LabelIDs, ",")) if err != nil { return ret } - labelIDMark := make(container.Set[int64]) - labelIDMark.AddMultiple(labelIDs...) - - for i := range labels { - if labelIDMark.Contains(labels[i].ID) { - labels[i].IsChecked = true - hasSelected = true - } - } + labelsData.SetSelectedLabelIDs(labelIDs) } - ctx.Data["Labels"] = labels - ctx.Data["HasSelectedLabel"] = hasSelected - ctx.Data["label_ids"] = form.LabelIDs - // Check milestone. milestoneID := form.MilestoneID if milestoneID > 0 { @@ -1579,38 +1619,15 @@ func ViewIssue(ctx *context.Context) { } } - // Metas. - // Check labels. - labelIDMark := make(container.Set[int64]) - for _, label := range issue.Labels { - labelIDMark.Add(label.ID) - } - labels, err := issues_model.GetLabelsByRepoID(ctx, repo.ID, "", db.ListOptions{}) - if err != nil { - ctx.ServerError("GetLabelsByRepoID", err) + retrieveRepoMetasForIssueWriter(ctx, repo, issue.IsPull) + if ctx.Written() { return } - ctx.Data["Labels"] = labels - - if repo.Owner.IsOrganization() { - orgLabels, err := issues_model.GetLabelsByOrgID(ctx, repo.Owner.ID, ctx.FormString("sort"), db.ListOptions{}) - if err != nil { - ctx.ServerError("GetLabelsByOrgID", err) - return - } - ctx.Data["OrgLabels"] = orgLabels - - labels = append(labels, orgLabels...) - } - - hasSelected := false - for i := range labels { - if labelIDMark.Contains(labels[i].ID) { - labels[i].IsChecked = true - hasSelected = true - } + labelsData := retrieveRepoLabels(ctx, repo, issue.ID, issue.IsPull) + if ctx.Written() { + return } - ctx.Data["HasSelectedLabel"] = hasSelected + labelsData.SetSelectedLabels(issue.Labels) // Check milestone and assignee. if ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { diff --git a/routers/web/repo/issue_label.go b/routers/web/repo/issue_label.go index 81bee4dbb53..4874baaa543 100644 --- a/routers/web/repo/issue_label.go +++ b/routers/web/repo/issue_label.go @@ -53,11 +53,11 @@ func InitializeLabels(ctx *context.Context) { ctx.Redirect(ctx.Repo.RepoLink + "/labels") } -// RetrieveLabels find all the labels of a repository and organization -func RetrieveLabels(ctx *context.Context) { +// RetrieveLabelsForList find all the labels of a repository and organization, it is only used by "/labels" page to list all labels +func RetrieveLabelsForList(ctx *context.Context) { labels, err := issues_model.GetLabelsByRepoID(ctx, ctx.Repo.Repository.ID, ctx.FormString("sort"), db.ListOptions{}) if err != nil { - ctx.ServerError("RetrieveLabels.GetLabels", err) + ctx.ServerError("RetrieveLabelsForList.GetLabels", err) return } diff --git a/routers/web/repo/issue_label_test.go b/routers/web/repo/issue_label_test.go index 93fc72300b3..c86a03da51a 100644 --- a/routers/web/repo/issue_label_test.go +++ b/routers/web/repo/issue_label_test.go @@ -62,7 +62,7 @@ func TestRetrieveLabels(t *testing.T) { contexttest.LoadUser(t, ctx, 2) contexttest.LoadRepo(t, ctx, testCase.RepoID) ctx.Req.Form.Set("sort", testCase.Sort) - RetrieveLabels(ctx) + RetrieveLabelsForList(ctx) assert.False(t, ctx.Written()) labels, ok := ctx.Data["Labels"].([]*issues_model.Label) assert.True(t, ok) diff --git a/routers/web/web.go b/routers/web/web.go index 29dd8a8edcb..907bf88f6f7 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1163,7 +1163,7 @@ func registerRoutes(m *web.Router) { m.Get("/issues/posters", repo.IssuePosters) // it can't use {type:issues|pulls} because it would conflict with other routes like "/pulls/{index}" m.Get("/pulls/posters", repo.PullPosters) m.Get("/comments/{id}/attachments", repo.GetCommentAttachments) - m.Get("/labels", repo.RetrieveLabels, repo.Labels) + m.Get("/labels", repo.RetrieveLabelsForList, repo.Labels) m.Get("/milestones", repo.Milestones) m.Get("/milestone/{id}", context.RepoRef(), repo.MilestoneIssuesAndPulls) m.Group("/{type:issues|pulls}", func() { diff --git a/templates/repo/issue/labels/label.tmpl b/templates/repo/issue/labels/label.tmpl deleted file mode 100644 index f40c792da70..00000000000 --- a/templates/repo/issue/labels/label.tmpl +++ /dev/null @@ -1,7 +0,0 @@ - - {{- ctx.RenderUtils.RenderLabel .label -}} - diff --git a/templates/repo/issue/labels/labels_selector_field.tmpl b/templates/repo/issue/labels/labels_selector_field.tmpl deleted file mode 100644 index 96fb6586640..00000000000 --- a/templates/repo/issue/labels/labels_selector_field.tmpl +++ /dev/null @@ -1,46 +0,0 @@ -
- - {{ctx.Locale.Tr "repo.issues.new.labels"}} - {{if and .HasIssuesOrPullsWritePermission (not .Repository.IsArchived)}} - {{svg "octicon-gear" 16 "tw-ml-1"}} - {{end}} - -
- {{if or .Labels .OrgLabels}} -
- {{svg "octicon-search" 16}} - -
- {{end}} - {{ctx.Locale.Tr "repo.issues.new.clear_labels"}} - {{if or .Labels .OrgLabels}} - {{$previousExclusiveScope := "_no_scope"}} - {{range .Labels}} - {{$exclusiveScope := .ExclusiveScope}} - {{if and (ne $previousExclusiveScope "_no_scope") (ne $previousExclusiveScope $exclusiveScope)}} -
- {{end}} - {{$previousExclusiveScope = $exclusiveScope}} - {{svg (Iif $exclusiveScope "octicon-dot-fill" "octicon-check")}}  {{ctx.RenderUtils.RenderLabel .}} - {{if .Description}}
{{.Description | ctx.RenderUtils.RenderEmoji}}{{end}} -

{{template "repo/issue/labels/label_archived" .}}

- - {{end}} -
- {{$previousExclusiveScope = "_no_scope"}} - {{range .OrgLabels}} - {{$exclusiveScope := .ExclusiveScope}} - {{if and (ne $previousExclusiveScope "_no_scope") (ne $previousExclusiveScope $exclusiveScope)}} -
- {{end}} - {{$previousExclusiveScope = $exclusiveScope}} - {{svg (Iif $exclusiveScope "octicon-dot-fill" "octicon-check")}}  {{ctx.RenderUtils.RenderLabel .}} - {{if .Description}}
{{.Description | ctx.RenderUtils.RenderEmoji}}{{end}} -

{{template "repo/issue/labels/label_archived" .}}

- - {{end}} - {{else}} -
{{ctx.Locale.Tr "repo.issues.new.no_items"}}
- {{end}} -
-
diff --git a/templates/repo/issue/labels/labels_sidebar.tmpl b/templates/repo/issue/labels/labels_sidebar.tmpl deleted file mode 100644 index 0b7b9b89696..00000000000 --- a/templates/repo/issue/labels/labels_sidebar.tmpl +++ /dev/null @@ -1,11 +0,0 @@ -
- - {{ctx.Locale.Tr "repo.issues.new.no_label"}} - {{range .root.Labels}} - {{template "repo/issue/labels/label" dict "root" $.root "label" .}} - {{end}} - {{range .root.OrgLabels}} - {{template "repo/issue/labels/label" dict "root" $.root "label" .}} - {{end}} - -
diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 190d52cf477..65d359e9dcd 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -18,15 +18,15 @@ {{range .Fields}} {{if eq .Type "input"}} - {{template "repo/issue/fields/input" "item" .}} + {{template "repo/issue/fields/input" dict "item" .}} {{else if eq .Type "markdown"}} - {{template "repo/issue/fields/markdown" "item" .}} + {{template "repo/issue/fields/markdown" dict "item" .}} {{else if eq .Type "textarea"}} - {{template "repo/issue/fields/textarea" "item" . "root" $}} + {{template "repo/issue/fields/textarea" dict "item" . "root" $}} {{else if eq .Type "dropdown"}} - {{template "repo/issue/fields/dropdown" "item" .}} + {{template "repo/issue/fields/dropdown" dict "item" .}} {{else if eq .Type "checkboxes"}} - {{template "repo/issue/fields/checkboxes" "item" .}} + {{template "repo/issue/fields/checkboxes" dict "item" .}} {{end}} {{end}} {{else}} @@ -49,13 +49,11 @@
{{template "repo/issue/branch_selector_field" $}} {{if .PageIsComparePull}} - {{template "repo/issue/sidebar/reviewer_list" dict "IssueSidebarReviewersData" $.IssueSidebarReviewersData}} + {{template "repo/issue/sidebar/reviewer_list" $.IssueSidebarReviewersData}}
{{end}} - - {{template "repo/issue/labels/labels_selector_field" .}} - {{template "repo/issue/labels/labels_sidebar" dict "root" $}} + {{template "repo/issue/sidebar/label_list" $.IssueSidebarLabelsData}}
diff --git a/templates/repo/issue/sidebar/label_list.tmpl b/templates/repo/issue/sidebar/label_list.tmpl new file mode 100644 index 00000000000..e9f4baa4335 --- /dev/null +++ b/templates/repo/issue/sidebar/label_list.tmpl @@ -0,0 +1,51 @@ +{{$data := .}} +{{$canChange := and ctx.RootData.HasIssuesOrPullsWritePermission (not $data.Repository.IsArchived)}} +
+ +
+ + {{ctx.Locale.Tr "repo.issues.new.labels"}} {{if $canChange}}{{svg "octicon-gear"}}{{end}} + +
+ {{if not $data.AllLabels}} +
{{ctx.Locale.Tr "repo.issues.new.no_items"}}
+ {{else}} +
+ {{svg "octicon-search" 16}} + +
+ {{ctx.Locale.Tr "repo.issues.new.clear_labels"}} + {{$previousExclusiveScope := "_no_scope"}} + {{range .RepoLabels}} + {{$exclusiveScope := .ExclusiveScope}} + {{if and (ne $previousExclusiveScope "_no_scope") (ne $previousExclusiveScope $exclusiveScope)}} +
+ {{end}} + {{$previousExclusiveScope = $exclusiveScope}} + {{template "repo/issue/sidebar/label_list_item" dict "Label" .}} + {{end}} +
+ {{$previousExclusiveScope = "_no_scope"}} + {{range .OrgLabels}} + {{$exclusiveScope := .ExclusiveScope}} + {{if and (ne $previousExclusiveScope "_no_scope") (ne $previousExclusiveScope $exclusiveScope)}} +
+ {{end}} + {{$previousExclusiveScope = $exclusiveScope}} + {{template "repo/issue/sidebar/label_list_item" dict "Label" .}} + {{end}} + {{end}} +
+
+ +
+ {{ctx.Locale.Tr "repo.issues.new.no_label"}} + {{range $data.AllLabels}} + {{if .IsChecked}} + + {{- ctx.RenderUtils.RenderLabel . -}} + + {{end}} + {{end}} +
+
diff --git a/templates/repo/issue/sidebar/label_list_item.tmpl b/templates/repo/issue/sidebar/label_list_item.tmpl new file mode 100644 index 00000000000..ad878e918be --- /dev/null +++ b/templates/repo/issue/sidebar/label_list_item.tmpl @@ -0,0 +1,11 @@ +{{$label := .Label}} + + {{svg (Iif $label.ExclusiveScope "octicon-dot-fill" "octicon-check")}} + {{ctx.RenderUtils.RenderLabel $label}} +
+ {{if $label.Description}}
{{$label.Description | ctx.RenderUtils.RenderEmoji}}
{{end}} +
{{template "repo/issue/labels/label_archived" $label}}
+
+ diff --git a/templates/repo/issue/sidebar/reviewer_list.tmpl b/templates/repo/issue/sidebar/reviewer_list.tmpl index 2d3218e9272..cf7b97c02b1 100644 --- a/templates/repo/issue/sidebar/reviewer_list.tmpl +++ b/templates/repo/issue/sidebar/reviewer_list.tmpl @@ -1,11 +1,9 @@ -{{$data := .IssueSidebarReviewersData}} +{{$data := .}} {{$hasCandidates := or $data.Reviewers $data.TeamReviewers}} -
+
{{/* match CreateIssueForm */}} -
- +
+ {{ctx.Locale.Tr "repo.issues.review.reviewers"}} {{if and $data.CanChooseReviewer}}{{svg "octicon-gear"}}{{end}}
@@ -19,7 +17,8 @@ {{if .User}} - {{svg "octicon-check"}} {{ctx.AvatarUtils.Avatar .User 20}} {{template "repo/search_name" .User}} + {{svg "octicon-check"}} + {{ctx.AvatarUtils.Avatar .User 20}} {{template "repo/search_name" .User}} {{end}} {{end}} @@ -29,7 +28,8 @@ {{if .Team}} - {{svg "octicon-check"}} {{svg "octicon-people" 20}} {{$data.RepoOwnerName}}/{{.Team.Name}} + {{svg "octicon-check"}} + {{svg "octicon-people" 20}} {{$data.RepoOwnerName}}/{{.Team.Name}} {{end}} {{end}} diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index 7a402747597..0fae1e9e1c1 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -2,13 +2,12 @@ {{template "repo/issue/branch_selector_field" $}} {{if .Issue.IsPull}} - {{template "repo/issue/sidebar/reviewer_list" dict "IssueSidebarReviewersData" $.IssueSidebarReviewersData}} + {{template "repo/issue/sidebar/reviewer_list" $.IssueSidebarReviewersData}} {{template "repo/issue/sidebar/wip_switch" $}}
{{end}} - {{template "repo/issue/labels/labels_selector_field" $}} - {{template "repo/issue/labels/labels_sidebar" dict "root" $}} + {{template "repo/issue/sidebar/label_list" $.IssueSidebarLabelsData}} {{template "repo/issue/sidebar/milestone_list" $}} {{template "repo/issue/sidebar/project_list" $}} diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 185a5f6f558..ff8342d29aa 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -50,7 +50,7 @@ width: 300px; } -.issue-sidebar-combo .ui.dropdown .item:not(.checked) svg.octicon-check { +.issue-sidebar-combo .ui.dropdown .item:not(.checked) .item-check-mark { visibility: hidden; } /* ideally, we should move these styles to ".ui.dropdown .menu.flex-items-menu > .item ...", could be done later */ @@ -62,6 +62,8 @@ .issue-content-right .dropdown > .menu { max-width: 270px; min-width: 0; + max-height: 500px; + overflow-x: auto; } @media (max-width: 767.98px) { @@ -110,10 +112,6 @@ left: 0; } -.repository .select-label .desc { - padding-left: 23px; -} - /* For the secondary pointing menu, respect its own border-bottom */ /* style reference: https://semantic-ui.com/collections/menu.html#pointing */ .repository .ui.tabs.container .ui.menu:not(.secondary.pointing) { diff --git a/web_src/css/repo/issue-label.css b/web_src/css/repo/issue-label.css index 9b4b144a009..0a25d31da9e 100644 --- a/web_src/css/repo/issue-label.css +++ b/web_src/css/repo/issue-label.css @@ -47,6 +47,7 @@ } .archived-label-hint { - float: right; - margin: -12px; + position: absolute; + top: 10px; + right: 5px; } diff --git a/web_src/js/features/common-page.ts b/web_src/js/features/common-page.ts index beec92d152b..56c5915b6db 100644 --- a/web_src/js/features/common-page.ts +++ b/web_src/js/features/common-page.ts @@ -32,13 +32,13 @@ export function initGlobalDropdown() { const $uiDropdowns = fomanticQuery('.ui.dropdown'); // do not init "custom" dropdowns, "custom" dropdowns are managed by their own code. - $uiDropdowns.filter(':not(.custom)').dropdown(); + $uiDropdowns.filter(':not(.custom)').dropdown({hideDividers: 'empty'}); // The "jump" means this dropdown is mainly used for "menu" purpose, // clicking an item will jump to somewhere else or trigger an action/function. // When a dropdown is used for non-refresh actions with tippy, // it must have this "jump" class to hide the tippy when dropdown is closed. - $uiDropdowns.filter('.jump').dropdown({ + $uiDropdowns.filter('.jump').dropdown('setting', { action: 'hide', onShow() { // hide associated tooltip while dropdown is open diff --git a/web_src/js/features/repo-issue-sidebar-combolist.ts b/web_src/js/features/repo-issue-sidebar-combolist.ts index d5416159887..f408eb43ba0 100644 --- a/web_src/js/features/repo-issue-sidebar-combolist.ts +++ b/web_src/js/features/repo-issue-sidebar-combolist.ts @@ -1,6 +1,6 @@ import {fomanticQuery} from '../modules/fomantic/base.ts'; import {POST} from '../modules/fetch.ts'; -import {queryElemChildren, toggleElem} from '../utils/dom.ts'; +import {queryElemChildren, queryElems, toggleElem} from '../utils/dom.ts'; // if there are draft comments, confirm before reloading, to avoid losing comments export function issueSidebarReloadConfirmDraftComment() { @@ -27,20 +27,37 @@ function collectCheckedValues(elDropdown: HTMLElement) { } export function initIssueSidebarComboList(container: HTMLElement) { - if (!container) return; - const updateUrl = container.getAttribute('data-update-url'); const elDropdown = container.querySelector(':scope > .ui.dropdown'); const elList = container.querySelector(':scope > .ui.list'); const elComboValue = container.querySelector(':scope > .combo-value'); - const initialValues = collectCheckedValues(elDropdown); + let initialValues = collectCheckedValues(elDropdown); elDropdown.addEventListener('click', (e) => { const elItem = (e.target as HTMLElement).closest('.item'); if (!elItem) return; e.preventDefault(); - if (elItem.getAttribute('data-can-change') !== 'true') return; - elItem.classList.toggle('checked'); + if (elItem.hasAttribute('data-can-change') && elItem.getAttribute('data-can-change') !== 'true') return; + + if (elItem.matches('.clear-selection')) { + queryElems(elDropdown, '.menu > .item', (el) => el.classList.remove('checked')); + elComboValue.value = ''; + return; + } + + const scope = elItem.getAttribute('data-scope'); + if (scope) { + // scoped items could only be checked one at a time + const elSelected = elDropdown.querySelector(`.menu > .item.checked[data-scope="${CSS.escape(scope)}"]`); + if (elSelected === elItem) { + elItem.classList.toggle('checked'); + } else { + queryElems(elDropdown, `.menu > .item[data-scope="${CSS.escape(scope)}"]`, (el) => el.classList.remove('checked')); + elItem.classList.toggle('checked', true); + } + } else { + elItem.classList.toggle('checked'); + } elComboValue.value = collectCheckedValues(elDropdown).join(','); }); @@ -61,29 +78,28 @@ export function initIssueSidebarComboList(container: HTMLElement) { if (changed) issueSidebarReloadConfirmDraftComment(); }; - const syncList = (changedValues) => { + const syncUiList = (changedValues) => { const elEmptyTip = elList.querySelector('.item.empty-list'); queryElemChildren(elList, '.item:not(.empty-list)', (el) => el.remove()); for (const value of changedValues) { - const el = elDropdown.querySelector(`.menu > .item[data-value="${value}"]`); + const el = elDropdown.querySelector(`.menu > .item[data-value="${CSS.escape(value)}"]`); const listItem = el.cloneNode(true) as HTMLElement; - listItem.querySelector('svg.octicon-check')?.remove(); + queryElems(listItem, '.item-check-mark, .item-secondary-info', (el) => el.remove()); elList.append(listItem); } const hasItems = Boolean(elList.querySelector('.item:not(.empty-list)')); toggleElem(elEmptyTip, !hasItems); }; - fomanticQuery(elDropdown).dropdown({ + fomanticQuery(elDropdown).dropdown('setting', { action: 'nothing', // do not hide the menu if user presses Enter fullTextSearch: 'exact', async onHide() { + // TODO: support "Esc" to cancel the selection. Use partial page loading to avoid losing inputs. const changedValues = collectCheckedValues(elDropdown); - if (updateUrl) { - await updateToBackend(changedValues); // send requests to backend and reload the page - } else { - syncList(changedValues); // only update the list in the sidebar - } + syncUiList(changedValues); + if (updateUrl) await updateToBackend(changedValues); + initialValues = changedValues; }, }); } diff --git a/web_src/js/features/repo-issue-sidebar.md b/web_src/js/features/repo-issue-sidebar.md new file mode 100644 index 00000000000..3022b52d05b --- /dev/null +++ b/web_src/js/features/repo-issue-sidebar.md @@ -0,0 +1,27 @@ +A sidebar combo (dropdown+list) is like this: + +```html +
+ +
+
+
clear
+
+ ... + ... +
+
+
+
+ no item + ... +
+
+``` + +When the selected items change, the `combo-value` input will be updated. +If there is `data-update-url`, it also calls backend to attach/detach the changed items. + +Also, the changed items will be syncronized to the `ui list` items. + +The items with the same data-scope only allow one selected at a time. diff --git a/web_src/js/features/repo-issue-sidebar.ts b/web_src/js/features/repo-issue-sidebar.ts index 4a1ef02aab7..52878848e8c 100644 --- a/web_src/js/features/repo-issue-sidebar.ts +++ b/web_src/js/features/repo-issue-sidebar.ts @@ -3,7 +3,7 @@ import {POST} from '../modules/fetch.ts'; import {updateIssuesMeta} from './repo-common.ts'; import {svg} from '../svg.ts'; import {htmlEscape} from 'escape-goat'; -import {toggleElem} from '../utils/dom.ts'; +import {queryElems, toggleElem} from '../utils/dom.ts'; import {initIssueSidebarComboList, issueSidebarReloadConfirmDraftComment} from './repo-issue-sidebar-combolist.ts'; function initBranchSelector() { @@ -28,7 +28,7 @@ function initBranchSelector() { } else { // for new issue, only update UI&form, do not send request/reload const selectedHiddenSelector = this.getAttribute('data-id-selector'); - document.querySelector(selectedHiddenSelector).value = selectedValue; + document.querySelector(selectedHiddenSelector).value = selectedValue; elSelectBranch.querySelector('.text-branch-name').textContent = selectedText; } }); @@ -53,7 +53,7 @@ function initListSubmits(selector, outerSelector) { for (const [elementId, item] of itemEntries) { await updateIssuesMeta( item['update-url'], - item.action, + item['action'], item['issue-id'], elementId, ); @@ -80,14 +80,14 @@ function initListSubmits(selector, outerSelector) { if (scope) { // Enable only clicked item for scoped labels if (this.getAttribute('data-scope') !== scope) { - return true; + return; } if (this !== clickedItem && !this.classList.contains('checked')) { - return true; + return; } } else if (this !== clickedItem) { // Toggle for other labels - return true; + return; } if (this.classList.contains('checked')) { @@ -258,13 +258,13 @@ export function initRepoIssueSidebar() { initRepoIssueDue(); // TODO: refactor the legacy initListSubmits&selectItem to initIssueSidebarComboList - initListSubmits('select-label', 'labels'); initListSubmits('select-assignees', 'assignees'); initListSubmits('select-assignees-modify', 'assignees'); + selectItem('.select-assignee', '#assignee_id'); + selectItem('.select-project', '#project_id'); selectItem('.select-milestone', '#milestone_id'); - selectItem('.select-assignee', '#assignee_id'); - // init the combo list: a dropdown for selecting reviewers, and a list for showing selected reviewers and related actions - initIssueSidebarComboList(document.querySelector('.issue-sidebar-combo[data-sidebar-combo-for="reviewers"]')); + // init the combo list: a dropdown for selecting items, and a list for showing selected items and related actions + queryElems(document, '.issue-sidebar-combo', (el) => initIssueSidebarComboList(el)); } diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index 92916ec8d72..7457531ece4 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -98,6 +98,7 @@ export function initRepoIssueSidebarList() { }); }); + // FIXME: it is wrong place to init ".ui.dropdown.label-filter" $('.menu .ui.dropdown.label-filter').on('keydown', (e) => { if (e.altKey && e.key === 'Enter') { const selectedItem = document.querySelector('.menu .ui.dropdown.label-filter .menu .item.selected'); @@ -106,7 +107,6 @@ export function initRepoIssueSidebarList() { } } }); - $('.ui.dropdown.label-filter, .ui.dropdown.select-label').dropdown('setting', {'hideDividers': 'empty'}).dropdown('refreshItems'); } export function initRepoIssueCommentDelete() { @@ -652,19 +652,6 @@ function initIssueTemplateCommentEditors($commentForm) { } } -// This function used to show and hide archived label on issue/pr -// page in the sidebar where we select the labels -// If we have any archived label tagged to issue and pr. We will show that -// archived label with checked classed otherwise we will hide it -// with the help of this function. -// This function runs globally. -export function initArchivedLabelHandler() { - if (!document.querySelector('.archived-label-hint')) return; - for (const label of document.querySelectorAll('[data-is-archived]')) { - toggleElem(label, label.classList.contains('checked')); - } -} - export function initRepoCommentFormAndSidebar() { const $commentForm = $('.comment.form'); if (!$commentForm.length) return; diff --git a/web_src/js/index.ts b/web_src/js/index.ts index 487aac97aa3..eeead37333b 100644 --- a/web_src/js/index.ts +++ b/web_src/js/index.ts @@ -30,7 +30,7 @@ import { initRepoIssueWipTitle, initRepoPullRequestMergeInstruction, initRepoPullRequestAllowMaintainerEdit, - initRepoPullRequestReview, initRepoIssueSidebarList, initArchivedLabelHandler, + initRepoPullRequestReview, initRepoIssueSidebarList, } from './features/repo-issue.ts'; import {initRepoEllipsisButton, initCommitStatuses} from './features/repo-commit.ts'; import {initRepoTopicBar} from './features/repo-home.ts'; @@ -182,7 +182,6 @@ onDomReady(() => { initRepoIssueContentHistory, initRepoIssueList, initRepoIssueSidebarList, - initArchivedLabelHandler, initRepoIssueReferenceRepositorySearch, initRepoIssueTimeTracking, initRepoIssueWipTitle, diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index 79ce05a7adb..29b34dd1e3f 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -3,7 +3,7 @@ import type {Promisable} from 'type-fest'; import type $ from 'jquery'; type ElementArg = Element | string | NodeListOf | Array | ReturnType; -type ElementsCallback = (el: Element) => Promisable; +type ElementsCallback = (el: T) => Promisable; type ElementsCallbackWithArgs = (el: Element, ...args: any[]) => Promisable; type ArrayLikeIterable = ArrayLike & Iterable; // for NodeListOf and Array @@ -58,7 +58,7 @@ export function isElemHidden(el: ElementArg) { return res[0]; } -function applyElemsCallback(elems: ArrayLikeIterable, fn?: ElementsCallback): ArrayLikeIterable { +function applyElemsCallback(elems: ArrayLikeIterable, fn?: ElementsCallback): ArrayLikeIterable { if (fn) { for (const el of elems) { fn(el); @@ -67,7 +67,7 @@ function applyElemsCallback(elems: ArrayLikeIterable, fn?: return elems; } -export function queryElemSiblings(el: Element, selector = '*', fn?: ElementsCallback): ArrayLikeIterable { +export function queryElemSiblings(el: Element, selector = '*', fn?: ElementsCallback): ArrayLikeIterable { const elems = Array.from(el.parentNode.children) as T[]; return applyElemsCallback(elems.filter((child: Element) => { return child !== el && child.matches(selector); @@ -75,13 +75,13 @@ export function queryElemSiblings(el: Element, selector = '*' } // it works like jQuery.children: only the direct children are selected -export function queryElemChildren(parent: Element | ParentNode, selector = '*', fn?: ElementsCallback): ArrayLikeIterable { +export function queryElemChildren(parent: Element | ParentNode, selector = '*', fn?: ElementsCallback): ArrayLikeIterable { return applyElemsCallback(parent.querySelectorAll(`:scope > ${selector}`), fn); } // it works like parent.querySelectorAll: all descendants are selected // in the future, all "queryElems(document, ...)" should be refactored to use a more specific parent -export function queryElems(parent: Element | ParentNode, selector: string, fn?: ElementsCallback): ArrayLikeIterable { +export function queryElems(parent: Element | ParentNode, selector: string, fn?: ElementsCallback): ArrayLikeIterable { return applyElemsCallback(parent.querySelectorAll(selector), fn); }