TudbuT/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Fix aria.js bugs: incorrect role element problem, mobile focus problem, tippy problem (#23450)

Browse source 
This PR is extracted from #23346 to address some unclear (I don't
understand) code-belonging concerns.

This PR needs to be backported, otherwise the `aria.js` is too buggy in
some cases. Since there would be two minor conflicts, I will do the
backport manually.

Before: the `aria.js` is still buggy in some cases.

After: tested with AppleVoice, Android TalkBack

* Fix incorrect dropdown init code
* Fix incorrect role element (the menu role should be on the `$menu`
element, but not on the `$focusable`)
* Fix the focus-show-click-hide problem on mobile. Now the language menu
works as expected
* Fix incorrect dropdown template function setting
* Clarify the logic in aria.js
* Hide item's tippy after menu gets hidden
* Fix incorrect tippy `setProps` after `destroy`
* Fix UI lag problem when page gets redirected during menu hiding
animation with screen reader
* Improve comments
* Implement the layout proposed by #19861

<details>


d74a7efb60/web_src/js/features/aria.md (L38-L47)

</details>
This commit is contained in:
wxiaoguang 2023-03-17 11:08:05 +08:00 • committed by GitHub
parent e200c68bad
commit 345aa09756
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 203 additions and 90 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
templates/base/footer_content.tmpl
View file

@ -21,7 +21,7 @@
{{end}}
<div class="ui language bottom floating slide up dropdown link item">
{{svg "octicon-globe"}}
<div class="text">{{.locale.LangName}}</div>
<span>{{.locale.LangName}}</span>
<div class="menu language-menu">
{{range .AllLangs}}
<a lang="{{.Lang}}" data-url="{{AppSubUrl}}/?lang={{.Lang}}" class="item {{if eq $.locale.Lang .Lang}}active selected{{end}}">{{.Name}}</a>

2
templates/repo/issue/view_content/add_reaction.tmpl
View file

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

2
templates/repo/issue/view_content/context_menu.tmpl
View file

@ -1,5 +1,5 @@
{{if .ctxData.IsSigned}}
<div class="item action ui pointing custom dropdown top right context-dropdown">
<div class="item action ui dropdown jump pointing top right context-dropdown">
<a class="context-menu">
{{svg "octicon-kebab-horizontal"}}
</a>

165
web_src/js/features/aria.js
View file

@ -6,42 +6,16 @@ function generateAriaId() {
return `_aria_auto_id_${ariaIdCounter++}`;
}
// make the item has role=option, and add an id if there wasn't one yet.
function prepareMenuItem($item) {
if (!$item.attr('id')) $item.attr('id', generateAriaId());
$item.attr({'role': 'menuitem', 'tabindex': '-1'});
$item.find('a').attr('tabindex', '-1'); // as above, the elements inside the dropdown menu item should not be focusable, the focus should always be on the dropdown primary element.
}
// when the menu items are loaded from AJAX requests, the items are created dynamically
const defaultCreateDynamicMenu = $.fn.dropdown.settings.templates.menu;
$.fn.dropdown.settings.templates.menu = function(response, fields, preserveHTML, className) {
const ret = defaultCreateDynamicMenu(response, fields, preserveHTML, className);
const $wrapper = $('<div>').append(ret);
const $items = $wrapper.find('> .item');
$items.each((_, item) => {
prepareMenuItem($(item));
});
return $wrapper.html();
};
function attachOneDropdownAria($dropdown) {
if ($dropdown.attr('data-aria-attached')) return;
if ($dropdown.attr('data-aria-attached') || $dropdown.hasClass('custom')) return;
$dropdown.attr('data-aria-attached', 1);
const $textSearch = $dropdown.find('input.search').eq(0);
const $focusable = $textSearch.length ? $textSearch : $dropdown; // see comment below
if (!$focusable.length) return;
// prepare menu list
const $menu = $dropdown.find('> .menu');
if (!$menu.attr('id')) $menu.attr('id', generateAriaId());
// dropdown has 2 different focusing behaviors
// * with search input: the input is focused, and it works perfectly with aria-activedescendant pointing another sibling element.
// Dropdown has 2 different focusing behaviors
// * with search input: the input is focused, and it works with aria-activedescendant pointing another sibling element.
// * without search input (but the readonly text), the dropdown itself is focused. then the aria-activedescendant points to the element inside dropdown
// Some desktop screen readers may change the focus, but dropdown requires that the focus must be on its primary element, then they don't work well.
// expected user interactions for dropdown with aria support:
// Expected user interactions for dropdown with aria support:
// * user can use Tab to focus in the dropdown, then the dropdown menu (list) will be shown
// * user presses Tab on the focused dropdown to move focus to next sibling focusable element (but not the menu item)
// * user can use arrow key Up/Down to navigate between menu items
@ -51,31 +25,83 @@ function attachOneDropdownAria($dropdown) {
// TODO: multiple selection is not supported yet.
const $textSearch = $dropdown.find('input.search').eq(0);
const $focusable = $textSearch.length ? $textSearch : $dropdown; // the primary element for focus, see comment above
if (!$focusable.length) return;
// There are 2 possible solutions about the role: combobox or menu.
// The idea is that if there is an input, then it's a combobox, otherwise it's a menu.
// Since #19861 we have prepared the "combobox" solution, but didn't get enough time to put it into practice and test before.
const isComboBox = $dropdown.find('input').length > 0;
const focusableRole = isComboBox ? 'combobox' : 'button';
const listPopupRole = isComboBox ? 'listbox' : 'menu';
const listItemRole = isComboBox ? 'option' : 'menuitem';
// make the item has role=option/menuitem, add an id if there wasn't one yet, make items as non-focusable
// the elements inside the dropdown menu item should not be focusable, the focus should always be on the dropdown primary element.
function prepareMenuItem($item) {
if (!$item.attr('id')) $item.attr('id', generateAriaId());
$item.attr({'role': listItemRole, 'tabindex': '-1'});
$item.find('a').attr('tabindex', '-1');
}
// delegate the dropdown's template function to add aria attributes.
// the "template" functions are used for dynamic creation (eg: AJAX)
const dropdownTemplates = {...$dropdown.dropdown('setting', 'templates')};
const dropdownTemplatesMenuOld = dropdownTemplates.menu;
dropdownTemplates.menu = function(response, fields, preserveHTML, className) {
// when the dropdown menu items are loaded from AJAX requests, the items are created dynamically
const menuItems = dropdownTemplatesMenuOld(response, fields, preserveHTML, className);
const $wrapper = $('<div>').append(menuItems);
const $items = $wrapper.find('> .item');
$items.each((_, item) => prepareMenuItem($(item)));
return $wrapper.html();
};
$dropdown.dropdown('setting', 'templates', dropdownTemplates);
// use tooltip's content as aria-label if there is no aria-label
if ($dropdown.hasClass('tooltip') && $dropdown.attr('data-content') && !$dropdown.attr('aria-label')) {
$dropdown.attr('aria-label', $dropdown.attr('data-content'));
}
// prepare dropdown menu list popup
const $menu = $dropdown.find('> .menu');
if (!$menu.attr('id')) $menu.attr('id', generateAriaId());
$menu.find('> .item').each((_, item) => {
prepareMenuItem($(item));
});
// this role could only be changed after its content is ready, otherwise some browsers+readers (like Chrome+AppleVoice) crash
$menu.attr('role', listPopupRole);
// make the primary element (focusable) aria-friendly
$focusable.attr({
'role': 'menu',
'aria-haspopup': 'menu',
'role': $focusable.attr('role') ?? focusableRole,
'aria-haspopup': listPopupRole,
'aria-controls': $menu.attr('id'),
'aria-expanded': 'false',
});
if ($dropdown.attr('data-content') && !$dropdown.attr('aria-label')) {
$dropdown.attr('aria-label', $dropdown.attr('data-content'));
}
$menu.find('> .item').each((_, item) => {
prepareMenuItem($(item));
});
// when showing, it has class: ".animating.in"
// when hiding, it has class: ".visible.animating.out"
const isMenuVisible = () => ($menu.hasClass('visible') && !$menu.hasClass('out')) || $menu.hasClass('in');
// update aria attributes according to current active/selected item
const refreshAria = () => {
const isMenuVisible = !$menu.is('.hidden') && !$menu.is('.animating.out');
$focusable.attr('aria-expanded', isMenuVisible ? 'true' : 'false');
const menuVisible = isMenuVisible();
$focusable.attr('aria-expanded', menuVisible ? 'true' : 'false');
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
$focusable.attr('aria-activedescendant', $active.attr('id'));
// if there is an active item, use it (the user is navigating between items)
// otherwise use the "selected" for combobox (for the last selected item)
const $active = $menu.find('> .item.active, > .item.selected');
// if the popup is visible and has an active/selected item, use its id as aria-activedescendant
if (menuVisible) {
$focusable.attr('aria-activedescendant', $active.attr('id'));
} else if (!isComboBox) {
// for menu, when the popup is hidden, no need to keep the aria-activedescendant, and clear the active/selected item
$focusable.removeAttr('aria-activedescendant');
$active.removeClass('active').removeClass('selected');
}
};
$dropdown.on('keydown', (e) => {
@ -85,16 +111,51 @@ function attachOneDropdownAria($dropdown) {
if (!$item) $item = $menu.find('> .item.selected'); // when dropdown filters items by input, there is no "value", so query the "selected" item
// if the selected item is clickable, then trigger the click event.
// we can not click any item without check, because Fomantic code might also handle the Enter event. that would result in double click.
if ($item && ($item.is('a') || $item.is('.js-aria-clickable'))) $item[0].click();
if ($item && ($item.is('a') || $item.hasClass('js-aria-clickable'))) $item[0].click();
}
});
// 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.
$focusable.on('focus', deferredRefreshAria);
$focusable.on('mouseup', deferredRefreshAria);
$focusable.on('blur', deferredRefreshAria);
// do not return any value, jQuery has return-value related behaviors.
// when the popup is hiding, it's better to have a small "delay", because there is a Fomantic UI animation
// without the delay for hiding, the UI will be somewhat laggy and sometimes may get stuck in the animation.
const deferredRefreshAria = (delay = 0) => { setTimeout(refreshAria, delay) };
$dropdown.on('keyup', (e) => { if (e.key.startsWith('Arrow')) deferredRefreshAria(); });
// if the dropdown has been opened by focus, do not trigger the next click event again.
// otherwise the dropdown will be closed immediately, especially on Android with TalkBack
// * desktop event sequence: mousedown -> focus -> mouseup -> click
// * mobile event sequence: focus -> mousedown -> mouseup -> click
// Fomantic may stop propagation of blur event, use capture to make sure we can still get the event
let ignoreClickPreEvents = 0, ignoreClickPreVisible = 0;
$dropdown[0].addEventListener('mousedown', () => {
ignoreClickPreVisible += isMenuVisible() ? 1 : 0;
ignoreClickPreEvents++;
}, true);
$dropdown[0].addEventListener('focus', () => {
ignoreClickPreVisible += isMenuVisible() ? 1 : 0;
ignoreClickPreEvents++;
deferredRefreshAria();
}, true);
$dropdown[0].addEventListener('blur', () => {
ignoreClickPreVisible = ignoreClickPreEvents = 0;
deferredRefreshAria(100);
}, true);
$dropdown[0].addEventListener('mouseup', () => {
setTimeout(() => {
ignoreClickPreVisible = ignoreClickPreEvents = 0;
deferredRefreshAria(100);
}, 0);
}, true);
$dropdown[0].addEventListener('click', (e) => {
if (isMenuVisible() &&
ignoreClickPreVisible !== 2 && // dropdown is switch from invisible to visible
ignoreClickPreEvents === 2 // the click event is related to mousedown+focus
) {
e.stopPropagation(); // if the dropdown menu has been opened by focus, do not trigger the next click event again
}
ignoreClickPreEvents = ignoreClickPreVisible = 0;
}, true);
}
export function attachDropdownAria($dropdowns) {

81
web_src/js/features/aria.md
View file

@ -1,4 +1,27 @@
**This document is used as aria/a11y reference for future developers**
# Background
This document is used as aria/accessibility(a11y) reference for future developers.
There are a lot of a11y problems in the Fomantic UI library. This `aria.js` is used
as a workaround to make the UI more accessible.
The `aria.js` is designed to avoid touching the official Fomantic UI library,
and to be as independent as possible, so it can be easily modified/removed in the future.
To test the aria/accessibility with screen readers, developers can use the following steps:
* On macOS, you can use VoiceOver.
* Press `Command + F5` to turn on VoiceOver.
* Try to operate the UI with keyboard-only.
* Use Tab/Shift+Tab to switch focus between elements.
* Arrow keys to navigate between menu/combobox items (only aria-active, not really focused).
* Press Enter to trigger the aria-active element.
* On Android, you can use TalkBack.
* Go to Settings -> Accessibility -> TalkBack, turn it on.
* Long-press or press+swipe to switch the aria-active element (not really focused).
* Double-tap means old single-tap on the aria-active element.
* Double-finger swipe means old single-finger swipe.
* TODO: on Windows, on Linux, on iOS
# Checkbox
@ -10,7 +33,8 @@ The ideal checkboxes should be:
<label><input type="checkbox"> ... </label>
```
However, related styles aren't supported (not implemented) yet, so at the moment, almost all the checkboxes are still using Fomantic UI checkbox.
However, related CSS styles aren't supported (not implemented) yet, so at the moment,
almost all the checkboxes are still using Fomantic UI checkbox.
## Fomantic UI Checkbox
@ -21,33 +45,52 @@ However, related styles aren't supported (not implemented) yet, so at the moment
</div>
```
Then the JS `$.checkbox()` should be called to make it work with keyboard and label-clicking, then it works like the ideal checkboxes.
Then the JS `$.checkbox()` should be called to make it work with keyboard and label-clicking,
then it works like the ideal checkboxes.
There is still a problem: Fomantic UI checkbox is not friendly to screen readers, so we add IDs to all the Fomantic UI checkboxes automatically by JS.
There is still a problem: Fomantic UI checkbox is not friendly to screen readers,
so we add IDs to all the Fomantic UI checkboxes automatically by JS.
If the `label` part is empty, then the checkbox needs to get the `aria-label` attribute manually.
# Dropdown
## ARIA Dropdown
## Fomantic UI Dropdown
Fomantic Dropdown is designed to be used for many purposes:
* Menu (the profile menu in navbar, the language menu in footer)
* Popup (the branch/tag panel, the review box)
* Simple `<select>` , used in many forms
* Searchable option-list with static items (used in many forms)
* Searchable option-list with dynamic items (ajax)
* Searchable multiple selection option-list with dynamic items: the repo topic setting
* More complex usages, like the Issue Label selector
Fomantic Dropdown requires that the focus must be on its primary element.
If the focus changes, it hides or panics.
At the moment, `aria.js` only tries to partially resolve the a11y problems for dropdowns with items.
There are different solutions:
* combobox + listbox + option
* menu + menuitem
At the moment, `menu + menuitem` seems to work better with Fomantic UI Dropdown, so we only use it now.
* combobox + listbox + option:
* https://www.w3.org/WAI/ARIA/apg/patterns/combobox/
* A combobox is an input widget with an associated popup that enables users to select a value for the combobox from
a collection of possible values. In some implementations, the popup presents allowed values, while in other implementations,
the popup presents suggested values, and users may either select one of the suggestions or type a value.
* menu + menuitem:
* https://www.w3.org/WAI/ARIA/apg/patterns/menubar/
* A menu is a widget that offers a list of choices to the user, such as a set of actions or functions.
```html
<div>
<input role="combobox" aria-haspopup="listbox" aria-expanded="false" aria-controls="the-menu-listbox" aria-activedescendant="item-id-123456">
<ul id="the-menu-listbox" role="listbox">
<li role="option" id="item-id-123456" aria-selected="true">
<a tabindex="-1" href="....">....</a>
</li>
</ul>
</div>
```
The current approach is: detect if the dropdown has an input,
if yes, it works like a combobox, otherwise it works like a menu.
Multiple selection dropdown is not well-supported yet, it needs more work.
Some important pages for dropdown testing:
## Fomantic UI Dropdown
* Home(dashboard) page, the "Create Repo" / "Profile" / "Language" menu.
* Create New Repo page, a lot of dropdowns as combobox.
* Collaborators page, the "permission" dropdown (the old behavior was not quite good, it just works).
```html
<!-- read-only dropdown -->

35
web_src/js/features/common-global.js
View file

@ -89,9 +89,14 @@ export function initGlobalCommon() {
// Semantic UI modules.
const $uiDropdowns = $('.ui.dropdown');
$uiDropdowns.filter(':not(.custom)').dropdown({
fullTextSearch: 'exact'
});
// do not init "custom" dropdowns, "custom" dropdowns are managed by their own code.
$uiDropdowns.filter(':not(.custom)').dropdown({fullTextSearch: 'exact'});
// The "jump" means this dropdown is mainly used for "menu" purpose,
// clicking an item will jump to somewhere else or trigger an action/function.
// When a dropdown is used for non-refresh actions with tippy,
// it must have this "jump" class to hide the tippy when dropdown is closed.
$uiDropdowns.filter('.jump').dropdown({
action: 'hide',
onShow() {
@ -101,17 +106,23 @@ export function initGlobalCommon() {
},
onHide() {
this._tippy?.enable();
// hide all tippy elements of items after a while. eg: use Enter to click "Copy Link" in the Issue Context Menu
setTimeout(() => {
const $dropdown = $(this);
if ($dropdown.dropdown('is hidden')) {
$(this).find('.menu > .item').each((_, item) => {
item._tippy?.hide();
});
}
}, 2000);
},
fullTextSearch: 'exact'
});
$uiDropdowns.filter('.slide.up').dropdown({
transition: 'slide up',
fullTextSearch: 'exact'
});
$uiDropdowns.filter('.upward').dropdown({
direction: 'upward',
fullTextSearch: 'exact'
});
// special animations/popup-directions
$uiDropdowns.filter('.slide.up').dropdown({transition: 'slide up'});
$uiDropdowns.filter('.upward').dropdown({direction: 'upward'});
attachDropdownAria($uiDropdowns);
attachCheckboxAria($('.ui.checkbox'));

3
web_src/js/features/repo-legacy.js
View file

@ -602,9 +602,6 @@ export function initRepository() {
}
function initRepoIssueCommentEdit() {
// Issue/PR Context Menus
$('.comment-header-right .context-dropdown').dropdown({action: 'hide'});
// Edit issue or comment content
$(document).on('click', '.edit-content', onEditContent);

3
web_src/js/modules/tippy.js
View file

@ -53,10 +53,11 @@ export function showTemporaryTooltip(target, content) {
onHidden: (tippy) => {
if (oldContent) {
tippy.setContent(oldContent);
tippy.setProps({onHidden: undefined});
} else {
tippy.destroy();
// after destroy, the `_tippy` is detached, it can't do "setProps (etc...)" anymore
}
tippy.setProps({onHidden: undefined});
},
});
}