-
Notifications
You must be signed in to change notification settings - Fork 837
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
[EuiLoading] Accessibility props #4814
Comments
Thanks for bringing this up – great catch! Do you find much use from the |
Whoops, good point! I never tested that it was needed I'm also not convinced |
The role is good but your mention of it made me take a second look at what you have we actually don't want To give an example, you might do something like this: <p>The paragraphs below are loaded async</p>
<EuiLoadingContent
lines={10}
role="progressbar"
aria-label="Loading content"
/>
<section aria-busy={isLoading}>
{isLoading && render()}
</section> It's important to note that |
@j-m I hope it's all right, I rewrote your issue description to focus on the things we should do. If you want to revive any of the historical context or if I missed anything, Github saves a history of edits and you can get your original version back where it says "edited by myasonik". Thanks! |
Of course! Makes it easier for everyone Hmm in that case would it make more sense to change how EuiLoadingContent works? <EuiLoadingContent
lines={10}
role="progressbar"
aria-label="Loading content"
isLoaded={true}
>
<EuiDataGrid .../>
</EuiLoadingContent> |
That's a really good idea and it doesn't look like that's a breaking change either which is always easier to roll out. Updating the issue description once more. |
- Added tintOrShade and shadeOrTint functions - Converted margins to use either `gap` or logical property - Using Emotion `keyframes` - Cleanup classes and for loop for bars - Improved accessibility #4814
Fixed by #6562 |
Summary
EuiLoading needs to better communicate it's state to assistive tech. We can improve the component in two ways:
role="progressbar"
and a defaultaria-label
value to the loading statearia-busy
Details
Doing the first, improves the existing usage of the component. Doing the second, is a non-breaking API change that should better handle a more ideal UX.
For existing implementations, or for those that can't/don't want to upgrade to the new pattern, we should provide better documentation around
aria-label
andaria-busy
Documentation on
aria-label
should largely ask that something better than "loading content" be passed in.Documentation on
aria-busy
should talk about where to put it (around the loading content) and give warnings on how it works (that it treats all children asaria-hidden
) so devs don't overuse it.In addition to accepting children to handle the
aria-busy
attribute setting, it'll need to also accept anisLoading
prop (which is required ifchildren
are passed in).The text was updated successfully, but these errors were encountered: