Repair some Gitea accessibility issues

[fsologureng]

Updated 2023-03-22 removing old ideas and merges done apart.

This PR is an improvement of #21743 taking the combobox+listbox ARIA design pattern as a better fit for some Fomantic dropdowns, with the addition of other a11y improvements.

The idea is to resolve the main problem described in #21742.

Changed foomantic dropdowns to the combobox+listbox ARIA design pattern

• Changed scope application of aria.js functions to apply it after all widgets are initialized.
• Fixes and improve a11y structure of:
  • Reactions in issue/pr comments view.
  • Comment options in issue/pr comments view.
  • Milestone dropdown in repo issue/pr view.
  • Projects dropdown in repo issue/pr view.
  • Assignees dropdown in repo issue/pr view.
  • Reviewers dropdown in repo pr view.
  • Due date dropdown in repo issue/pr view.
  • Dependency dropdown in repo issue/pr view.
  • Reference copy button in repo issue/pr view.
  • Language selector.
  • Create... button.
  • Profile and settings... button.
• Fixes lack of aria label of Close issue button.
• Add label for some buttons.
• Remove dropdown class to an svg which generates false positives

[silverwind]

This looks like a bunch of good changes, sorry for not picking it up earlier.

Maybe @wxiaoguang can have a look too.

[silverwind]

Yeah I already commented above on the unnecessary IDs. If we can get the rendering to work with nesting, it's definitely the way to go. If this change is too massive, I would suggest factoring it out into a separate PR to ease review here.

[fsologureng]

Although generating IDs dynamically affects performance slightly, I think it's acceptable and better than writing unnecessary IDs everywhere.

I don't understand why you say unnecessary IDs everywhere, are all those IDs added by aria.js unnecessary? If they are unnecessary, why are they added?

If the current markup of this input+labels pair are not accessible, why would it be more difficult to change them with id+for attributes added towards a nested schema? sounds illogical; that's a deep change with or without attributes, includes templates, css and UI framework.

In the future, these components could be replaced by some well-designed a11y ones, still do not force developers to manually write every ID for the elements in dropdowns.

In the future for a11y solutions is practically never. Why not implement accessible solutions as soon as possible?
Why are developers and contributors exempt from knowing about accessibility? Why not about css, javascript or UI frameworks? Why shouldn't a web developer know about ARIA?
ARIA was invented to customise web controls and user interaction, to refresh the static and boring accessible world of simple selections and ugly checkboxes. Why not embrace it? It's pure web design. It's not that difficult.

In many institutions it is essential to provide accessibility, even in financial institutions, so why is it seemingly unnecessary here?

[fsologureng]

Do you promise to pay all attention to all future commits to make sure all of them work properly with a11y?

I will do my best effort to provide accessibility.

why the code looks like this at the moment?
Because there are strong incentives in order to develop without taking in account accessibility.

---

To resolve the problem fundamentally, IMO there could be an intermediate framework to fix a11y problems automatically, and then replace buggy Fomantic UI components with modern/native ones -- instead of forcing developers to write IDs/aria-labels again and again.

So developers will never have to deal with IDs nor ARIA stuff in templates, but they have to develop that intermediate framework you propose, dealing with IDs and ARIA stuff on pure javascript. The problem I see is who is going to develop that intermediate framework if, in your words, developers aren't called to deal with ARIA stuff?

Also, developers will have to choose a good UI framework that provide good accessibility and implement that framework on top of gitea, but without knowing too much about a11y, it sounds a complex decision. Who, without knowing too much about a11y, are willing to choose a good UI framework like the one you describe for the whole frontend of gitea while ensuring accessibility?

On the other hand, with your last statements, I start to understand that aria.js is a kind of prototype of that intermediate framework that frees developers from dealing with ARIA and IDs in templates and fixes foomantic UI bugs in dropdowns. But I have opened a bug a month ago and nobody (zero) person has even reacted (less confirmed) anything about it. What is the commitment to this approach?

Finally, the checkboxes on the configuration page don't have an accessibility problem due to foomantic UI, it's a simple case of malformed html (not the only one in that template). If you want to preserve a malformed html template while you find a good framework fixing the html with javascript, well I'm not going to provide it. It's easy to see why checkboxes are siblings with their labels in foomantic (and in other frameworks like Vue, too). Adding the ID and FOR attrs is a straightforward native solution if you remove the fomantic UI from them.

[fsologureng]

@wxiaoguang what do you think about this aria.js refactory proposal?

[fsologureng]

I came here for asking why this change left the cancel button acting as submit button because I couldn't find an explanation debugging, but I saw #23337 and I am afraid I will need to revert it or wait to @delvh's pr being merged.

[fsologureng]

@wxiaoguang what do you think about this aria.js refactory proposal?

Some problems have been addressed by other PRs, some problems have more details and backgrounds now, the comments in this PR are quite a few, maybe we can start a fresh discussion based on the latest situation?

(update: I hide my previous comments as out-dated, indeed they are out-dated)

* I always agree to improve (anything)  if the refactoring doesn't introduce maintainability problems.

* For `aria.js`,  the major  problem of it at the moment is it always uses `menu + menuitem`, in the near future, if the dropdowns are still there, as a hacky patch, `aria.js` could use `combobox` instead of `menu` when the dropdowns have input elements.

About latest changes (just a quick and brief review)

* Could these `tabindex="0"` for buttons be removed? By default they are focus-able.

done

* About `js-role-combobox`
  
  * I think the "language" menu is a menu, it shows the menu item, if you click any one, then page refreshes, it doesn't look like a combobox which provides value for the form.

menu role doesn't have any element except the menuitem set. Language selector has the current value as a text which appears to be a readonly input. This is why I implemented as a combobox.

  * In most cases, IMO we do not need  the `js-role-combobox`, because if there is a `input` in dropdown, then it must be a combobox.
  * Update: according to new changes in `#23346`, I think deciding the role by `input` is doable.

Unfortunately, that approach is totally insufficient IMO. There are more structures than those two. In fact, the unique selectors which match a menu pattern are the reactions menu and the comment options menu (the only pair you didn't fix with aria.js at the beginning, btw). You can check this statement using the Axe DevTools extension with a fomantic dropdown opened. The majority of dropdowns have a kind of label in the middle incompatible with a menu pattern and have noise produced by elements as options headers and dividers which I am addressing here.

As I said in Forgejo Dropdowns UX bug some days ago, keep talking about dropdowns as a whole IMO is a bad approach and is bad aligned with your proposal made in #23345. If you observe each widget implemented with Fomantic dropdown there are little differences that doesn't fit well in a rigid overview. That's why I didn't took the presence of an input as a final indicator of a combobox. The worst case is the tag/branch selector, which, as far as I can see, is a unique totally inaccessible structure.

This is the reason why I chosen to do specific identification of aria roles via js- prefixed classes and do a specific implementation accordingly. Is extensible and compatible with a step by step proposal. IMO the dependencies selector is the most near structure to an accessible combobox, I think that change some sidebar selectors to that structure will provide better accessibility of them.

The language selector is pretty close to be accessible, but auto-submit is a bad practice which can produce annoying interactions if you choose bad your desired language. Ideally you should be able to choose your language and then submit it to change the UI with clarity that the choosing was made without a mistake. That's the reason why combobox has a fillable field. If you want just a menu structure, you should put the current language label out of the selector or remove the label and only mark the current language in the corresponding menuitem (as a native selector does).

* `<div class="ui button" ...` could be `<button ...` then no need to add `tabindex`?

done

  * I think the `<button>` problem will be completely fixed by [Convert `<div class="button">` to `<button class="button">` #23337](https://github.com/go-gitea/gitea/pull/23337) (@delvh)

I made a change here that needs to wait a merge of that pull request or be reverted because is failing currently in the cancel button of the locking issue modal.

* `<button class="delete-dependency-button tooltip ci muted"`
  
  * I am not sure about the style because I haven't tested this change, but I am sure `muted` is not needed because `muted`  is for `a` only.

Agreed. I just opted to conserve classes. Is now removed.

* The `class="dropdown icon"`  classes on SVG elements in dropdowns  is done by purpose to fine-tune the UI alignment, ~there are CSS styles for them, I do not think the `dropdown` could be removed (I haven't tested, just a guess)~
  
  * Checked, for other SVG icons, the `svg dropdown icon` is done by purpose.
  * For this one, it's better to remove the `dropdown` class. This icon already has its own `gt-mt-...` styles.

* ~`attachDropdownAria($('.ui.dropdown'));` doens't seem right. We should skip `custom`  dropdown elements, these `custom` elements have their own JS to work.~
  
  * Hmm ... old code also did so, maybe I didn't realize this problem at that time.

* The new JS code: I will take a look carefully later. A quick thought in my mind: maybe we need two functions, one for `menu` , one for `combobox` (they could share  some  code  by common functions), to make them independent, then any change on one side won't affect another one, and it's easier to maintain and test in the future.

That's what I said above. If you implement combobox based in input presence IMO you will produce more technical debt.

  * I made some new changes in [Fix accessibility behavior for topic management in the repository home, make some dropdown work as combobox #23346](https://github.com/go-gitea/gitea/pull/23346)
  * According to [w3 demo](https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-autocomplete-none/), `combobox` is enough, no need to use `searchbox`

Perfect, but you should need to remove the label inside because is out of that pattern as I explained above with language selector.

  * Most code could be shared between `combobox` and `menu`

It depends. Several dropdowns have the input inside the menu container, which is out of accessible standard.

  * After some quick tests, I think these changes work well for many cases.
  * If `#23346` is good enough, could we continue to work on it?

* Update: one more thing, the `item` shouldn't be clicked without any check. See the comment in:
  
  * [Fix incorrect display for comment context menu  #23343](https://github.com/go-gitea/gitea/pull/23343)

This reversion was already integrated today.

[wxiaoguang]

More proofs about "why the language dropdown should be a menu (at the moment)".

• If we define the behavior of the language dropdown as: click item, then switch, it's a perfect menu

• If we define the behavior of the language dropdown as: choose item, hide, then click a separate button to switch: it should be a comobox. However, that's not how the dropdown works at the moment. If you could propose a new change to add a separate confirm button, then the language dropdown could be changed to a combobox.

Take a look at W3 standard:

• Combobox:
  • 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:
  • 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.

And take a look at Apple's page (maybe Apple also doesn't do perfectly, but they also use "menu")

After a public discussion, the conclusion is: it's possible to always use "combobox", never use "menu" for Fomantic Dropdowns (it's doable and acceptable, while in my mind the "Create Repo" / "Profile" are menus - it doesn't matter).

I marked my previous comments as Outdated.

---

Update: although some of my recent PRs (#23450, #23542) conflict with your work, I never meant to "take over" your work or idea. I highly respect your work and idea, and I believe most of them are pretty valuable.

My PRs are doing: fix my bugs, make the aria.js more maintainable, introducing general approaches to handle all dropdowns transparently. After #23542 , the code structure would be clearer than before, it might be easier for you to add more improvements to it.

If you feel that: something in my PRs is wrong, or I missed something, or I need to mention something, or I should help to resolve the conflicts, don't hesitate to tell me.

[wxiaoguang]

Stale for long time, I think I have been open and clear for this problem.

Feel free to reopen or propose new PRs, thank you.