Merge pull request 'Fix issue/comment menus' (#6419) from fnetx/issue-view-a11y into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6419
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

options/locale/locale_en-US.ini

@@ -1635,6 +1635,12 @@ issues.num_reviews_one = %d review
 issues.num_reviews_few = %d reviews
 issues.commented_at = `commented <a href="#%s">%s</a>`
 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

templates/repo/issue/view_content/add_reaction.tmpl

@@ -1,5 +1,5 @@
 {{if .ctxData.IsSigned}}
-<div class="item action ui dropdown jump pointing top right select-reaction" data-action-url="{{.ActionURL}}">
+<div class="item action ui dropdown jump pointing top right select-reaction" data-action-url="{{.ActionURL}}" aria-label="{{ctx.Locale.Tr "repo.issues.reaction.add"}}">
 	<a class="add-reaction muted">
 		{{svg "octicon-smiley"}}
 	</a>

templates/repo/issue/view_content/context_menu.tmpl

@@ -1,4 +1,4 @@
-<div class="item action ui dropdown jump pointing top right context-dropdown">
+<div class="item action ui dropdown jump pointing top right context-dropdown" aria-label="{{ctx.Locale.Tr "repo.issues.context.menu"}}">
 	<a class="context-menu muted">
 		{{svg "octicon-kebab-horizontal"}}
 	</a>

templates/repo/issue/view_content/reactions.tmpl

@@ -2,9 +2,18 @@
 {{range $key, $value := .Reactions}}
 	{{$hasReacted := $value.HasUser $.ctxData.SignedUserID}}
 	<a role="button" class="ui label basic{{if $hasReacted}} primary{{end}}{{if not $.ctxData.IsSigned}} disabled{{end}} comment-reaction-button"
-		data-tooltip-content
-		title="{{$value.GetFirstUsers}}{{if gt ($value.GetMoreUserCount) 0}} {{ctx.Locale.Tr "repo.reactions_more" $value.GetMoreUserCount}}{{end}}"
-		aria-label="{{$value.GetFirstUsers}}{{if gt ($value.GetMoreUserCount) 0}} {{ctx.Locale.Tr "repo.reactions_more" $value.GetMoreUserCount}}{{end}}"
+		title="{{$value.GetFirstUsers}}"
+		aria-label="
+			{{if eq ($value.GetMoreUserCount) 0}}
+				{{ctx.Locale.Tr "repo.issues.reaction.alt_few" $value.GetFirstUsers $key}}
+			{{else}}
+				{{ctx.Locale.Tr "repo.issues.reaction.alt_many" $value.GetFirstUsers $value.GetMoreUserCount $key}}
+			{{end}}
+			{{if $hasReacted}}
+				{{ctx.Locale.Tr "repo.issues.reaction.alt_remove" $key}}
+			{{else}}
+				{{ctx.Locale.Tr "repo.issues.reaction.alt_add" $key}}
+			{{end}}"
 		data-tooltip-placement="bottom-start"
 		data-reaction-content="{{$key}}" data-has-reacted="{{$hasReacted}}">
 		<span class="reaction">{{ReactionToEmoji $key}}</span>

templates/shared/combomarkdowneditor.tmpl

@@ -37,14 +37,14 @@ Template Attributes:
 				<md-task-list class="markdown-toolbar-button" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.list.task.tooltip"}}">{{svg "octicon-tasklist"}}</md-task-list>
 				<button type="button" class="markdown-toolbar-button" data-md-button data-md-action="unindent" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.unindent.tooltip"}}">{{svg "octicon-arrow-left"}}</button>
 				<button type="button" class="markdown-toolbar-button" data-md-button data-md-action="indent" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.indent.tooltip"}}">{{svg "octicon-arrow-right"}}</button>
-				<button type="button" class="markdown-toolbar-button show-modal button" data-md-action="new-table" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.new_table.tooltip"}}">{{svg "octicon-table"}}</button>
 			</div>
 			<div class="markdown-toolbar-group">
+				<button type="button" class="markdown-toolbar-button show-modal button" data-md-button data-md-action="new-table" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.new_table.tooltip"}}">{{svg "octicon-table"}}</button>
 				<md-mention class="markdown-toolbar-button" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.mention.tooltip"}}">{{svg "octicon-mention"}}</md-mention>
 				<md-ref class="markdown-toolbar-button" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.ref.tooltip"}}">{{svg "octicon-cross-reference"}}</md-ref>
 			</div>
 			<div class="markdown-toolbar-group">
-				<button class="markdown-toolbar-button markdown-switch-monospace" role="switch" data-enable-text="{{ctx.Locale.Tr "editor.buttons.enable_monospace_font"}}" data-disable-text="{{ctx.Locale.Tr "editor.buttons.disable_monospace_font"}}">{{svg "octicon-typography"}}</button>
+				<button class="markdown-toolbar-button markdown-switch-monospace" data-md-button role="switch" data-enable-text="{{ctx.Locale.Tr "editor.buttons.enable_monospace_font"}}" data-disable-text="{{ctx.Locale.Tr "editor.buttons.disable_monospace_font"}}">{{svg "octicon-typography"}}</button>
 				{{if .EasyMDE}}
 					<button class="markdown-toolbar-button markdown-switch-easymde" data-tooltip-content="{{ctx.Locale.Tr "editor.buttons.switch_to_legacy.tooltip"}}">{{svg "octicon-arrow-switch"}}</button>
 				{{end}}

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

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 <relative-time> which will re-add
     // title if it is absent