Skip to content
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

Try to improve a11y for the "button groups" in the SecondaryToolbar/Sidebar (issue 14526) #14554

Merged
merged 1 commit into from
Mar 11, 2022

Conversation

Snuffleupagus
Copy link
Collaborator

Please note: I don't really know anything about a11y, hence it's possible that this patch either doesn't work correctly or at least isn't a complete solution.

In both the SecondaryToolbar and the Sidebar we have "button groups" that functionally acts essentially like radio-buttons. Based on skimming through this MDN article it thus appears that we should tag them as such, using role="radiogroup" and role="radio", and then utilize the aria-checked attribute to indicate to a11y software which button is currently active.

@brendandahl
Copy link
Contributor

Were you able to test this with a screen reader or the a11y tree in Firefox?

@Snuffleupagus
Copy link
Collaborator Author

Were you able to test this with a screen reader or the a11y tree in Firefox?

Well, I've got no screen-reader unfortunately.
When looking at the "Accessibility" panel of the Devtools I'm not entirely sure what it's supposed to look like, however the button-groups now end up under their respective grouping: categories and the "checked" attributes seem to toggle as expected.

@Snuffleupagus Snuffleupagus marked this pull request as ready for review February 18, 2022 10:00
@Snuffleupagus Snuffleupagus force-pushed the issue-14526 branch 2 times, most recently from 4a3cfa2 to 3225757 Compare February 20, 2022 15:43
@mozilla mozilla deleted a comment from pdfjsbot Feb 21, 2022
@mozilla mozilla deleted a comment from pdfjsbot Feb 21, 2022
@Snuffleupagus
Copy link
Collaborator Author

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/d6b1fac070aeff2/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/d6b1fac070aeff2/output.txt

Total script time: 2.61 mins

Published

@Snuffleupagus
Copy link
Collaborator Author

@seahawksq Please check if the preview above is enough to address the issue!
If that doesn't work, with a screen-reader, can you please provide any information about what we need to do differently?

@marco-c
Copy link
Contributor

marco-c commented Mar 3, 2022

@jcsteh maybe you can help here?

…idebar (issue 14526)

*Please note:* I don't really know anything about a11y, hence it's possible that this patch either doesn't work correctly or at least isn't a complete solution.

In both the SecondaryToolbar and the Sidebar we have "button groups" that functionally acts essentially like radio-buttons. Based on skimming through [this MDN article](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/radio_role) it thus appears that we should tag them as such, using `role="radiogroup"` and `role="radio"`, and then utilize the `aria-checked` attribute to indicate to a11y software which button is currently active.
@brendandahl
Copy link
Contributor

Looks like this fixes the issue

Here's the output of NVDA:

Before:

Tools  button  collapsed  
expanded
Go to Last Page  button  
Rotate Clockwise  button  
Rotate Counterclockwise  button  
Text Selection Tool  button    Enable Text Selection Tool
Hand Tool  button    Enable Hand Tool
Page Scrolling  button    Use Page Scrolling
Vertical Scrolling  button    Use Vertical Scrolling
Horizontal Scrolling  button    Use Horizontal Scrolling
Wrapped Scrolling  button    Use Wrapped Scrolling
No Spreads  button    Do not join page spreads
Odd Spreads  button    Join page spreads starting with odd-numbered pages
Even Spreads  button    Join page spreads starting with even-numbered pages
Document Properties…  button 

After:

Tools  button  collapsed  
expanded
Go to Last Page  button  
Rotate Clockwise  button  
Rotate Counterclockwise  button  
Text Selection Tool  radio button  checked  Enable Text Selection Tool  1 of 2
Hand Tool  radio button  not checked  Enable Hand Tool  2 of 2
Page Scrolling  radio button  not checked  Use Page Scrolling  1 of 4
Vertical Scrolling  radio button  checked  Use Vertical Scrolling  2 of 4
Horizontal Scrolling  radio button  not checked  Use Horizontal Scrolling  3 of 4
Wrapped Scrolling  radio button  not checked  Use Wrapped Scrolling  4 of 4
No Spreads  radio button  checked  Do not join page spreads  1 of 3
Odd Spreads  radio button  not checked  Join page spreads starting with odd-numbered pages  2 of 3
Even Spreads  radio button  not checked  Join page spreads starting with even-numbered pages  3 of 3
Document Properties…  button

@Snuffleupagus Snuffleupagus merged commit 4318bc8 into mozilla:master Mar 11, 2022
@Snuffleupagus Snuffleupagus deleted the issue-14526 branch March 11, 2022 19:46
@seahawksq
Copy link

It has been fixed, thanks for the quick turnaround!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Secondary Toolbar]: Screen reader does not announce the selected/unselected state information for the buttons
5 participants