Fix duplicate dropdown dividers

.eslintrc.yaml

@@ -818,7 +818,7 @@ rules:
   unicorn/consistent-destructuring: [2]
   unicorn/consistent-empty-array-spread: [2]
   unicorn/consistent-existence-index-check: [0]
-  unicorn/consistent-function-scoping: [2]
+  unicorn/consistent-function-scoping: [0]
   unicorn/custom-error-definition: [0]
   unicorn/empty-brace-spaces: [2]
   unicorn/error-message: [0]

modules/templates/helper.go

@@ -294,17 +294,15 @@ func timeEstimateString(timeSec any) string {
 	return util.TimeEstimateString(v)
 }
 
-type QueryString string
-
-func queryBuild(a ...any) QueryString {
+func queryBuild(a ...any) template.URL {
 	var s string
 	if len(a)%2 == 1 {
 		if v, ok := a[0].(string); ok {
 			if v == "" || (v[0] != '?' && v[0] != '&') {
 				panic("queryBuild: invalid argument")
 			}
 			s = v
-		} else if v, ok := a[0].(QueryString); ok {
+		} else if v, ok := a[0].(template.URL); ok {
 			s = string(v)
 		} else {
 			panic("queryBuild: invalid argument")
@@ -356,7 +354,7 @@ func queryBuild(a ...any) QueryString {
 	if s != "" && s != "&" && s[len(s)-1] == '&' {
 		s = s[:len(s)-1]
 	}
-	return QueryString(s)
+	return template.URL(s)
 }
 
 func panicIfDevOrTesting() {

templates/projects/view.tmpl

@@ -36,10 +36,11 @@
 							{{range .Labels}}
 								{{$exclusiveScope := .ExclusiveScope}}
 								{{if and (ne $previousExclusiveScope $exclusiveScope)}}
-									<div class="divider"></div>
+									<div class="divider" data-scope="{{.ExclusiveScope}}"></div>
 								{{end}}
 								{{$previousExclusiveScope = $exclusiveScope}}
-								<a class="item label-filter-item tw-flex tw-items-center" {{if .IsArchived}}data-is-archived{{end}} href="?labels={{.QueryString}}&assignee={{$.AssigneeID}}{{if $.ShowArchivedLabels}}&archived=true{{end}}" data-label-id="{{.ID}}">
+								<a class="item label-filter-item tw-flex tw-items-center" data-label-id="{{.ID}}" data-scope="{{.ExclusiveScope}}" {{if .IsArchived}}data-is-archived{{end}}
+									href="?labels={{.QueryString}}&assignee={{$.AssigneeID}}{{if $.ShowArchivedLabels}}&archived=true{{end}}">
 									{{if .IsExcluded}}
 										{{svg "octicon-circle-slash"}}
 									{{else if .IsSelected}}

templates/repo/issue/filter_list.tmpl

@@ -30,10 +30,11 @@
 		{{range .Labels}}
 			{{$exclusiveScope := .ExclusiveScope}}
 			{{if and (ne $previousExclusiveScope $exclusiveScope)}}
-				<div class="divider"></div>
+				<div class="divider" data-scope="{{.ExclusiveScope}}"></div>
 			{{end}}
 			{{$previousExclusiveScope = $exclusiveScope}}
-			<a class="item label-filter-item tw-flex tw-items-center" {{if .IsArchived}}data-is-archived{{end}} href="{{QueryBuild $queryLink "labels" .QueryString}}" data-label-id="{{.ID}}">
+			<a class="item label-filter-item tw-flex tw-items-center" data-label-id="{{.ID}}" data-scope="{{.ExclusiveScope}}" {{if .IsArchived}}data-is-archived{{end}}
+				href="{{QueryBuild $queryLink "labels" .QueryString}}">
 				{{if .IsExcluded}}
 					{{svg "octicon-circle-slash"}}
 				{{else if .IsSelected}}

templates/repo/issue/sidebar/label_list.tmpl

@@ -22,7 +22,7 @@
 					{{range $data.RepoLabels}}
 						{{$exclusiveScope := .ExclusiveScope}}
 						{{if and (ne $previousExclusiveScope "_no_scope") (ne $previousExclusiveScope $exclusiveScope)}}
-							<div class="divider"></div>
+							<div class="divider" data-scope="{{.ExclusiveScope}}"></div>
 						{{end}}
 						{{$previousExclusiveScope = $exclusiveScope}}
 						{{template "repo/issue/sidebar/label_list_item" dict "Label" .}}
@@ -32,7 +32,7 @@
 					{{range $data.OrgLabels}}
 						{{$exclusiveScope := .ExclusiveScope}}
 						{{if and (ne $previousExclusiveScope "_no_scope") (ne $previousExclusiveScope $exclusiveScope)}}
-							<div class="divider"></div>
+							<div class="divider" data-scope="{{.ExclusiveScope}}"></div>
 						{{end}}
 						{{$previousExclusiveScope = $exclusiveScope}}
 						{{template "repo/issue/sidebar/label_list_item" dict "Label" .}}

web_src/js/features/repo-issue.ts

@@ -8,6 +8,7 @@ import {parseIssuePageInfo, toAbsoluteUrl} from '../utils.ts';
 import {GET, POST} from '../modules/fetch.ts';
 import {showErrorToast} from '../modules/toast.ts';
 import {initRepoIssueSidebar} from './repo-issue-sidebar.ts';
+import {fomanticQuery} from '../modules/fomantic/base.ts';
 
 const {appSubUrl} = window.config;
 
@@ -31,46 +32,45 @@ export function initRepoIssueSidebarList() {
   if (crossRepoSearch === 'true') {
     issueSearchUrl = `${appSubUrl}/issues/search?q={query}&priority_repo_id=${issuePageInfo.repoId}&type=${issuePageInfo.issueDependencySearchType}`;
   }
-  $('#new-dependency-drop-list')
-    .dropdown({
-      apiSettings: {
-        url: issueSearchUrl,
-        onResponse(response) {
-          const filteredResponse = {success: true, results: []};
-          const currIssueId = $('#new-dependency-drop-list').data('issue-id');
-          // Parse the response from the api to work with our dropdown
-          $.each(response, (_i, issue) => {
-            // Don't list current issue in the dependency list.
-            if (issue.id === currIssueId) {
-              return;
-            }
-            filteredResponse.results.push({
-              name: `<div class="gt-ellipsis">#${issue.number} ${htmlEscape(issue.title)}</div>
+  fomanticQuery('#new-dependency-drop-list').dropdown({
+    fullTextSearch: true,
+    apiSettings: {
+      url: issueSearchUrl,
+      onResponse(response) {
+        const filteredResponse = {success: true, results: []};
+        const currIssueId = $('#new-dependency-drop-list').data('issue-id');
+        // Parse the response from the api to work with our dropdown
+        $.each(response, (_i, issue) => {
+          // Don't list current issue in the dependency list.
+          if (issue.id === currIssueId) {
+            return;
+          }
+          filteredResponse.results.push({
+            name: `<div class="gt-ellipsis">#${issue.number} ${htmlEscape(issue.title)}</div>
 <div class="text small tw-break-anywhere">${htmlEscape(issue.repository.full_name)}</div>`,
-              value: issue.id,
-            });
+            value: issue.id,
           });
-          return filteredResponse;
-        },
-        cache: false,
+        });
+        return filteredResponse;
       },
+      cache: false,
+    },
+  });
+}
 
-      fullTextSearch: true,
-    });
-
-  $('.menu a.label-filter-item').each(function () {
+export function initRepoIssueLabelFilter() {
+  // the "label-filter" is used in 2 templates: projects/view, issue/filter_list (issue list page including the milestone page)
+  $('.ui.dropdown.label-filter a.label-filter-item').each(function () {
     $(this).on('click', function (e) {
       if (e.altKey) {
         e.preventDefault();
         excludeLabel(this);
       }
     });
   });
-
-  // FIXME: it is wrong place to init ".ui.dropdown.label-filter"
-  $('.menu .ui.dropdown.label-filter').on('keydown', (e) => {
+  $('.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');
+      const selectedItem = document.querySelector('.ui.dropdown.label-filter .menu .item.selected');
       if (selectedItem) {
         excludeLabel(selectedItem);
       }

web_src/js/index.ts

@@ -29,7 +29,7 @@ import {
   initRepoIssueWipTitle,
   initRepoPullRequestMergeInstruction,
   initRepoPullRequestAllowMaintainerEdit,
-  initRepoPullRequestReview, initRepoIssueSidebarList,
+  initRepoPullRequestReview, initRepoIssueSidebarList, initRepoIssueLabelFilter,
 } from './features/repo-issue.ts';
 import {initRepoEllipsisButton, initCommitStatuses} from './features/repo-commit.ts';
 import {initRepoTopicBar} from './features/repo-home.ts';
@@ -181,6 +181,7 @@ onDomReady(() => {
     initRepoGraphGit,
     initRepoIssueContentHistory,
     initRepoIssueList,
+    initRepoIssueLabelFilter,
     initRepoIssueSidebarList,
     initRepoIssueReferenceRepositorySearch,
     initRepoIssueWipTitle,

web_src/js/modules/fomantic/dropdown.test.ts

@@ -0,0 +1,56 @@
+import {createElementFromHTML} from '../../utils/dom.ts';
+import {hideScopedEmptyDividers} from './dropdown.ts';
+
+test('hideScopedEmptyDividers-simple', () => {
+  const container = createElementFromHTML(`<div>
+<div class="divider"></div>
+<div class="item">a</div>
+<div class="divider"></div>
+<div class="divider"></div>
+<div class="divider"></div>
+<div class="item">b</div>
+<div class="divider"></div>
+</div>`);
+  hideScopedEmptyDividers(container);
+  expect(container.innerHTML).toEqual(`
+<div class="divider hidden transition"></div>
+<div class="item">a</div>
+<div class="divider hidden transition"></div>
+<div class="divider hidden transition"></div>
+<div class="divider"></div>
+<div class="item">b</div>
+<div class="divider hidden transition"></div>
+`);
+});
+
+test('hideScopedEmptyDividers-hidden1', () => {
+  const container = createElementFromHTML(`<div>
+<div class="item">a</div>
+<div class="divider" data-scope="b"></div>
+<div class="item tw-hidden" data-scope="b">b</div>
+</div>`);
+  hideScopedEmptyDividers(container);
+  expect(container.innerHTML).toEqual(`
+<div class="item">a</div>
+<div class="divider hidden transition" data-scope="b"></div>
+<div class="item tw-hidden" data-scope="b">b</div>
+`);
+});
+
+test('hideScopedEmptyDividers-hidden2', () => {
+  const container = createElementFromHTML(`<div>
+<div class="item" data-scope="">a</div>
+<div class="divider" data-scope="b"></div>
+<div class="item tw-hidden" data-scope="b">b</div>
+<div class="divider" data-scope=""></div>
+<div class="item" data-scope="">c</div>
+</div>`);
+  hideScopedEmptyDividers(container);
+  expect(container.innerHTML).toEqual(`
+<div class="item" data-scope="">a</div>
+<div class="divider hidden transition" data-scope="b"></div>
+<div class="item tw-hidden" data-scope="b">b</div>
+<div class="divider hidden transition" data-scope=""></div>
+<div class="item" data-scope="">c</div>
+`);
+});

web_src/js/modules/fomantic/dropdown.ts

@@ -59,6 +59,12 @@ function updateSelectionLabel(label: HTMLElement) {
   }
 }
 
+function processMenuItems($dropdown, dropdownCall) {
+  const hideEmptyDividers = dropdownCall('setting', 'hideDividers') === 'empty';
+  const itemsMenu = $dropdown[0].querySelector('.scrolling.menu') || $dropdown[0].querySelector('.menu');
+  if (hideEmptyDividers) hideScopedEmptyDividers(itemsMenu);
+}
+
 // delegate the dropdown's template functions and callback functions to add aria attributes.
 function delegateOne($dropdown: any) {
   const dropdownCall = fomanticDropdownFn.bind($dropdown);
@@ -72,6 +78,18 @@ function delegateOne($dropdown: any) {
   // * If the "dropdown icon" is clicked again when the menu is visible, Fomantic calls "blurSearch", so hide the menu
   dropdownCall('internal', 'blurSearch', function () { oldBlurSearch.call(this); dropdownCall('hide') });
 
+  const oldFilterItems = dropdownCall('internal', 'filterItems');
+  dropdownCall('internal', 'filterItems', function (...args: any[]) {
+    oldFilterItems.call(this, ...args);
+    processMenuItems($dropdown, dropdownCall);
+  });
+
+  const oldShow = dropdownCall('internal', 'show');
+  dropdownCall('internal', 'show', function (...args: any[]) {
+    oldShow.call(this, ...args);
+    processMenuItems($dropdown, dropdownCall);
+  });
+
   // the "template" functions are used for dynamic creation (eg: AJAX)
   const dropdownTemplates = {...dropdownCall('setting', 'templates'), t: performance.now()};
   const dropdownTemplatesMenuOld = dropdownTemplates.menu;
@@ -271,3 +289,65 @@ function attachDomEvents(dropdown: HTMLElement, focusable: HTMLElement, menu: HT
     ignoreClickPreEvents = ignoreClickPreVisible = 0;
   }, true);
 }
+
+// Although Fomantic Dropdown supports "hideDividers", it doesn't really work with our "scoped dividers"
+// At the moment, "label dropdown items" use scopes, a sample case is:
+// * a-label
+// * divider
+// * scope/1
+// * scope/2
+// * divider
+// * z-label
+// when the "scope/*" are filtered out, we'd like to see "a-label" and "z-label" without the divider.
+export function hideScopedEmptyDividers(container: Element) {
+  const visibleItems: Element[] = [];
+  const curScopeVisibleItems: Element[] = [];
+  let curScope: string = '', lastVisibleScope: string = '';
+  const isScopedDivider = (item: Element) => item.matches('.divider') && item.hasAttribute('data-scope');
+  const hideDivider = (item: Element) => item.classList.add('hidden', 'transition'); // dropdown has its own classes to hide items
+
+  const handleScopeSwitch = (itemScope: string) => {
+    if (curScopeVisibleItems.length === 1 && isScopedDivider(curScopeVisibleItems[0])) {
+      hideDivider(curScopeVisibleItems[0]);
+    } else if (curScopeVisibleItems.length) {
+      if (isScopedDivider(curScopeVisibleItems[0]) && lastVisibleScope === curScope) {
+        hideDivider(curScopeVisibleItems[0]);
+        curScopeVisibleItems.shift();
+      }
+      visibleItems.push(...curScopeVisibleItems);
+      lastVisibleScope = curScope;
+    }
+    curScope = itemScope;
+    curScopeVisibleItems.length = 0;
+  };
+
+  // hide the scope dividers if the scope items are empty
+  for (const item of container.children) {
+    const itemScope = item.getAttribute('data-scope') || '';
+    if (itemScope !== curScope) {
+      handleScopeSwitch(itemScope);
+    }
+    if (!item.classList.contains('filtered') && !item.classList.contains('tw-hidden')) {
+      curScopeVisibleItems.push(item as HTMLElement);
+    }
+  }
+  handleScopeSwitch('');
+
+  // hide all leading and trailing dividers
+  while (visibleItems.length) {
+    if (!visibleItems[0].matches('.divider')) break;
+    hideDivider(visibleItems[0]);
+    visibleItems.shift();
+  }
+  while (visibleItems.length) {
+    if (!visibleItems[visibleItems.length - 1].matches('.divider')) break;
+    hideDivider(visibleItems[visibleItems.length - 1]);
+    visibleItems.pop();
+  }
+  // hide all duplicate dividers, hide current divider if next sibling is still divider
+  // no need to update "visibleItems" array since this is the last loop
+  for (const item of visibleItems) {
+    if (!item.matches('.divider')) continue;
+    if (item.nextElementSibling?.matches('.divider')) hideDivider(item);
+  }
+}

web_src/js/webcomponents/overflow-menu.ts

@@ -12,7 +12,7 @@ window.customElements.define('overflow-menu', class extends HTMLElement {
   mutationObserver: MutationObserver;
   lastWidth: number;
 
-  updateItems = throttle(100, () => { // eslint-disable-line unicorn/consistent-function-scoping -- https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2088
+  updateItems = throttle(100, () => {
     if (!this.tippyContent) {
       const div = document.createElement('div');
       div.classList.add('tippy-target');