-
-
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
Fix a11y for repo topic management #23346
Conversation
cd4fab5
to
902be92
Compare
38179a4
to
c379fc5
Compare
964760f
to
fa2fa02
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this case: If original topics are 'aaa' 'bbb' 'ccc' , and then click on "Manage topic", the aria-label
for the delete svg on all three topics will become "Remove topic "aaa""
Oops, my bad, fixed by 02c65fe |
5d48dcd
to
2754740
Compare
41c4972
to
a868de8
Compare
a868de8
to
e912d35
Compare
a2405c4
to
1fbff84
Compare
According to @fsologureng 's suggestion, always use "combobox", never use "menu" : 1fbff84 Just one-line change ( update: reverted |
To address some unclear (I don't understand) code-belonging concerns: #23450 The changes in 23450 are necessary for this PR. Without 23450, the dropdown doesn't work well on mobile. |
# Conflicts: # web_src/less/_repository.less
…m, tippy problem (#23450) 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> https://github.com/go-gitea/gitea/blob/d74a7efb60f94a4b8e6e5f65332f94f1be31b761/web_src/js/features/aria.md?plain=1#L38-L47 </details>
Which PR do you want to land first now, this one or #23542? I'm losing track. |
If I don't mention "block" or "merging order" in my PRs, then there is no order for them. This PR will be discard because this PR caused some arguments about "attribution of ideas and code". I will start new PRs from scratch, avoid touching unclear "idea" or "code". |
Partially replace #22631
This PR improves the a11y for the Repo Topic Edit feature.
Thanks to @fsologureng for the idea and help.
Major changes
A11y for Fomantic Dropdown
extracted to a separate PR: #23450
Repo Topic Edit
<button>
to replace<a>
, then theManage Topics
button is focus-able and clickable (by keyboard).<svg>
delete icon
elements have correct aria-related attributes (mainly for mobile)Now the Repo Topic Edit can be operated by keyboard-only.
And mobile screen readers (like Talkback) could recognize the
delete icon
as a button.Before
UI Misalignment & No Cancel Button
Delete Icon Misalignment
After
UI Aligned and Cancel Button
Delete Icon Aligned