-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The add reaction button and the comment options button are not accessible in the web view of an issue #21742
Comments
lunny
added a commit
that referenced
this issue
Feb 24, 2023
) As the title. Label/assignee share the same code. * Close #22607 * Close #20727 Also: * partially fix for #21742, now the comment reaction and menu work with keyboard. * partially fix for #17705, in most cases the comment won't be lost. * partially fix for #21539 * partially fix for #20347 * partially fix for #7329 ### The `Enter` support Before, if user presses Enter, the dropdown just disappears and nothing happens or the window reloads. After, Enter can be used to select/deselect labels, and press Esc to hide the dropdown to update the labels (still no way to cancel .... maybe you can do a Cmd+R or F5 to refresh the window to discard the changes .....) This is only a quick patch, the UX is still not perfect, but it's much better than before. ### The `confirm` before reloading And more fixes for the `reload` problem, the new behaviors: * If nothing changes (just show/hide the dropdown), then the page won't be reloaded. * If there are draft comments, show a confirm dialog before reloading, to avoid losing comments. That's the best effect can be done at the moment, unless completely refactor these dropdown related code. Screenshot of the confirm dialog: <details> ![image](https://user-images.githubusercontent.com/2114189/220538288-e2da8459-6a4e-43cb-8596-74057f8a03a2.png) </details> --------- Co-authored-by: Brecht Van Lommel <brecht@blender.org> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
yardenshoham
pushed a commit
to yardenshoham/gitea
that referenced
this issue
Feb 24, 2023
…gitea#23014) As the title. Label/assignee share the same code. * Close go-gitea#22607 * Close go-gitea#20727 Also: * partially fix for go-gitea#21742, now the comment reaction and menu work with keyboard. * partially fix for go-gitea#17705, in most cases the comment won't be lost. * partially fix for go-gitea#21539 * partially fix for go-gitea#20347 * partially fix for go-gitea#7329 ### The `Enter` support Before, if user presses Enter, the dropdown just disappears and nothing happens or the window reloads. After, Enter can be used to select/deselect labels, and press Esc to hide the dropdown to update the labels (still no way to cancel .... maybe you can do a Cmd+R or F5 to refresh the window to discard the changes .....) This is only a quick patch, the UX is still not perfect, but it's much better than before. ### The `confirm` before reloading And more fixes for the `reload` problem, the new behaviors: * If nothing changes (just show/hide the dropdown), then the page won't be reloaded. * If there are draft comments, show a confirm dialog before reloading, to avoid losing comments. That's the best effect can be done at the moment, unless completely refactor these dropdown related code. Screenshot of the confirm dialog: <details> ![image](https://user-images.githubusercontent.com/2114189/220538288-e2da8459-6a4e-43cb-8596-74057f8a03a2.png) </details> --------- Co-authored-by: Brecht Van Lommel <brecht@blender.org> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
lunny
added a commit
that referenced
this issue
Feb 24, 2023
) (#23102) Backport #23014 As the title. Label/assignee share the same code. * Close #22607 * Close #20727 Also: * partially fix for #21742, now the comment reaction and menu work with keyboard. * partially fix for #17705, in most cases the comment won't be lost. * partially fix for #21539 * partially fix for #20347 * partially fix for #7329 ### The `Enter` support Before, if user presses Enter, the dropdown just disappears and nothing happens or the window reloads. After, Enter can be used to select/deselect labels, and press Esc to hide the dropdown to update the labels (still no way to cancel .... maybe you can do a Cmd+R or F5 to refresh the window to discard the changes .....) This is only a quick patch, the UX is still not perfect, but it's much better than before. ### The `confirm` before reloading And more fixes for the `reload` problem, the new behaviors: * If nothing changes (just show/hide the dropdown), then the page won't be reloaded. * If there are draft comments, show a confirm dialog before reloading, to avoid losing comments. That's the best effect can be done at the moment, unless completely refactor these dropdown related code. Screenshot of the confirm dialog: <details> ![image](https://user-images.githubusercontent.com/2114189/220538288-e2da8459-6a4e-43cb-8596-74057f8a03a2.png) </details> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Brecht Van Lommel <brecht@blender.org> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
Hello,
In each issue view to the right side of the issue's own comment and for each comment made there is a button to make reactions and another button for other options like reply, cite, etc.
Those buttons use foomantic dropdowns and are processed by the recently added
aria.js
script to make it accessible, but it doesn't work because the html markup is incompatible.The particular problem
Steps to reproduce for a sighted person:
Expected behavior: This reaction is added to the post.
Actual behavior: The reaction menu appears when focusing and it is possible to select items, but when pressing the ENTER key on an emoji nothing happens.
The same happens with the comment options button as well.
Any blind person can't even understand the name of the buttons because they are not labelled with aria.
The main problem
As a general solution I think adding html markup via javascript is a bad idea because in issues with large threads, the browser performance gets worse in loading as the page length increases. Also, the browser has parsed all the markup and has the corresponding DOM ready when it has to start again to change much part of its structure.
Perhaps for people with powerful machines this is not a big problem, but there is a lower limit thread length for any machine where performance degrades considerably. The better the DOM handling, the higher that lower limit will be.
Therefore, I think it's better to add the semantic aria markup directly to the templates and reserve JavaScript to add only the functionality needed to operate the controls where the markup can't do it on its own. This will also benefit the work of visual designers, as they can understand the meaning of the markup without digging into the JavaScript code.
Also, following this principle always result in better performance: less time to build the DOM, less JavaScript code, less network transfer, less bandwidth used by the server.
I suggest following this principle as a rule of thumb for dealing with a11y semantics (and other similar too).
A solution
I have developed a partial fix to make the add reaction button and the comment options button accessible following the principles detailed above, but the problem of colour contrast remains open.
Moreover, the markup used by foomantic dropdowns in Gitea is slightly different to that expected for menu roles, as there is no button assigned to toggle the display of menu options (see https://www.w3.org/WAI/ARIA/apg/example-index/menu-button/menu-button-links). Instead, the entire div container is assigned the menu role without a control button. A change to add this component I think is much deeper than what is being done here. If I'm wrong about this, please comment.
From now on the
attachOneDropdownAria
function should be replaced by the forkedattachOneDropdownAriaTemplated
function as long as every template behind a foomantic dropdown is being prepared with aria markup.Final thought
Is there a lot of work to be done on the Gitea web interface to make it usable for people with visual impairments. Although the perception of the main problem is apparently different. I have seen more issues than mentioned here analysing the interface for this particular problem.
It is not just a question of functionality of the buttons, is that there are many colour definitions that are not designed to provide the necessary contrast levels for partially sighted people.
Unfortunately, I have not been able to find any report of the work that accessibility experts are said to have done with Gitea this year. Perhaps they have not finished yet?
So I have probably started to repeat the analysis work done by some people over the last few years on many reported issues.
I hope that suggestions made here are welcome.
Thanks for reading this far.
Screenshots
Firefox developer accessibility tool focusing add reaction button:
Capture of add reaction button with a warning about interactive semantics lack:
Firefox developer accessibility tools focusing add reaction button with a warning popup:
Gitea Version
dd7f1c0
Can you reproduce the bug on the Gitea demo site?
Yes
Operating System
Linux
Browser Version
Firefox 106.0.3
The text was updated successfully, but these errors were encountered: