Fixes a11y behaviour for reactions and options in issue comments view

fsologureng · Nov 9, 2022 · 7df8d5f · 7df8d5f
1 parent dd7f1c0
commit 7df8d5f
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 14 deletions.
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
@@ -1333,6 +1333,7 @@ issues.draft_title = Draft
 issues.num_comments = %d comments
 issues.commented_at = `commented <a href="#%s">%s</a>`
 issues.delete_comment_confirm = Are you sure you want to delete this comment?
+issues.context.options_button = Comment Options
 issues.context.copy_link = Copy Link
 issues.context.quote_reply = Quote Reply
 issues.context.reference_issue = Reference in new issue

diff --git a/templates/repo/issue/view_content/add_reaction.tmpl b/templates/repo/issue/view_content/add_reaction.tmpl
@@ -1,13 +1,13 @@
 {{if .ctx.IsSigned}}
-<div class="item action ui pointing select-reaction dropdown top right" data-action-url="{{.ActionURL}}">
-	<a class="add-reaction">
+<div class="item action ui pointing select-reaction dropdown top right" role="menu" aria-haspopup="menu" aria-expanded="false" aria-label="{{.ctx.locale.Tr "repo.pick_reaction"}}" tabindex="0" data-action-url="{{.ActionURL}}" data-aria-templated="true">
+	<a class="add-reaction" role="none" aria-hidden="true">
 		{{svg "octicon-smiley"}}
 	</a>
 	<div class="menu">
-		<div class="header">{{.ctx.locale.Tr "repo.pick_reaction"}}</div>
-		<div class="divider"></div>
+		<div class="header" role="none">{{.ctx.locale.Tr "repo.pick_reaction"}}</div>
+		<div class="divider" role="none"></div>
 		{{range $value := AllowedReactions}}
-			<div class="item reaction tooltip" data-content="{{$value}}">{{ReactionToEmoji $value}}</div>
+			<div class="item reaction tooltip" role="menuitem" aria-label="{{$value}}" tabindex="-1" data-content="{{$value}}">{{ReactionToEmoji $value}}</div>
 		{{end}}
 	</div>
 </div>

diff --git a/templates/repo/issue/view_content/context_menu.tmpl b/templates/repo/issue/view_content/context_menu.tmpl
@@ -1,6 +1,6 @@
 {{if .ctx.IsSigned}}
-<div class="item action ui pointing custom dropdown top right context-dropdown">
-	<a class="context-menu">
+<div class="item action ui pointing custom dropdown top right context-dropdown" role="menu" aria-haspopup="menu" aria-expanded="false" aria-label="{{.locale.Tr "repo.issues.context.options_button"}}" tabindex="0" data-aria-templated="true">
+	<a class="context-menu" role="none" aria-hidden="true">
 		{{svg "octicon-kebab-horizontal"}}
 	</a>
 	<div class="menu">
@@ -10,16 +10,16 @@
 		{{else}}
 			{{$referenceUrl = Printf "%s/files#%s" .ctx.Issue.HTMLURL .item.HashTag}}
 		{{end}}
-		<div class="item context" data-clipboard-text="{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.copy_link"}}</div>
-		<div class="item context quote-reply {{if .diff}}quote-reply-diff{{end}}" data-target="{{.item.ID}}">{{.ctx.locale.Tr "repo.issues.context.quote_reply"}}</div>
+		<div class="item context" role="menuitem" tabindex="-1" data-clipboard-text="{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.copy_link"}}</div>
+		<div class="item context quote-reply {{if .diff}}quote-reply-diff{{end}}" role="menuitem" tabindex="-1" data-target="{{.item.ID}}">{{.ctx.locale.Tr "repo.issues.context.quote_reply"}}</div>
 		{{if not .ctx.UnitIssuesGlobalDisabled}}
-			<div class="item context reference-issue" data-target="{{.item.ID}}" data-modal="#reference-issue-modal" data-poster="{{.item.Poster.GetDisplayName}}" data-poster-username="{{.item.Poster.Name}}" data-reference="{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.reference_issue"}}</div>
+			<div class="item context reference-issue" role="menuitem" tabindex="-1" data-target="{{.item.ID}}" data-modal="#reference-issue-modal" data-poster="{{.item.Poster.GetDisplayName}}" data-poster-username="{{.item.Poster.Name}}" data-reference="{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.reference_issue"}}</div>
 		{{end}}
 		{{if or .ctx.Permission.IsAdmin .IsCommentPoster .ctx.HasIssuesOrPullsWritePermission}}
-			<div class="divider"></div>
-			<div class="item context edit-content">{{.ctx.locale.Tr "repo.issues.context.edit"}}</div>
+			<div class="divider" role="none"></div>
+			<div class="item context edit-content" role="menuitem" tabindex="-1">{{.ctx.locale.Tr "repo.issues.context.edit"}}</div>
 			{{if .delete}}
-				<div class="item context delete-comment" data-comment-id={{.item.HashTag}} data-url="{{.ctx.RepoLink}}/comments/{{.item.ID}}/delete" data-locale="{{.ctx.locale.Tr "repo.issues.delete_comment_confirm"}}">{{.ctx.locale.Tr "repo.issues.context.delete"}}</div>
+				<div class="item context delete-comment" role="menuitem" tabindex="-1" data-comment-id={{.item.HashTag}} data-url="{{.ctx.RepoLink}}/comments/{{.item.ID}}/delete" data-locale="{{.ctx.locale.Tr "repo.issues.delete_comment_confirm"}}">{{.ctx.locale.Tr "repo.issues.context.delete"}}</div>
 			{{end}}
 		{{end}}
 	</div>

diff --git a/web_src/js/features/aria.js b/web_src/js/features/aria.js
@@ -95,6 +95,47 @@ function attachOneDropdownAria($dropdown) {
   $dropdown.on('keyup', (e) => { if (e.key.startsWith('Arrow')) deferredRefreshAria(); });
 }
 
+function attachOneDropdownAriaTemplated($dropdown) {
+  window.console && console.log('[attachOneDropdownAriaTemplated] '+$dropdown.prop('id'));
+  if ($dropdown.attr('data-aria-attached')) return;
+  $dropdown.attr('data-aria-attached', 1);
+
+  const $menu = $dropdown.find('> .menu');
+  // update aria attributes according to current active/selected item
+  const refreshAria = () => {
+    const isMenuVisible = !$menu.is('.hidden') && !$menu.is('.animating.out');
+    isMenuVisible ? $dropdown.attr('aria-expanded', 'true') : $dropdown.removeAttr('aria-expanded');
+
+    let $active = $menu.find('> .item.active');
+    //if (!$active.length) $active = $menu.find('> .item.selected'); // it's strange that we need this fallback at the moment
+
+    // if there is an active item, use its id. if no active item, then the empty string is set
+    $dropdown.attr('aria-activedescendant', $active.attr('id'));
+  };
+
+  $dropdown.on('keydown', (e) => {
+    window.console && console.log('keydown:'+e.key);
+    // here it must use keydown event before dropdown's keyup handler, otherwise there is no Enter event in our keyup handler
+    if (e.key === 'Enter') {
+      const $item = $dropdown.dropdown('get item', $dropdown.dropdown('get value'));
+      // if the selected item is clickable, then trigger the click event. in the future there could be a special CSS class for it.
+      if ($item) $item[0].click();
+    }
+    else if (e.key === 'ESC') {
+      $dropdown.dropdown('hide');
+      $dropdown.removeAttr('aria-expanded');
+    }
+  });
+
+  // use setTimeout to run the refreshAria in next tick (to make sure the Fomantic UI code has finished its work)
+  const deferredRefreshAria = () => { setTimeout(refreshAria, 0) }; // do not return any value, jQuery has return-value related behaviors.
+  $dropdown.on('focus', deferredRefreshAria);
+  $dropdown.on('mouseup', deferredRefreshAria);
+  $dropdown.on('blur', deferredRefreshAria);
+  $dropdown.on('keyup', (e) => { if (e.key.startsWith('Arrow')) deferredRefreshAria(); });
+}
+
 export function attachDropdownAria($dropdowns) {
-  $dropdowns.each((_, e) => attachOneDropdownAria($(e)));
+  $dropdowns.filter(':not([data-aria-templated])').each((_, e) => attachOneDropdownAria($(e)));
+  $dropdowns.filter('[data-aria-templated]').each((_, e) => attachOneDropdownAriaTemplated($(e)));
 }