[EuiSkeletonLoading] Improve loading accessibility and bake in `isLoading` API handling by cee-chen · Pull Request #6562 · elastic/eui

cee-chen · 2023-01-31T02:43:33Z

Summary

This adds the loading accessibility detailed in #4814. I recommend following along by commit if possible.

All EuiSkeleton components now accept isLoading and children props, which automatically handles conditionally rendering the skeleton components vs. the loaded content (children) for consumers.
A Loaded {contentAriaLabel} announcement is now made whenever isLoading flips to false.

QA

General checklist

Checked in Chrome, Safari, Edge, and Firefox
Props have proper autodocs (using @default if default values are missing) and playground toggles
Added documentation
Added or updated jest and cypress tests
Checked for accessibility including keyboard-only and screenreader modes
A changelog entry exists and is marked appropriately

~~- [ ] Checked in both light and dark modes~~
- [ ] Checked in mobile
~~- [ ] Checked Code Sandbox works for any docs examples~~
~~- [ ] Checked for breaking changes and labeled appropriately~~
~~- [ ] Updated the Figma library counterpart~~

- for DRYing out correct accessibility attributes, wrappers, and a simpler loading API

+ update docs/example to how `isLoading`/`children` behavior

…apper + update docs/example to how `isLoading`/`children` behavior

- the new `aria-busy` wrapper appears to make the `z-index` workaround not work, so use `isolation` (https://developer.mozilla.org/en-US/docs/Web/CSS/isolation) instead to force the necessary stacking context

kibanamachine · 2023-01-31T03:14:03Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6562/

1Copenut

This is such a boon improvement @Cee. I've been testing this morning with various browsers and VoiceOver. Have one small change in the src-docs example for Skeleton Loading to make axe happy.

For whatever reason, Safari + VO isn't announcing the content loaded when it's just text, title, or icons. If we have mixed content or are wrapping multiples with the single EuiSkeletonLoading it works perfectly. Firefox + VO works perfectly all around.

I'm bringing it to our team meeting, but my hunch is to launch, verify with other screen readers, and course correct if necessary.

1Copenut · 2023-01-31T15:01:30Z

src/components/skeleton/skeleton_loading.tsx

+      {...rest}
+    >
+      {isLoading ? (
+        React.cloneElement(loadingContent, loadingProps)


This pattern is just the ticket. Controlling the aria-busy with the parent, then adding the relevant ARIA props to a consumer-passed child component.

I had a thought this could get us into a situation where axe-core throws about a nested element role, but MDN bailed me out with this gem on the progressbar entry:

All descendants are presentational

There are some types of user interface components that, when represented in a platform accessibility API, can only contain text. Accessibility APIs do not have a way of representing semantic elements contained in a progressbar. To deal with this limitation, browsers, automatically apply role presentation to all descendant elements of any progressbar element as it is a role that does not support semantic children.

This nullifies the implicit roles of child elements, making elements and content inert.

Credit where credit is due, I got the aria-busy should be on the parent thing from Michael Yasonik here: #4814 (comment)

src-docs/src/views/skeleton/skeleton_loading.tsx

(or in this case, `EuiPanel` for looks

1Copenut

💯 LGTM! Steady a11y improvement for the win.

kibanamachine · 2023-01-31T18:41:54Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6562/

- Missed in #6562 - Playground still isn't perfect; but good enough for now

* EuiSkeleton components (#6502) * Setup EuiSkeleton files and folders structure * Moved Skeleton under the Display side menu category * Component variants setup * feat: skeleton setup [text, rect, circle] * feat: added skeleton item draft and other minors * chore: renamed sizes * feat: added Heading in favor of Rect component * Update src-docs/src/views/skeleton/skeleton_example.js Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co> * Update src-docs/src/views/skeleton/skeleton_example.js Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co> * Update src-docs/src/views/skeleton/skeleton_example.js Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co> * chore: wrapped p with EuiText and indentation * chore: removed useless styles * chore: centralized and renamed skeleton animation keyfram * refactor: renamed euiSkeletonHeading to euiSkeletonTitle and updated sizes with euiTitle ones * chore: renamed EuiSkeletonItem to EuiSkeletonRectangle * Remove hard-coded `euiSkeletonTitleStyles` heights map - in favor of calling `euiTitle`'s font utils and grabbing `lineHeight` directly from it * [docs] Add EuiSwitch toggle to EuiSkeletonTitle tests - this toggle help demonstrates to consumers how to conditionally switch from loading to non-loading components, and also allows us to test heights * Add `size` prop to `EuiSkeletonText` + add calculations & offsets to account for text scale + update docs to allow changing size & toggle between text to test visual behavior * [docs] Add loading toggle for EuiSkeletonCircle + prettier fixes * [PR feedback] Fix incorrect BEM naming; remove extra modifier classes - BEM: `__` should only be used for children/descendants, and these are parent-level styles and should use regular camelCasing - modifier classes removal: we're moving away from setting these extra CSS classes, which do not have any actual CSS tied to them (since we're using Emotion CSS-in-JS instead). The top-level className remains as an API for consumers to hook into for overrides if needed. * [PR feedback] DRY out all `aria`/a11y attributes to reusable helper - not all components were correctly receiving all the aria attrs they should have been - this helper fixes that * [PR feedback] Rename` _skeleton` to `utils`, DRY out repeated gradient CSS - `utils` better matches naming/architecture convention in other components (+ ignore i18n complaint) + tweak gradient size and animation for circle and rectangle components to look a bit better + rename global animation to a more generic name/usage * [PR feedback] DRY out repeated CSS between modifiers - should exist in the base styles instead + bonus - DRY out `logicalSizeCSS` to only require 1 input if both sides are the same * [PR feedback] Avoid dynamic wildcard Emotion classes - use `style` instead The reason for this is performance - Emotion generates a completely new hash/className for every single possible permutation of width/height passed to it. We want to avoid this for wildcard prop-based styles and use inline styles instead * [PR feedback] Improve/add unit tests + cover all props + prefer RTL over Enzyme (part of ongoing migration) + fix type issue found during testing * [docs] Add toggle to EuiSkeletonRectangle example * [PR feedback] EuiSkeletonRectangle tweaks - allow numbers as well as strings - now that we're using inline `style`s for the dimensions, React accepts numbers as well (which matches EuiImage API) - remove image URI in favor of a picsum link - set width/height on img to prevent page jumping * [PR feedback] Tweak EuiSkeletonText typing - for whatever reason, using `|` prints the line numbers in non-ascending order in the props table - using an array and `as const` fixes this, and is reusable in tests * [PR feedback] Convert docs files to Typescript + improve mobile display of demos * [PR feedback] Documentation copy + order - move EuiSkeletonCircle slightly lower in docs order - EuiSkeletonText is likely going to be more popular since it already exists - misc copy tweaks * [Docs] Set up playgrounds * DRY out aria-label further - since `euiLoadingStrings.ariaLabel` already exists, just reuse that instead of making our translators re-translate the same string * Add fix for Safari workaround - note: this is already broken on prod as well for `EuiLoadingContent` * Changelog * chore: comment typo and zero value for border radius --------- Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co> Co-authored-by: Constance Chen <constance.chen.3@gmail.com> * Design review (#6560) * Deprecate `EuiLoadingContent` in favor of `EuiSkeletonText` (#6557) * Deprecate EuiLoadingContent + dogfood EuiSkeletonText * Remove unnecessary tests - leave a basic `is rendered` test in just to confirm the content, but should be deleted in the future * Update documentation with deprecation notice * changelog * Fix Emotion styles var name * [EuiSkeletonLoading] Improve loading accessibility and bake in `isLoading` API handling (#6562) * Create reusable `EuiSkeletonLoading` component - for DRYing out correct accessibility attributes, wrappers, and a simpler loading API * Convert `EuiSkeletonText` to dogfood new `EuiSkeletonLoading` wrapper + update docs/example to how `isLoading`/`children` behavior * Convert `EuiSkeletonTitle` to dogfood new `EuiSkeletonLoading` wrapper + update docs/example to how `isLoading`/`children` behavior * Convert `EuiSkeletonRectangle` to dogfood new `EuiSkeletonLoading` wrapper + update docs/example to how `isLoading`/`children` behavior * Convert `EuiSkeletonRectangle` to dogfood new `EuiSkeletonLoading` wrapper + update docs/example to how `isLoading`/`children` behavior * Add docs example + a11y callout for `EuiSkeletonLoading` * Fix annoying Safari bug popping up again - the new `aria-busy` wrapper appears to make the `z-index` workaround not work, so use `isolation` (https://developer.mozilla.org/en-US/docs/Web/CSS/isolation) instead to force the necessary stacking context * changelog * [PR feedback] use `div` instead of `section` (or in this case, `EuiPanel` for looks * [PR feedback] Add `children` to playground - Missed in #6562 - Playground still isn't perfect; but good enough for now * Tweak default `EuiSkeletonCircle` size to match default `EuiAvatar` size --------- Co-authored-by: Andrea Della Valle <andreareva@gmail.com> Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>

## Summary `eui@74.0.1` ⏩ `eui@74.1.0` ___ ## [`74.1.0`](https://github.com/elastic/eui/releases/tag/v74.1.0) - Added new `EuiSkeletonText`, `EuiSkeletonTitle`, `EuiSkeletonCircle`, and `EuiSkeletonRectangle` components ([#6502](elastic/eui#6502)) - Updated `EuiSuperSelect` screen reader instructions to be more specific ([#6549](elastic/eui#6549)) - Added `error` and updated `alert` glyphs to `EuiIcon` ([#6550](elastic/eui#6550)) - All `EuiSkeleton` components now accept an `isLoading` flag and `children`, which automatically handles conditionally rendering loading skeletons vs. loaded content (`children`) ([#6562](elastic/eui#6562)) - All `EuiSkeleton` components now accept a `contentAriaLabel` prop, which more meaningfully describes the loaded content to screen readers ([#6562](elastic/eui#6562)) - Updated `EuiPopover` screen reader instructions for mobile and click behaviors ([#6567](elastic/eui#6567)) **Bug fixes** - Fixed `EuiCard` to ensure `onClick` method only runs once when `title` contains a React node ([#6551](elastic/eui#6551)) **Deprecations** - Deprecated `EuiLoadingContent` - use `EuiSkeletonText` instead ([#6557](elastic/eui#6557)) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

cee-chen added 7 commits January 30, 2023 18:31

Create reusable EuiSkeletonLoading component

daf1f7f

- for DRYing out correct accessibility attributes, wrappers, and a simpler loading API

Convert EuiSkeletonText to dogfood new EuiSkeletonLoading wrapper

71aeecc

+ update docs/example to how `isLoading`/`children` behavior

Convert EuiSkeletonTitle to dogfood new EuiSkeletonLoading wrapper

3285d1b

+ update docs/example to how `isLoading`/`children` behavior

Convert EuiSkeletonRectangle to dogfood new EuiSkeletonLoading wr…

44a629a

…apper + update docs/example to how `isLoading`/`children` behavior

Convert EuiSkeletonRectangle to dogfood new EuiSkeletonLoading wr…

5bbdfc8

…apper + update docs/example to how `isLoading`/`children` behavior

Add docs example + a11y callout for EuiSkeletonLoading

3b0adae

Fix annoying Safari bug popping up again

b39d789

- the new `aria-busy` wrapper appears to make the `z-index` workaround not work, so use `isolation` (https://developer.mozilla.org/en-US/docs/Web/CSS/isolation) instead to force the necessary stacking context

cee-chen requested a review from 1Copenut January 31, 2023 02:43

changelog

e444feb

This was referenced Jan 31, 2023

[EuiLoadingChart][SCREENREADER]: Consider updating the loading indicators' role to "status" and adding aria-busy attribute #5843

Closed

Add new EuiSkeleton components; deprecate EuiLoadingContent #6558

Merged

1Copenut reviewed Jan 31, 2023

View reviewed changes

[PR feedback] use div instead of section

03460d5

(or in this case, `EuiPanel` for looks

1Copenut approved these changes Jan 31, 2023

View reviewed changes

cee-chen merged commit 364238c into elastic:feature/skeletons Jan 31, 2023

cee-chen deleted the skeletons-aria-attrs branch January 31, 2023 19:21

cee-chen added a commit that referenced this pull request Jan 31, 2023

[PR feedback] Add children to playground

9b2e7f9

- Missed in #6562 - Playground still isn't perfect; but good enough for now

This was referenced Feb 2, 2023

Upgrade EUI to v74.1.0 elastic/kibana#150235

Merged

[EuiLoading] Accessibility props #4814

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiSkeletonLoading] Improve loading accessibility and bake in `isLoading` API handling#6562

[EuiSkeletonLoading] Improve loading accessibility and bake in `isLoading` API handling#6562
cee-chen merged 9 commits intoelastic:feature/skeletonsfrom
cee-chen:skeletons-aria-attrs

cee-chen commented Jan 31, 2023 •

edited

Loading

Uh oh!

kibanamachine commented Jan 31, 2023

Uh oh!

1Copenut left a comment

Uh oh!

1Copenut Jan 31, 2023

Uh oh!

cee-chen Jan 31, 2023

Uh oh!

Uh oh!

1Copenut left a comment

Uh oh!

kibanamachine commented Jan 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

cee-chen commented Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

QA

General checklist

Uh oh!

kibanamachine commented Jan 31, 2023

Uh oh!

1Copenut left a comment

Choose a reason for hiding this comment

Uh oh!

1Copenut Jan 31, 2023

Choose a reason for hiding this comment

All descendants are presentational

Uh oh!

cee-chen Jan 31, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

1Copenut left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jan 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cee-chen commented Jan 31, 2023 •

edited

Loading