From 3496c819da1c0cb53f24606aa7c678ab09f50965 Mon Sep 17 00:00:00 2001 From: Otto Richter Date: Mon, 30 Dec 2024 16:06:18 +0100 Subject: [PATCH] Fix issue/comment menus Closes https://codeberg.org/forgejo/forgejo/issues/1120 - Adds labels to reaction and context menu. - Fixes taborder in markdown combobox buttons. They are now only one "tab" stop and can be navigated with arrow buttons and in the right order (previously, it would skip the table button). - Generates more verbose output for the reactio selectors to provide content for users who cannot identify the meaning of these buttons visually. Explicit aria-labels are now preferred over auto-generated ones. (cherry picked from commit c67d63d88a77f81b3f79b75642624b3457d4e8b6) --- options/locale/locale_en-US.ini | 6 ++++++ .../repo/issue/view_content/add_reaction.tmpl | 2 +- .../repo/issue/view_content/context_menu.tmpl | 2 +- templates/repo/issue/view_content/reactions.tmpl | 15 ++++++++++++--- templates/shared/combomarkdowneditor.tmpl | 4 ++-- tests/e2e/issue-comment.test.e2e.ts | 13 +++++++++++++ web_src/js/modules/tippy.js | 5 ++++- 7 files changed, 39 insertions(+), 8 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 5a89662cb2..44d1e0e61d 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1633,6 +1633,12 @@ issues.num_reviews_one = %d review issues.num_reviews_few = %d reviews issues.commented_at = `commented %s` issues.delete_comment_confirm = Are you sure you want to delete this comment? +issues.reaction.add = Add reaction +issues.reaction.alt_few = %[1]s reacted %[2]s. +issues.reaction.alt_many = %[1]s and %[2]d more reacted %[3]s. +issues.reaction.alt_remove = Remove %[1]s reaction from comment. +issues.reaction.alt_add = Add %[1]s reaction to comment. +issues.context.menu = Comment menu issues.context.copy_link = Copy link issues.context.quote_reply = Quote reply issues.context.reference_issue = Reference in a new issue diff --git a/templates/repo/issue/view_content/add_reaction.tmpl b/templates/repo/issue/view_content/add_reaction.tmpl index 37931f287e..5d4aa2298e 100644 --- a/templates/repo/issue/view_content/add_reaction.tmpl +++ b/templates/repo/issue/view_content/add_reaction.tmpl @@ -1,5 +1,5 @@ {{if .ctxData.IsSigned}} -
+
{{svg "octicon-smiley"}} diff --git a/templates/repo/issue/view_content/context_menu.tmpl b/templates/repo/issue/view_content/context_menu.tmpl index f020a9ea4b..5fc5af400f 100644 --- a/templates/repo/issue/view_content/context_menu.tmpl +++ b/templates/repo/issue/view_content/context_menu.tmpl @@ -1,4 +1,4 @@ -
+
{{svg "octicon-kebab-horizontal"}} diff --git a/templates/repo/issue/view_content/reactions.tmpl b/templates/repo/issue/view_content/reactions.tmpl index da6319667b..e5cb9a8ed3 100644 --- a/templates/repo/issue/view_content/reactions.tmpl +++ b/templates/repo/issue/view_content/reactions.tmpl @@ -2,9 +2,18 @@ {{range $key, $value := .Reactions}} {{$hasReacted := $value.HasUser $.ctxData.SignedUserID}} {{ReactionToEmoji $key}} diff --git a/templates/shared/combomarkdowneditor.tmpl b/templates/shared/combomarkdowneditor.tmpl index 9fcab8a9ae..e7bace6962 100644 --- a/templates/shared/combomarkdowneditor.tmpl +++ b/templates/shared/combomarkdowneditor.tmpl @@ -37,14 +37,14 @@ Template Attributes: {{svg "octicon-tasklist"}} -
+ {{svg "octicon-mention"}} {{svg "octicon-cross-reference"}}
- + {{if .EasyMDE}} {{end}} diff --git a/tests/e2e/issue-comment.test.e2e.ts b/tests/e2e/issue-comment.test.e2e.ts index f493f64506..4fce16764b 100644 --- a/tests/e2e/issue-comment.test.e2e.ts +++ b/tests/e2e/issue-comment.test.e2e.ts @@ -11,6 +11,19 @@ test.beforeAll(async ({browser}, workerInfo) => { await login_user(browser, workerInfo, 'user2'); }); +test('Menu accessibility', async ({browser}, workerInfo) => { + const page = await login({browser}, workerInfo); + await page.goto('/user2/repo1/issues/1'); + await expect(page.getByLabel('user2 reacted eyes. Remove eyes')).toBeVisible(); + await expect(page.getByLabel('reacted laugh. Remove laugh')).toBeVisible(); + await expect(page.locator('#issue-1').getByLabel('Comment menu')).toBeVisible(); + await expect(page.locator('#issue-1').getByRole('heading').getByLabel('Add reaction')).toBeVisible(); + page.getByLabel('reacted laugh. Remove').click(); + await expect(page.getByLabel('user1 reacted laugh. Add laugh')).toBeVisible(); + page.getByLabel('user1 reacted laugh.').click(); + await expect(page.getByLabel('user1, user2 reacted laugh. Remove laugh')).toBeVisible(); +}); + test('Hyperlink paste behaviour', async ({browser}, workerInfo) => { test.skip(['Mobile Safari', 'Mobile Chrome', 'webkit'].includes(workerInfo.project.name), 'Mobile clients seem to have very weird behaviour with this test, which I cannot confirm with real usage'); const page = await login({browser}, workerInfo); diff --git a/web_src/js/modules/tippy.js b/web_src/js/modules/tippy.js index 83b28e5745..887e7194d9 100644 --- a/web_src/js/modules/tippy.js +++ b/web_src/js/modules/tippy.js @@ -103,7 +103,10 @@ function switchTitleToTooltip(target) { } } target.setAttribute('data-tooltip-content', title); - target.setAttribute('aria-label', title); + // only replace if not explicitly set + if (target.getAttribute('aria-label') !== null) { + target.setAttribute('aria-label', title); + } // keep the attribute, in case there are some other "[title]" selectors // and to prevent infinite loop with which will re-add // title if it is absent