Skip to content

fix: [M3-7477] - Fix accessibility on disabled SelectRegion menu items#9953

Merged
abailly-akamai merged 4 commits intolinode:developfrom
abailly-akamai:M3-7477
Dec 4, 2023
Merged

fix: [M3-7477] - Fix accessibility on disabled SelectRegion menu items#9953
abailly-akamai merged 4 commits intolinode:developfrom
abailly-akamai:M3-7477

Conversation

@abailly-akamai
Copy link
Contributor

@abailly-akamai abailly-akamai commented Dec 1, 2023

Description 📝

This PR addresses a bug/shortcoming of a disabled <ListItem /> within an <Autocomplete />.

The problem is that MUI treats the aria-disabled attribute in a list item as a non-focusable element.
see: mui/material-ui#33603

It's particularly problematic in our case because we do expect to be able to convey to the impaired user the reason why an element is disabled, just like we do for any other user. One of the way to remedy this is to remove the aria-disabled attribute and use a disabled one instead. Unfortunately in the case of a <li> element it is semantically wrong and not allowed.

The approach taken in this PR is to reinstate focusability by removing the aria-disabled (while preserving the onClick behaviors - or the lack thereof) and be extra verbose with the ListItem text. While not being the cleanest solution, it alows us to not break accessibility while waiting for MUI v6 or a proper fix to v5.

Changes 🔄

  • Restore accessibility to a disabled ListItem within Autocomplete
  • Handle onClick behavior to prevent regressions
  • Style item as a disabled one

Preview 📷

There is (or should be) no visual change introduced by this PR. The changes should be tested via VoiceOver or other related SR technology.

How to test 🧪

Prerequisites

  • pull code locally, have the DC Get Well flag and MSW turned on

Reproduction steps

  • On develop branch, try navigating the RegionSelect with voice over and keyboard. Notice disabled items are skipped.

Verification steps

  • Navigate (for instance) to http://localhost:3000/volumes/create
  • Turn on voice over
  • Navigate the region select with the keyboard
  • Confirm The disabled menu item read by the SR technology includes the copy related to the DC status

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@abailly-akamai abailly-akamai self-assigned this Dec 1, 2023
@abailly-akamai abailly-akamai marked this pull request as ready for review December 1, 2023 20:39
@abailly-akamai abailly-akamai requested a review from a team as a code owner December 1, 2023 20:39
@abailly-akamai abailly-akamai requested review from cliu-akamai, coliu-akamai, jaalah-akamai and jdamore-linode and removed request for a team December 1, 2023 20:39
Copy link
Contributor

@jdamore-linode jdamore-linode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Confirmed VoiceOver announces the "There may be limited capacity[...]" message when attempting to select Singapore. Tested in Chrome and Firefox!

@github-actions
Copy link

github-actions bot commented Dec 1, 2023

Coverage Report:
Base Coverage: 85.46%
Current Coverage: 85.39%

Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirmed voiceover works for chrome + firefox! ✅

I was having issues with it on safari (including eventually getting a 'Safari not responding' for everything). Not sure how much we can do about it though - seems like it might just be known that voicoever doesn't always work on Safari?

Unrelated to this pr, but another interesting thing I noticed is that the regions weren't sorted alphabetically within their continents on Firefox (but they are on chrome/safari)
image

@abailly-akamai
Copy link
Contributor Author

@coliu-akamai the safari voiceover issue isn't much of a concern to me but the sorting one on FF is odd so I'll make a ticket for it

@abailly-akamai abailly-akamai merged commit 434917d into linode:develop Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants