mirror of
https://codeberg.org/forgejo/forgejo
synced 2024-12-01 22:36:10 +01:00
Fix aria.js bugs: incorrect role element problem, mobile focus problem, tippy problem (#23450) (#23486)
Before: the `aria.js` is still buggy in some cases. After: tested with AppleVoice, Android TalkBack (I tested it with 1.19 again) * 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 * Fix incorrect tippy `setProps` after `destroy` * Improve comments * Implement the layout proposed by #19861
This commit is contained in:
wxiaoguang
GitHub
parent
22911a1ece
commit
420d015b76
|
|
@ -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>
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
{{if .ctx.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>
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
{{if .ctx.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>
|
||||
|
|
|
|||
|
|
@ -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
|
||||
// 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) {
|
||||
|
|
|
|||
|
|
@ -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 -->
|
||||
|
|
|
|||
|
|
@ -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'));
|
||||
|
|
|
|||
|
|
@ -601,9 +601,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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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});
|
||||
},
|
||||
});
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue