mirror
/
gitea
mirror of https://github.com/go-gitea/gitea
2
0
Fork 0
Code Issues Releases Wiki Activity

Fix duplicate dropdown dividers (#32760)

Browse Source 
Fix #27466

The problem is that any item in the menu could be hidden, pure CSS won't
work, and dropdown's builtin "hideDividers" doesn't work with our "scope
dividers". The newly introduced "archived" label makes the dividers
regression more.
pull/32756/head^2
wxiaoguang 2 months ago committed by GitHub
parent 2d13eafd69
commit 5675efb3e0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 179 additions and 42 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 2
      .eslintrc.yaml
  2. 8
      modules/templates/helper.go
  3. 5
      templates/projects/view.tmpl
  4. 5
      templates/repo/issue/filter_list.tmpl
  5. 4
      templates/repo/issue/sidebar/label_list.tmpl
  6. 56
      web_src/js/features/repo-issue.ts
  7. 3
      web_src/js/index.ts
  8. 56
      web_src/js/modules/fomantic/dropdown.test.ts
  9. 80
      web_src/js/modules/fomantic/dropdown.ts
  10. 2
      web_src/js/webcomponents/overflow-menu.ts

2
.eslintrc.yaml
Unescape View File

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

8
modules/templates/helper.go
Unescape View File

@ -294,9 +294,7 @@ 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 {
@ -304,7 +302,7 @@ func queryBuild(a ...any) QueryString {
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() {

5
templates/projects/view.tmpl
Unescape View File

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

5
templates/repo/issue/filter_list.tmpl
Unescape View File

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

4
templates/repo/issue/sidebar/label_list.tmpl
Unescape View File

@ -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" .}}

56
web_src/js/features/repo-issue.ts
Unescape View File

@ -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,34 +32,35 @@ 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();
@ -66,11 +68,9 @@ export function initRepoIssueSidebarList() {
}
});
});
// 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);
}

3
web_src/js/index.ts
Unescape View File

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

56
web_src/js/modules/fomantic/dropdown.test.ts
Unescape View File

@ -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>
`);
});

80
web_src/js/modules/fomantic/dropdown.ts
Unescape View File

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

2
web_src/js/webcomponents/overflow-menu.ts
Unescape View File

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

Write Preview
Loading…
Cancel
Save