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.

options/locale/locale_en-US.ini

@@ -1634,6 +1634,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