Aria tags and general accessibility update

JakeWags commented 10 months ago

Does this PR close any open issues?

Closes #275 Closes #265

Give a longer description of what this PR addresses and why it's needed

This PR adds aria-label tags to MANY elements in which a screen reader was not able to easily discern what the item/group is. This was tested with MacOS voiceover and a very limited understanding of screenreader usage. Tags such as aria-haspopup were used to notate items which cause popup actions.

Additionally, some sidebar elements such as the sorting and aggregation radio buttons were consolidated into more succinct groups (from the screenreader perspective) using aria-hidden, aria-label, and adding keypress listeners for better flow when tabbing through the application.

When importing an UpSet config state file, there were errors possible (wrong file type, etc), but these were invisible to a screenreader. aria-errormessage was added to the importer so that the Snackbar alerts would properly read when an import error occurred.

Also, the footer button configuration was altered so that there is no more custom element. The nature of the custom JSX element was causing issues with aria-label and nested group elements so the flow was not consistent for a screenreader. Instead, the buttons were just made into their respective semantic elements.

Also, the default configuration had both the alt-text and element view sidebar open at instantiation. Now, there are no sidebars open on-load.

Here are some assorted notes I took on aria tags while drafting this PR:

Notes for aria tags:

It is very important to be deliberate about where/when aria tags are used. Where HTML semantics suffice, use those instead.

aria-haspopup: menu, listbox, tree, grid, dialog, or true. Should we include this on buttons which open sidebars (show alt-text, Element View, etc)?

For elements like menu, the MUI elements (ie <Menu> , <MenuItem>) automatically have their respective roles applied, so this should not be overwritten

aria-label: this should be applied to any action which does not have a semantic label, or is not clear from the name/label of the element. This should also be used for any link navigation (aka. “Send email to VDL faculty” rather than vdl-faculty@sci.utah.edu)

aria-errormessage: should be put on any action which may trigger a user-readable error (i.e: importing an invalid UpSet state)

aria-live/aria-relevant: this can be used to declare that an element has updated. polite will not interrupt the current reader queue, while assertive will. aria-relevant dictates which updates should trigger the aria-live announcement. These can be additions, removals, text, all, or some combination. I’m unsure of how to add this to a modal or similar to indicate that it has been opened. Ideally for something like importing state, there should be an announcement to the screenreader post-import, but triggering this event may prove difficult as it doesn’t really add or remove any text or HTML elements. It could be possible to put a hidden element with the time of the last import and updating the import could alter the text, triggering the event. My concern with this method is that hacking around aria seems to be problematic at best.

role: this can be problematic at times, because this affects semantic context for some screenreaders when used incorrectly. However, for something like MUI’s Alert component, the info alert is being used as a styled information box. Since these have the role alert, screenreaders will read these on any update, if they are rendered and visible. To get around this while using the styling from the info alert, the role can be changed to generic. Since there is no real semantic context for the infobox, using the generic role makes sense, and eliminates unwanted behavior, such as reading the text any time there is a screen update.

Provide pictures/videos of the behavior before and after these changes (optional)

There should be no visual change

Are there any additional TODOs before this PR is ready to go?

TODOs:

[ ] MAYBE add aria-live updates to modals, sidebars, and menus on open. I'm not sure exactly how this works, or even if it is supposed to happen for these elements.

netlify[bot] commented 10 months ago

Deploy Preview for upset2 ready!

Name	Link
Latest commit	053afbc294a6001e508cc603fa937324b82c5c3d
Latest deploy log	https://app.netlify.com/sites/upset2/deploys/6568e525d56c600008398f77
Deploy Preview	https://deploy-preview-276--upset2.netlify.app/
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

JakeWags commented 10 months ago

Changing the sort label in the datatable from Sort by ASC isn't possible without significant CSS hacking, which results in non-screenreader accessible text. The solution to this would be to create a custom implementation of the datatable rather than using MUI's DataGrid. This is possible, as the tables aren't particularly complicated, but I don't think this is worth the development time.

visdesignlab / upset2