Skip to content

Commit

Permalink
remove unrelated code and comments, add more documents
Browse files Browse the repository at this point in the history
  • Loading branch information
wxiaoguang committed Mar 17, 2023
1 parent a040544 commit e42624a
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 55 deletions.
10 changes: 0 additions & 10 deletions web_src/js/features/aria.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,41 +127,31 @@ function attachOneDropdownAria($dropdown) {
// * 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
// keep the debug code for developers who want to confirm&debug this code for different browsers (without attaching a remote debugger)
const showDebug = false;
const debug = (msg) => showDebug && $('.page-content').append($('<div>').text(`${$menu.attr('id')} ${msg}, menu visible=${isMenuVisible()}`));
let ignoreClickPreEvents = 0, ignoreClickPreVisible = 0;
$dropdown[0].addEventListener('mousedown', (e) => {
debug(e.type);
ignoreClickPreVisible += isMenuVisible() ? 1 : 0;
ignoreClickPreEvents++;
}, true);
$dropdown[0].addEventListener('focus', (e) => {
debug(e.type);
ignoreClickPreVisible += isMenuVisible() ? 1 : 0;
ignoreClickPreEvents++;
deferredRefreshAria();
}, true);
$dropdown[0].addEventListener('blur', (e) => {
debug(e.type);
ignoreClickPreVisible = ignoreClickPreEvents = 0;
deferredRefreshAria(100);
}, true);
$dropdown[0].addEventListener('mouseup', (e) => {
debug(e.type);
setTimeout(() => {
debug(`${e.type} (deferred)`);
ignoreClickPreVisible = ignoreClickPreEvents = 0;
deferredRefreshAria(100);
}, 0);
}, true);
$dropdown[0].addEventListener('click', (e) => {
debug(`${e.type}, pre-visible=${ignoreClickPreVisible}, pre-events=${ignoreClickPreEvents}`);
if (isMenuVisible() &&
ignoreClickPreVisible !== 2 && // dropdown is switch from invisible to visible
ignoreClickPreEvents === 2 // the click event is related to mousedown+focus
) {
debug(`${e.type}, stop click propagation`);
e.stopPropagation(); // if the dropdown menu has been opened by focus, do not trigger the next click event again
}
ignoreClickPreEvents = ignoreClickPreVisible = 0;
Expand Down
102 changes: 57 additions & 45 deletions web_src/js/features/aria.md
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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

Expand All @@ -21,64 +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

There are different solutions:

* combobox + listbox + option
* menu + menuitem

```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>
```


## Fomantic UI Dropdown

There are 2 possible solutions about the role: combobox or menu
Fomantic Dropdown is designed to be used for many purposes:

1. Detect if the dropdown has an input, if yes, it works like a combobox, otherwise it works like a menu
2. Always use "combobox", never use "menu"
* 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

According to the public discussion with fsologureng in chat channel:
Fomantic Dropdown requires that the focus must be on its primary element.
If the focus changes, it hides or panics.

> 2023-03-13 14:52:00 UTC
>
> wxiaoguang: Yup, the "language dropdown" is arguable (menu or combobox). It's the only one we need to mark it as combobox if you strongly prefer.
> But for others (Create Repo / Profile / Reaction Menu / Issue Context Menu), I think they are not suitable to be combobox.
>
> fsologureng: I repeat an argument stated in my first comment: "On the old web there were many menus implemented with an auto-submit select,
> but that didn't change the fact that they were selects for screen readers.
> That is the case with Fomantic dropdowns as used in Gitea.
> Implementations of auto-submit select menus fell behind in modern web design precisely because they are not usable or accessible."
>
> wxiaoguang: If you strongly prefer, we can mark all "dropdown" as "combobox", never use "menu" in code. Do you think this solution is clear enough?
>
> fsologureng: Yes. I think it will provide better accessibility because is more coherent with the current fomantic based implementation.
At the moment, `aria.js` only tries to partially resolve the a11y problems for dropdowns with items.

Reference:
There are different solutions:

* Combobox:
* 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 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:
* 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**.
* A menu is a widget that offers a list of choices to the user, such as a set of actions or functions.

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:

* 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 -->
Expand Down

0 comments on commit e42624a

Please sign in to comment.