diff --git a/models/issues/label.go b/models/issues/label.go index 4a5c3b4bfe..29ac82cfa3 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -150,6 +150,7 @@ func (l *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64, curr for i, curSel := range currentSelectedLabels { if curSel == l.ID { labelSelected = true + l.IsExcluded = false } else if -curSel == l.ID { labelSelected = true l.IsExcluded = true @@ -161,9 +162,10 @@ func (l *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64, curr } } - if !labelSelected { + if !labelSelected || l.IsExcluded { labelQuerySlice = append(labelQuerySlice, l.ID) } + l.IsSelected = labelSelected // Sort and deduplicate the ids to avoid the crawlers asking for the diff --git a/models/issues/label_test.go b/models/issues/label_test.go index d3e6146fc9..f52c20e4a2 100644 --- a/models/issues/label_test.go +++ b/models/issues/label_test.go @@ -32,18 +32,27 @@ func TestLabel_LoadSelectedLabelsAfterClick(t *testing.T) { // First test : with negative and scope label.LoadSelectedLabelsAfterClick([]int64{1, -8}, []string{"", "scope"}) + // Flips to positive to include after click + assert.Equal(t, "1,8", label.QueryString) + assert.True(t, label.IsSelected) + assert.True(t, label.IsExcluded) + + // Second test : with positive and scope + label.LoadSelectedLabelsAfterClick([]int64{1, 8}, []string{"", "scope"}) assert.Equal(t, "1", label.QueryString) assert.True(t, label.IsSelected) - // Second test : with duplicates + // Third test : with duplicates label.LoadSelectedLabelsAfterClick([]int64{1, 7, 1, 7, 7}, []string{"", "scope", "", "scope", "scope"}) assert.Equal(t, "1,8", label.QueryString) assert.False(t, label.IsSelected) + assert.False(t, label.IsExcluded) - // Third test : empty set + // Fourth test : empty set label.LoadSelectedLabelsAfterClick([]int64{}, []string{}) assert.False(t, label.IsSelected) assert.Equal(t, "8", label.QueryString) + assert.False(t, label.IsExcluded) } func TestLabel_ExclusiveScope(t *testing.T) { diff --git a/modules/templates/util_render.go b/modules/templates/util_render.go index bec8d5f5e3..02851ed75d 100644 --- a/modules/templates/util_render.go +++ b/modules/templates/util_render.go @@ -162,8 +162,8 @@ func RenderLabel(ctx *Context, label *issues_model.Label) template.HTML { // Regular label labelColor := label.Color + hex.EncodeToString([]byte{GetLabelOpacityByte(label.IsArchived())}) - s := fmt.Sprintf("
%s
", - archivedCSSClass, textColor, labelColor, description, RenderEmoji(ctx, label.Name)) + s := fmt.Sprintf("
%s
", + archivedCSSClass, textColor, labelColor, description, description, RenderEmoji(ctx, label.Name)) return template.HTML(s) } @@ -200,11 +200,11 @@ func RenderLabel(ctx *Context, label *issues_model.Label) template.HTML { scopeColor := "#" + hex.EncodeToString(scopeBytes) itemColor := "#" + hex.EncodeToString(itemBytes) - s := fmt.Sprintf(""+ + s := fmt.Sprintf(""+ "
%s
"+ "
%s
"+ "
", - archivedCSSClass, description, + archivedCSSClass, description, description, textColor, scopeColor, scopeText, textColor, itemColor, itemText) return template.HTML(s) diff --git a/modules/templates/util_render_test.go b/modules/templates/util_render_test.go index acf5353ca3..35412989e3 100644 --- a/modules/templates/util_render_test.go +++ b/modules/templates/util_render_test.go @@ -229,7 +229,8 @@ func TestRenderLabels(t *testing.T) { rendered := RenderLabels(ctx, []*issues_model.Label{label}, "user2/repo1", false) assert.Contains(t, rendered, "user2/repo1/issues?labels=1") assert.Contains(t, rendered, ">label1<") - assert.Contains(t, rendered, "title='First label'") + assert.Contains(t, rendered, "data-tooltip-content='First label'") + assert.Contains(t, rendered, "aria-description='First label'") rendered = RenderLabels(ctx, []*issues_model.Label{label}, "user2/repo1", true) assert.Contains(t, rendered, "user2/repo1/pulls?labels=1") assert.Contains(t, rendered, ">label1<") @@ -241,11 +242,11 @@ func TestRenderLabels(t *testing.T) { assert.Contains(t, rendered, "user2/repo1/issues?labels=11") assert.Contains(t, rendered, "> <script>malicious</script> <") assert.Contains(t, rendered, ">'?&<") - assert.Contains(t, rendered, "title='Malicious label ' <script>malicious</script>'") + assert.Contains(t, rendered, "data-tooltip-content='Malicious label ' <script>malicious</script>'") + assert.Contains(t, rendered, "aria-description='Malicious label ' <script>malicious</script>'") rendered = RenderLabels(ctx, []*issues_model.Label{labelArchived}, "user2/repo1", false) assert.Contains(t, rendered, "user2/repo1/issues?labels=12") assert.Contains(t, rendered, ">archived label<><") - assert.Contains(t, rendered, "title='repo.issues.archived_label_description'") } func TestRenderUser(t *testing.T) { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 543d75a0f9..00e4adb378 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1555,7 +1555,6 @@ issues.remove_ref_at = `removed reference %s %s` issues.add_ref_at = `added reference %s %s` issues.delete_branch_at = `deleted branch %s %s` issues.filter_label = Label -issues.filter_label_exclude = Use Alt + Click to exclude labels issues.filter_label_no_select = All labels issues.filter_label_select_no_label = No label issues.filter_milestone = Milestone diff --git a/templates/repo/issue/labels/label_archived.tmpl b/templates/repo/issue/labels/label_archived.tmpl index feaf77e456..cbcf462292 100644 --- a/templates/repo/issue/labels/label_archived.tmpl +++ b/templates/repo/issue/labels/label_archived.tmpl @@ -1,5 +1,5 @@ {{if .IsArchived}} - + {{ctx.Locale.Tr "archived"}} {{end}} diff --git a/templates/shared/label_filter.tmpl b/templates/shared/label_filter.tmpl index 0c8d537cc7..aa9a16f531 100644 --- a/templates/shared/label_filter.tmpl +++ b/templates/shared/label_filter.tmpl @@ -21,7 +21,6 @@ - {{ctx.Locale.Tr "repo.issues.filter_label_exclude"}}
{{ctx.Locale.Tr "repo.issues.filter_label_no_select"}} {{ctx.Locale.Tr "repo.issues.filter_label_select_no_label"}} @@ -32,19 +31,22 @@
{{end}} {{$previousExclusiveScope = $exclusiveScope}} - - {{if .IsExcluded}} - {{svg "octicon-circle-slash"}} - {{else if .IsSelected}} + {{end}} diff --git a/tests/e2e/repo-issues.test.e2e.ts b/tests/e2e/repo-issues.test.e2e.ts new file mode 100644 index 0000000000..b4b4c9bae4 --- /dev/null +++ b/tests/e2e/repo-issues.test.e2e.ts @@ -0,0 +1,47 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +// @watch start +// templates/shared/label_filter.tmpl +// web_src/js/features/repo-issue-sidebar-list.ts +// @watch end + +import {expect} from '@playwright/test'; +import {test} from './utils_e2e.ts'; + +test.use({user: 'user2'}); + +test('Label filter exclusion', async ({page}) => { + const response = await page.goto('/user2/repo1/issues'); + expect(response?.status()).toBe(200); + + await expect( + page.getByRole('link', {name: 'issue1'}), + ).toBeVisible(); + + // open label filter dropdown + await page.getByRole('combobox').filter({has: page.getByText('Label')}).click(); + + // exclude the label1 label attached to issue1 + const labelOption = page + .getByRole('option').filter({has: page.getByText('label1')}); + await expect(labelOption).toBeVisible(); + await labelOption.hover(); + await labelOption.getByRole('button', {name: 'Exclude label'}).click(); + + await expect( + page.getByRole('link', {name: 'issue1'}), + ).toBeHidden(); + + // open label filter dropdown + await page.getByRole('combobox').filter({has: page.getByText('Label')}).click(); + + // clear exclusion + await expect(labelOption).toBeVisible(); + await labelOption.hover(); + await labelOption.getByRole('button', {name: 'Clear exclusion'}).click(); + + await expect( + page.getByRole('link', {name: 'issue1'}), + ).toBeVisible(); +}); diff --git a/web_src/css/index.css b/web_src/css/index.css index feb84f1cf7..cf0521399c 100644 --- a/web_src/css/index.css +++ b/web_src/css/index.css @@ -84,6 +84,7 @@ @import "./review.css"; @import "./actions.css"; @import "./migrate.css"; +@import "./issues.css"; @tailwind utilities; @import "./helpers.css"; diff --git a/web_src/css/issues.css b/web_src/css/issues.css new file mode 100644 index 0000000000..28539e5eb3 --- /dev/null +++ b/web_src/css/issues.css @@ -0,0 +1,75 @@ +/* Copyright 2025 The Forgejo Authors. All rights reserved. + * SPDX-License-Identifier: GPL-3.0-or-later */ + +.archived-label-filter:has(#archived-label-filter:not(:checked)) +[data-is-archived] { + display: none !important; +} + +.menu .item:has(> .label-filter-item) .label-exclude-item-btn { + opacity: 0; + position: absolute; + right: 0; + height: 100%; + width: 40px; + background: none; + color: var(--color-secondary-dark-5); + + @media (any-hover: none) { + opacity: 1; + } + + &.active { + color: var(--color-secondary-dark-10); + } + + &:is(:hover, :focus-visible), + &.selected { + background: var(--color-secondary); + color: var(--color-secondary-dark-7); + } +} + +.ui.menu .ui.dropdown .menu > .item:has(> .label-filter-item) { + max-width: 288px; + + @media (any-hover: none) { + padding-right: 50px !important; + } + + &:hover, + &.selected, + &:has(.label-exclude-item-btn.active) { + padding-right: 50px !important; + + .label-exclude-item-btn { + opacity: 1; + } + } +} + +.menu:has(> .item[data-selected]) +.item:not([data-selected]) +> .label-filter-item { + padding-left: 27px; +} + +.menu .item .label-filter-item { + width: 100%; + min-width: 0; +} + +.menu .item .label-filter-item > .label { + text-overflow: ellipsis; + white-space: nowrap; + overflow: hidden; + display: block; +} + +.menu .item .archived-label-indicator { + margin: 0 0 0 10px !important; + + @media (prefers-reduced-motion: no-preference) { + transition: transform 200ms ease; + } +} diff --git a/web_src/js/features/repo-issue-list.js b/web_src/js/features/repo-issue-list.js index 92f058c4d2..a5f71034fb 100644 --- a/web_src/js/features/repo-issue-list.js +++ b/web_src/js/features/repo-issue-list.js @@ -220,13 +220,13 @@ function initArchivedLabelFilter() { const archivedElToggle = () => { for (const label of archivedLabels) { const id = label.getAttribute('data-label-id'); - toggleElem(label, archivedLabelEl.checked || selectedLabels.includes(id)); + toggleElem(label.closest('.item'), archivedLabelEl.checked || selectedLabels.includes(id)); } }; archivedElToggle(); + archivedLabelEl.addEventListener('change', () => { - archivedElToggle(); if (archivedLabelEl.checked) { url.searchParams.set('archived', 'true'); } else { diff --git a/web_src/js/features/repo-issue-sidebar-list.ts b/web_src/js/features/repo-issue-sidebar-list.ts new file mode 100644 index 0000000000..8e4127c58d --- /dev/null +++ b/web_src/js/features/repo-issue-sidebar-list.ts @@ -0,0 +1,186 @@ +import $ from 'jquery'; +import {htmlEscape} from 'escape-goat'; +import {emojiHTML} from './emoji.js'; + +const {appSubUrl} = window.config; + +export function initRepoIssueSidebarList() { + const repolink = $('#repolink').val(); + const repoId = $('#repoId').val(); + const crossRepoSearch = $('#crossRepoSearch').val() === 'true'; + const tp = $('#type').val(); + + $('#new-dependency-drop-list') + .dropdown({ + apiSettings: { + beforeSend(settings) { + if (!settings.urlData.query.trim()) { + settings.url = `${appSubUrl}/${repolink}/issues/search?q={query}&type=${tp}&sort=updated`; + } else if (crossRepoSearch) { + settings.url = `${appSubUrl}/issues/search?q={query}&priority_repo_id=${repoId}&type=${tp}&sort=relevance`; + } else { + settings.url = `${appSubUrl}/${repolink}/issues/search?q={query}&type=${tp}&sort=relevance`; + } + return settings; + }, + onResponse(response: Record) { + 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 + for (const [_, issue] of Object.entries(response)) { + // Don't list current issue in the dependency list. + if (issue.id === currIssueId) { + continue; + } + filteredResponse.results.push({ + name: `#${issue.number} ${issueTitleHTML(htmlEscape(issue.title)) + }
${htmlEscape(issue.repository.full_name)}
`, + value: issue.id, + }); + } + return filteredResponse; + }, + cache: false, + }, + + fullTextSearch: true, + }); + + $('.menu button.label-exclude-item-btn').each(function () { + $(this).on('click', function () { + const label = this.closest('.item').querySelector('a.label-filter-item'); + + if (!label) { + return; + } + + excludeLabel(label); + }); + }); + + // Increase surface area to include a label in the filters + for (const labelFilterItem of document.querySelectorAll('.menu a.label-filter-item')) { + const menuItem = labelFilterItem.closest('.item'); + menuItem.addEventListener('click', (event: MouseEvent) => { + if (labelFilterItem === event.target || (event.target as HTMLElement).closest('.label-exclude-item-btn')) { + return; + } + + labelFilterItem.click(); + }); + } + + $('.menu .ui.dropdown.label-filter').on('keydown', (e: KeyboardEvent) => { + const selectedItem = document.querySelector('.menu .ui.dropdown.label-filter .menu .item.selected'); + + if (!selectedItem) { + return; + } + + const selectedItemExcludeButton = selectedItem.querySelector('.label-exclude-item-btn'); + const selectExcludeButton = () => selectedItemExcludeButton?.classList.add('selected'); + const deselectExcludeButton = () => selectedItemExcludeButton?.classList.remove('selected'); + const isExcludeButtonSelected = () => selectedItemExcludeButton?.classList.contains('selected'); + + if (e.key === 'Enter') { + const labelElement = selectedItem.querySelector('a.label-filter-item'); + + if (!labelElement) { + return; + } + + if (isExcludeButtonSelected()) { + excludeLabel(labelElement); + } else { + labelElement.click(); + } + } + + // the menu can be navigated with or without the search input being focused + // therefore we check if the input is currently focused and the caret is + // at the end to make sure the moving the caret within the input works + const isOnInput = (e.target as HTMLElement).matches('input'); + const input = e.target as HTMLInputElement; + + if (e.key === 'ArrowRight' && (!isOnInput || isCaretAtEnd(input))) { + selectExcludeButton(); + } + + if (e.key === 'ArrowLeft') { + // it will deselect the exclude button before letting the user move along the focused input text + // so the user has to press once the left key to deselect and then another time to + // move the caret to the left side + if (isOnInput && isCaretAtEnd(input) && selectedItemExcludeButton.classList.contains('selected')) { + e.preventDefault(); + } + deselectExcludeButton(); + } + + // when a exclude button is selected moving to the prev or next item in the menu + // is still possible, but the exclude button can remain selected, this makes + // sure to clear the selection class from the exclude buttons that are not + // within the currently selected menu item + if (e.key === 'ArrowUp' || e.key === 'ArrowDown') { + for (const excludeButtonSelected of document.querySelectorAll('.label-exclude-item-btn.selected')) { + if (!selectedItem.contains(excludeButtonSelected)) { + excludeButtonSelected.classList.remove('selected'); + } + } + } + }); + + $('.ui.dropdown.label-filter, .ui.dropdown.select-label').dropdown('setting', {'hideDividers': 'empty'}).dropdown('refreshItems'); +} + +/** + * Render the issue's title. + * It converts emojis and code blocks syntax into their respective HTML equivalent. + */ +export function issueTitleHTML(title: string) { + return title.replaceAll(/:[-+\w]+:/g, (emoji) => emojiHTML(emoji.substring(1, emoji.length - 1))) + .replaceAll(/`[^`]+`/g, (code) => `${code.substring(1, code.length - 1)}`); +} + +/** + * Excludes a label from filters provided by the data-label-id attribute of an element. + * + * If the label is included it will be converted to an exclusion, if its already excluded it will get removed, otherwise, if not present at all it will get excluded. + */ +export function excludeLabel(item: HTMLElement) { + const id = item.getAttribute('data-label-id'); + const excludedId = `-${id}`; + + const params = new URLSearchParams(window.location.search); + const labelIds = new Set((params.get('labels') ?? '').split(',').filter((id) => id.length > 0)); + + if (labelIds.has(id)) { + labelIds.delete(id); + labelIds.add(excludedId); + } else if (labelIds.has(excludedId)) { + labelIds.delete(excludedId); + } else { + labelIds.add(excludedId); + } + + params.set('labels', Array.from(labelIds).join(',')); + + window.location.search = params.toString(); +} + +/** + * Returns true if the caret is at the end of the input even if it has content + */ +function isCaretAtEnd(inputElement: HTMLInputElement) { + const value = inputElement.value; + return ( + inputElement.selectionStart === inputElement.selectionEnd && + inputElement.selectionEnd === value.length + ); +} diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 73131706cc..eb6af4d48b 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -8,7 +8,6 @@ import {toAbsoluteUrl} from '../utils.js'; import {initDropzone} from './common-global.js'; import {POST, GET} from '../modules/fetch.js'; import {showErrorToast} from '../modules/toast.js'; -import {emojiHTML} from './emoji.js'; const {appSubUrl} = window.config; @@ -109,81 +108,6 @@ export function initRepoIssueDue() { }); } -/** - * @param {HTMLElement} item - */ -function excludeLabel(item) { - const href = item.getAttribute('href'); - const id = item.getAttribute('data-label-id'); - - const regStr = `labels=((?:-?[0-9]+%2c)*)(${id})((?:%2c-?[0-9]+)*)&`; - const newStr = 'labels=$1-$2$3&'; - - window.location.assign(href.replace(new RegExp(regStr), newStr)); -} - -export function initRepoIssueSidebarList() { - const repolink = $('#repolink').val(); - const repoId = $('#repoId').val(); - const crossRepoSearch = $('#crossRepoSearch').val() === 'true'; - const tp = $('#type').val(); - $('#new-dependency-drop-list') - .dropdown({ - apiSettings: { - beforeSend(settings) { - if (!settings.urlData.query.trim()) { - settings.url = `${appSubUrl}/${repolink}/issues/search?q={query}&type=${tp}&sort=updated`; - } else if (crossRepoSearch) { - settings.url = `${appSubUrl}/issues/search?q={query}&priority_repo_id=${repoId}&type=${tp}&sort=relevance`; - } else { - settings.url = `${appSubUrl}/${repolink}/issues/search?q={query}&type=${tp}&sort=relevance`; - } - return settings; - }, - 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 - for (const [_, issue] of Object.entries(response)) { - // Don't list current issue in the dependency list. - if (issue.id === currIssueId) { - continue; - } - filteredResponse.results.push({ - name: `#${issue.number} ${issueTitleHTML(htmlEscape(issue.title)) - }
${htmlEscape(issue.repository.full_name)}
`, - value: issue.id, - }); - } - return filteredResponse; - }, - cache: false, - }, - - fullTextSearch: true, - }); - - $('.menu a.label-filter-item').each(function () { - $(this).on('click', function (e) { - if (e.altKey) { - e.preventDefault(); - excludeLabel(this); - } - }); - }); - - // FIXME: this is broken, see discussion https://codeberg.org/forgejo/forgejo/pulls/8199 - $('.menu .ui.dropdown.label-filter').on('keydown', (e) => { - if (e.altKey && e.keyCode === 13) { - const selectedItem = document.querySelector('.menu .ui.dropdown.label-filter .menu .item.selected'); - if (selectedItem) { - excludeLabel(selectedItem); - } - } - }); - $('.ui.dropdown.label-filter, .ui.dropdown.select-label').dropdown('setting', {'hideDividers': 'empty'}).dropdown('refreshItems'); -} - export function initRepoIssueCommentDelete() { // Delete comment document.addEventListener('click', async (e) => { @@ -801,9 +725,3 @@ export function initArchivedLabelHandler() { toggleElem(label, label.classList.contains('checked')); } } - -// Render the issue's title. It converts emojis and code blocks syntax into their respective HTML equivalent. -export function issueTitleHTML(title) { - return title.replaceAll(/:[-+\w]+:/g, (emoji) => emojiHTML(emoji.substring(1, emoji.length - 1))) - .replaceAll(/`[^`]+`/g, (code) => `${code.substring(1, code.length - 1)}`); -} diff --git a/web_src/js/features/repo-issue.test.js b/web_src/js/features/repo-issue.test.js index f72eb5794b..acf4ea209e 100644 --- a/web_src/js/features/repo-issue.test.js +++ b/web_src/js/features/repo-issue.test.js @@ -1,6 +1,6 @@ import {vi} from 'vitest'; -import {issueTitleHTML} from './repo-issue.js'; +import {issueTitleHTML, excludeLabel} from './repo-issue-sidebar-list.ts'; vi.mock('./comp/ComboMarkdownEditor.js', () => ({})); // jQuery is missing @@ -21,3 +21,26 @@ test('Convert issue title to html', () => { expect(issueTitleHTML('issue title :+1: `code`')).toEqual(`issue title ${expected_thumbs_up} ${expected_code_block}`); }); + +const getLabelsParam = () => new URLSearchParams(window.location.search).get('labels'); + +test('Toggles label exclusion from filters', () => { + expect(getLabelsParam()).toEqual(null); + + const element = document.createElement('div'); + element.dataset['label-id'] = '1'; + + // excludes it + excludeLabel(element); + expect(getLabelsParam()).toEqual('-1'); + + // since it was excluded above, now it should delete it + excludeLabel(element); + expect(getLabelsParam()).toEqual(''); + + // if we add it manually it should swap it to an exclusion + window.location.search = '?labels=1'; + expect(getLabelsParam()).toEqual('1'); + excludeLabel(element); + expect(getLabelsParam()).toEqual('-1'); +}); diff --git a/web_src/js/index.js b/web_src/js/index.js index 8701385e92..37848d2d90 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -30,8 +30,10 @@ import { initRepoIssueTimeTracking, initRepoIssueWipTitle, initRepoPullRequestAllowMaintainerEdit, - initRepoPullRequestReview, initRepoIssueSidebarList, initArchivedLabelHandler, + initRepoPullRequestReview, + initArchivedLabelHandler, } from './features/repo-issue.js'; +import {initRepoIssueSidebarList} from './features/repo-issue-sidebar-list.ts'; import {initRepoEllipsisButton, initCommitStatuses, initCommitNotes} from './features/repo-commit.js'; import { initFootLanguageMenu, diff --git a/web_src/js/types.d.ts b/web_src/js/types.d.ts index 5efc36762b..d464f25fe9 100644 --- a/web_src/js/types.d.ts +++ b/web_src/js/types.d.ts @@ -1,6 +1,7 @@ interface Window { config?: { appUrl: string; + appSubUrl: string } }