In-progress accessibility fixes for screen reader users

opnsense / core

OPNsense GUI, API and systems backend

https://opnsense.org/

BSD 2-Clause "Simplified" License

3.22k stars 718 forks source link

In-progress accessibility fixes for screen reader users #7590

Open jfayre opened 2 months ago

jfayre commented 2 months ago

I am working on fixing the major accessibility issues that make OPNSense very difficult for someone who is totally blind to use. The following pull requests have been submitted. Items that are checked off have been merged into master:

[x] #7587 - added aria-label attribute to users list table.
[x] #7588 - fixes to firewall rules pages
[x] #7589 - removes aria-hidden attribute from MVC base dialogs. Aria-hidden makes the dialog invisible to screen readers. Bootstrap modified to remove aria-hidden globally. Now available in 24.1.10!
[x] #7593
[x] #7599 - as part of this PR, Bootstrap code was modified to add aria-label to all tooltips.
[x] #7617
[ ] #7619
[x] #7617
[ ] #7620

things to do:

[x] add accessible form labels to MVC forms
[x] fix aria-hidden dialog on firmware update page. Makes changelogs unreadable by screen readers.
[ ] Add accessible form labels to Interfaces.php forms. This is a larger commit so will do this by sections.
[ ] fix firewall nat page

jfayre commented 2 months ago

@AdSchellevis In one of my pull requests, you mentioned that you would look at updating Bootstrap to fix the aria-hidden attributes on modal dialogs. Any idea if this will make it into the 24.7 release? Thanks!

AdSchellevis commented 2 months ago

@jfayre almost forgot, https://github.com/opnsense/core/commit/cf6359c8a31561b2dc26a2c643cd5a561e156ccf should do the trick

jfayre commented 2 months ago

@AdSchellevis I don't think this worked. When I delete a firewall rule, such as a port forward, the aria-hidden attribute is still set on the modal. Aria-hidden should be removed or set to false in order for the dialog to be available to screen readers. In looking at this a bit further, it looks like bootstrap-dialog.js may be fixed, but the bootstrap-dialog.min.js file appears to be the old one. Is there something I need to do to make this live in the UI? Javascript isn't my strong point.

AdSchellevis commented 2 months ago

@jfayre my mistake, forgot the minified file. https://github.com/opnsense/core/commit/82085173b8cb9b8188b42f1f24be7d960f22c9bb should do the trick

jfayre commented 2 months ago

Thanks! That worked. Do we have any idea when other major sections of the UI will be moved to MVC? I'm thinking specifically of the firewall rules, nat, etc. In the current structure of those pages, the screen reader is interpreting the action buttons in the top right of the table as a column header. This makes the screen reader read those buttons every time you navigate into that column on any of the rows.

AdSchellevis commented 2 months ago

It's a long term process, we replaced a lot already but are certainly not finished.

jfayre commented 2 months ago

Ok no problem. I'll see what I can do to fix those tables. When you have a second, can you take a look at PR 7593? This is to add accessible form labels to MVC forms.

AdSchellevis commented 2 months ago

7593 is still on my list of things todo

jfayre commented 1 month ago

The bootstrap fixes are working great from what I can tell so far. It looks like this didn't catch everything though. At the top of the various tables, there are buttons to refresh, sort, and change the number of results displayed. These buttons don't have proper labels. I can't find where these elements are created. Can someone advise? Thanks!