Skip to content

Commit 7df8d5f

Browse files
committed
Fixes a11y behaviour for reactions and options in issue comments view
1 parent dd7f1c0 commit 7df8d5f

File tree

4 files changed

+56
-14
lines changed

4 files changed

+56
-14
lines changed

options/locale/locale_en-US.ini

+1
Original file line numberDiff line numberDiff line change
@@ -1333,6 +1333,7 @@ issues.draft_title = Draft
13331333
issues.num_comments = %d comments
13341334
issues.commented_at = `commented <a href="#%s">%s</a>`
13351335
issues.delete_comment_confirm = Are you sure you want to delete this comment?
1336+
issues.context.options_button = Comment Options
13361337
issues.context.copy_link = Copy Link
13371338
issues.context.quote_reply = Quote Reply
13381339
issues.context.reference_issue = Reference in new issue

templates/repo/issue/view_content/add_reaction.tmpl

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
{{if .ctx.IsSigned}}
2-
<div class="item action ui pointing select-reaction dropdown top right" data-action-url="{{.ActionURL}}">
3-
<a class="add-reaction">
2+
<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">
3+
<a class="add-reaction" role="none" aria-hidden="true">
44
{{svg "octicon-smiley"}}
55
</a>
66
<div class="menu">
7-
<div class="header">{{.ctx.locale.Tr "repo.pick_reaction"}}</div>
8-
<div class="divider"></div>
7+
<div class="header" role="none">{{.ctx.locale.Tr "repo.pick_reaction"}}</div>
8+
<div class="divider" role="none"></div>
99
{{range $value := AllowedReactions}}
10-
<div class="item reaction tooltip" data-content="{{$value}}">{{ReactionToEmoji $value}}</div>
10+
<div class="item reaction tooltip" role="menuitem" aria-label="{{$value}}" tabindex="-1" data-content="{{$value}}">{{ReactionToEmoji $value}}</div>
1111
{{end}}
1212
</div>
1313
</div>

templates/repo/issue/view_content/context_menu.tmpl

+8-8
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{{if .ctx.IsSigned}}
2-
<div class="item action ui pointing custom dropdown top right context-dropdown">
3-
<a class="context-menu">
2+
<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">
3+
<a class="context-menu" role="none" aria-hidden="true">
44
{{svg "octicon-kebab-horizontal"}}
55
</a>
66
<div class="menu">
@@ -10,16 +10,16 @@
1010
{{else}}
1111
{{$referenceUrl = Printf "%s/files#%s" .ctx.Issue.HTMLURL .item.HashTag}}
1212
{{end}}
13-
<div class="item context" data-clipboard-text="{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.copy_link"}}</div>
14-
<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>
13+
<div class="item context" role="menuitem" tabindex="-1" data-clipboard-text="{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.copy_link"}}</div>
14+
<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>
1515
{{if not .ctx.UnitIssuesGlobalDisabled}}
16-
<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>
16+
<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>
1717
{{end}}
1818
{{if or .ctx.Permission.IsAdmin .IsCommentPoster .ctx.HasIssuesOrPullsWritePermission}}
19-
<div class="divider"></div>
20-
<div class="item context edit-content">{{.ctx.locale.Tr "repo.issues.context.edit"}}</div>
19+
<div class="divider" role="none"></div>
20+
<div class="item context edit-content" role="menuitem" tabindex="-1">{{.ctx.locale.Tr "repo.issues.context.edit"}}</div>
2121
{{if .delete}}
22-
<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>
22+
<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>
2323
{{end}}
2424
{{end}}
2525
</div>

web_src/js/features/aria.js

+42-1
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,47 @@ function attachOneDropdownAria($dropdown) {
9595
$dropdown.on('keyup', (e) => { if (e.key.startsWith('Arrow')) deferredRefreshAria(); });
9696
}
9797

98+
function attachOneDropdownAriaTemplated($dropdown) {
99+
window.console && console.log('[attachOneDropdownAriaTemplated] '+$dropdown.prop('id'));
100+
if ($dropdown.attr('data-aria-attached')) return;
101+
$dropdown.attr('data-aria-attached', 1);
102+
103+
const $menu = $dropdown.find('> .menu');
104+
// update aria attributes according to current active/selected item
105+
const refreshAria = () => {
106+
const isMenuVisible = !$menu.is('.hidden') && !$menu.is('.animating.out');
107+
isMenuVisible ? $dropdown.attr('aria-expanded', 'true') : $dropdown.removeAttr('aria-expanded');
108+
109+
let $active = $menu.find('> .item.active');
110+
//if (!$active.length) $active = $menu.find('> .item.selected'); // it's strange that we need this fallback at the moment
111+
112+
// if there is an active item, use its id. if no active item, then the empty string is set
113+
$dropdown.attr('aria-activedescendant', $active.attr('id'));
114+
};
115+
116+
$dropdown.on('keydown', (e) => {
117+
window.console && console.log('keydown:'+e.key);
118+
// here it must use keydown event before dropdown's keyup handler, otherwise there is no Enter event in our keyup handler
119+
if (e.key === 'Enter') {
120+
const $item = $dropdown.dropdown('get item', $dropdown.dropdown('get value'));
121+
// if the selected item is clickable, then trigger the click event. in the future there could be a special CSS class for it.
122+
if ($item) $item[0].click();
123+
}
124+
else if (e.key === 'ESC') {
125+
$dropdown.dropdown('hide');
126+
$dropdown.removeAttr('aria-expanded');
127+
}
128+
});
129+
130+
// use setTimeout to run the refreshAria in next tick (to make sure the Fomantic UI code has finished its work)
131+
const deferredRefreshAria = () => { setTimeout(refreshAria, 0) }; // do not return any value, jQuery has return-value related behaviors.
132+
$dropdown.on('focus', deferredRefreshAria);
133+
$dropdown.on('mouseup', deferredRefreshAria);
134+
$dropdown.on('blur', deferredRefreshAria);
135+
$dropdown.on('keyup', (e) => { if (e.key.startsWith('Arrow')) deferredRefreshAria(); });
136+
}
137+
98138
export function attachDropdownAria($dropdowns) {
99-
$dropdowns.each((_, e) => attachOneDropdownAria($(e)));
139+
$dropdowns.filter(':not([data-aria-templated])').each((_, e) => attachOneDropdownAria($(e)));
140+
$dropdowns.filter('[data-aria-templated]').each((_, e) => attachOneDropdownAriaTemplated($(e)));
100141
}

0 commit comments

Comments
 (0)