Skip to content

Fix: Hide divider when labels are hidden #27747

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

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion templates/repo/issue/filter_list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
{{range .Labels}}
{{$exclusiveScope := .ExclusiveScope}}
{{if and (ne $previousExclusiveScope $exclusiveScope)}}
<div class="divider"></div>
<div class="divider exclusive-scope"></div>
{{end}}
{{$previousExclusiveScope = $exclusiveScope}}
<a class="item label-filter-item tw-flex tw-items-center" {{if .IsArchived}}data-is-archived{{end}} href="?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state={{$.State}}&labels={{.QueryString}}&milestone={{$.MilestoneID}}&project={{$.ProjectID}}&assignee={{$.AssigneeID}}&poster={{$.PosterID}}{{if $.ShowArchivedLabels}}&archived=true{{end}}" data-label-id="{{.ID}}">
Expand Down
6 changes: 3 additions & 3 deletions templates/repo/issue/labels/labels_selector_field.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,20 @@
{{range .Labels}}
{{$exclusiveScope := .ExclusiveScope}}
{{if and (ne $previousExclusiveScope "_no_scope") (ne $previousExclusiveScope $exclusiveScope)}}
<div class="divider"></div>
<div class="divider exclusive-scope"></div>
{{end}}
{{$previousExclusiveScope = $exclusiveScope}}
<a class="{{if .IsChecked}}checked{{end}} item" href="#" data-id="{{.ID}}" {{if .IsArchived}}data-is-archived{{end}} data-id-selector="#label_{{.ID}}" data-scope="{{$exclusiveScope}}"><span class="octicon-check {{if not .IsChecked}}tw-invisible{{end}}">{{if $exclusiveScope}}{{svg "octicon-dot-fill"}}{{else}}{{svg "octicon-check"}}{{end}}</span>&nbsp;&nbsp;{{RenderLabel $.Context ctx.Locale .}}
{{if .Description}}<br><small class="desc">{{.Description | RenderEmoji $.Context}}</small>{{end}}
<p class="archived-label-hint">{{template "repo/issue/labels/label_archived" .}}</p>
</a>
{{end}}
<div class="divider"></div>
<div class="divider exclusive-scope"></div>
{{$previousExclusiveScope = "_no_scope"}}
{{range .OrgLabels}}
{{$exclusiveScope := .ExclusiveScope}}
{{if and (ne $previousExclusiveScope "_no_scope") (ne $previousExclusiveScope $exclusiveScope)}}
<div class="divider"></div>
<div class="divider exclusive-scope"></div>
{{end}}
{{$previousExclusiveScope = $exclusiveScope}}
<a class="{{if .IsChecked}}checked{{end}} item" href="#" data-id="{{.ID}}" {{if .IsArchived}}data-is-archived{{end}} data-id-selector="#label_{{.ID}}" data-scope="{{$exclusiveScope}}"><span class="octicon-check {{if not .IsChecked}}tw-invisible{{end}}">{{if $exclusiveScope}}{{svg "octicon-dot-fill"}}{{else}}{{svg "octicon-check"}}{{end}}</span>&nbsp;&nbsp;{{RenderLabel $.Context ctx.Locale .}}
Expand Down
2 changes: 2 additions & 0 deletions web_src/js/features/repo-issue-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,8 @@ function initArchivedLabelFilter() {
for (const label of archivedLabels) {
const id = label.getAttribute('data-label-id');
toggleElem(label, archivedLabelEl.checked || selectedLabels.includes(id));
// also toggle the divider
toggleElem($(label).next('.exclusive-scope'), archivedLabelEl.checked || selectedLabels.includes(id));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. There is not necessarily a divider
  2. I think the condition is wrong. It works in the situation you've tested, but I'm fairly certain that in more complex situations it will hide the divider even though it shouldn't.

}
};

Expand Down
2 changes: 2 additions & 0 deletions web_src/js/features/repo-issue.js
Original file line number Diff line number Diff line change
Expand Up @@ -744,5 +744,7 @@ 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'));
// also toggle the divider
toggleElem($(label).next('.exclusive-scope'), label.classList.contains('checked'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will test it rigorously.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The usecase I imagine to be faulty is two (or more) labels in scope, one archived, the other not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same I was thinking, with org labels as well.

Copy link
Member Author

@puni9869 puni9869 Oct 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4805034b-ceec-46cd-8631-fca2b25a2d38.mp4

This is the current condition with org and repo labels with all possible usecase,
One is failing, when we have archived label with scoped and normal label with archived marked.

}
}