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

[EuiButton] Review the isLoading and isDisabled prop behavior for screen reader experience #5666

Open
1Copenut opened this issue Feb 28, 2022 · 9 comments

Comments

@1Copenut
Copy link
Contributor

1Copenut commented Feb 28, 2022

Description

@miukimiu brought up a great question in #5649 with her comment about the isLoading behavior:

To introduce a EuiButtonIcon isLoading state I can follow the other buttons isLoading state approach:

  • The button gets disabled.
  • The EuiIcon gets replaced with a EuiLoadingSpinner.
  • We introduce a new size to EuiLoadingSpinner "XXL" so that all the sizes match the EuiIcon sizes.

But I have some a11y questions.

  • When the button gets disabled the focus state is missed.
  • It gets removed from the accessibility tree and is invisible to assistive technology users

So when the EuiButtonIcon isLoading would adding an aria-disabled instead of disabled be more appropriate? Or is it OK to keep following the other buttons' pattern?

The challenge

Our buttons all use the HTML5 disabled attribute to remove buttons' default behavior. Visual users can see the dimmed button and make an assertion they need to take some action before the button will become enabled.

Screen reader users do not have this same experience. The disabled attribute removes the button's tabindex (so no keyboard navigation) and removes it from interactive element menus for screen readers. For all intents and purposes, the button is invisible to assistive technology.

Some relevant research

WCAG guidance

@1Copenut 1Copenut added this to Needs triage/design in Accessibility via automation Feb 28, 2022
@1Copenut 1Copenut added the accessibility - WCAG A Level A WCAG Accessibility Criteria label Feb 28, 2022
@chandlerprall
Copy link
Contributor

👍 If we decide to implement aria-disabled instead of disabled in some cases, we'll need to prevent onClick and possibly other events from firing.

@miukimiu
Copy link
Contributor

miukimiu commented Mar 3, 2022

The following article is also interesting to add to @1Copenut's relevant research list: Making Disabled Buttons More Inclusive.

@miukimiu miukimiu changed the title [EuiButton] Review the isLoading prop behavior for screen reader experience [EuiButton] Review the isLoading and isDisabled prop behavior for screen reader experience Apr 8, 2022
@github-actions
Copy link

github-actions bot commented Oct 5, 2022

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@1Copenut
Copy link
Contributor Author

Another discussion of this point, on Material UI: mui/material-ui#14455

@cee-chen
Copy link
Member

#4797 Should be handled at the same time

@github-actions
Copy link

👋 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.

This comment was marked as duplicate.

@JasonStoltz
Copy link
Member

We re-prioritized this in our queue as a High priority.

As it stands today, there are many instances throughout Kibana of disabled EuiButtons that have tooltips attached, and it is a pattern that is still unfortunately being propogated.

Disabled buttons are inaccessible to screenreaders, and the tooltip information in these disabled buttons give valuable context as to why the button is disabled. This means that screen readers have no idea that the button exists, is disabled, and contains additional context. This can be a dead end for screen reader users.

We would like to prioritize changing our buttons to leverage aria-disabled instead.

A challenge we'll have here is that we'll have to programatically mimic a disabled button rather than relying on the native disabled behavior, which could have downstream impacts to current usage. We should consider rolling this out gradually as a new beta variant of the button, rather than changing all buttons at once.

Additionally, we'll need to ensure this works for button groups as well.

@JasonStoltz
Copy link
Member

Beyond EuiButton, @JoseLuisGJ pointed out that we may have some other components that need to be reviewed. There is at least one case of an EuiCard in a disabled state using a tooltip.

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Accessibility
  
Needs triage/design
Development

No branches or pull requests

5 participants