eui icon indicating copy to clipboard operation
eui copied to clipboard

[EuiButton] Better communicate `isLoading` to assistive tech

Open myasonik opened this issue 4 years ago • 8 comments

Issue

Loading states aren't communicated to screen readers

Fix

  1. Include <span role="status" aria-atomic="true"> with buttons Why role="status"? This role acts as a live region that will allow us to notify assistive tech that something changed without a user having to do anything, even if they've navigated away from the button. Why aria-atomic="true"? This will tell the assistive tech to read the whole message instead of just the word that changed. (Because we don't know what content will be in the button, this is the safer option.)

  2. If/When isLoading changes, pipe the new button text into the <span> we setup If the text didn't change, make some up by appending "is loading" to the button text when isLoading=true

  3. Clear the <span> text after 2 seconds Even though we set aria-atomic="true" above, support is mixed so it's good practice to clear the text if you want to help ensure you're reading the whole string. Why 2 seconds? Because there's no way to know when the assistive tech might have finished reading it so it's a good guess at "this should have finished by now..."

Example DOM

Initial render

<EuiScreenreaderOnly><span role="status" aria-atomic="true"></span></EuiScreenreaderOnly>
<button>Click me to load data</button>

Button is loading

<EuiScreenreaderOnly><span role="status" aria-atomic="true">Loading data...</span></EuiScreenreaderOnly>
<button>Loading data... </button>

Button is done loading

<EuiScreenreaderOnly><span role="status" aria-atomic="true">Your data is ready</span></EuiScreenreaderOnly>
<button disabled>Your data is ready</button>

More details

https://adrianroselli.com/2021/01/multi-function-button.html

myasonik avatar May 13 '21 22:05 myasonik

I think this is a good instance just for better docs. With every new "type" of button we create, we increase the likelihood of it being used incorrectly. Just give them a copy/paste-able pattern. This also teaches them the right way to handle these situations, which are more on the rare side.

cchaos avatar May 14 '21 13:05 cchaos

At the same time if loading is really a state that always needs to be communicated in a particular way. Since we already have a dedicated isLoading prop, we should just add that extra functionality in the existing buttons.

cchaos avatar May 14 '21 13:05 cchaos

Ah, I totally forgot we already support a isLoading prop! Updated the issue for our existing setup.

myasonik avatar May 14 '21 16:05 myasonik

Some clarification questions.

<span role="status" aria-atomic="true">

  1. Where does this a new span live? Inside the button, or next to the button?
  2. Is it screenReaderOnly?
  3. Does it need to contain some text? If so, what?

Basically, what's the exact setup here?

Clear the <span> text after 2 seconds

Why clear after a certain time period and not when the isLoading prop is removed / set to false?

cchaos avatar May 14 '21 19:05 cchaos

  1. Sibling to the button, not inside (all button contents is collapsed into a simplified role that's plain text "button content" so we can't have anything inside.
  2. Yes! (Well, doesn't have to be, but I'm assuming that's the desired setup here.)
  3. On load, it doesn't need any text as it only needs to report when the button changes.

Basically, what's the exact setup here?

I added some DOM examples to the issue description that I hope help.

Why clear after a certain time period and not when the isLoading prop is removed / set to false?

Let's say the loading text is "Dashboards are loading" and then the default text is "Load more Dashboards".

Without removing the text, depending on the assistive tech being used, the status will say "Load more" and ignore the word "Dashboards" because it will think that the word didn't change. aria-atomic="true" should prevent that from happening but support isn't perfect.

Clearing the text behind us also leaves a cleaner DOM for assistive tech. (Assistive tech can navigate to the status message but it just repeats the button text so a user navigating through the page, if we didn't clear it, would come across the same message twice.)

myasonik avatar May 14 '21 19:05 myasonik

Hmm, the problem right now is the current isLoading: boolean prop only has truly one unique state, true. We won't want to add this span to every EuiButton instance with isLoading = false | undefined.

Do you have suggestions about how we can get this working only for buttons relying on this isLoading prop?

cchaos avatar May 14 '21 20:05 cchaos

I can't think of a way to not add a <span> to all buttons while also not introducing any new props or components...

myasonik avatar Aug 10 '21 19:08 myasonik

Related: https://github.com/elastic/eui/issues/5666

daveyholler avatar Jan 30 '23 18:01 daveyholler

👋 Hi there - this issue hasn't had any activity in 6 months. If the EUI team has not explicitly expressed that this is something on our roadmap, it's unlikely that we'll pick this issue up. We would sincerely appreciate a PR/community contribution if this is something that matters to you! If not, and there is no further activity on this issue for another 6 months (i.e. it's stale for over a year), the issue will be auto-closed.

github-actions[bot] avatar Sep 22 '25 00:09 github-actions[bot]