Update `EuiScreenReaderLive` and use it in EUI Docs to announce page loads to screen readers by 1Copenut · Pull Request #5995 · elastic/eui

1Copenut · 2022-06-23T20:18:13Z

Summary

Updated the EuiScreenReaderLive component to to accept an isFocusable boolean prop. The change lets users set focus on the outer containing DIV, so we can use it as a consistent announcement for SPA route changes.

This is a second PR that explores a different implementation of the same EUI Docs change as #5991. The second PR was prompted by @constancecchen comment here: #5991 (comment)

Checklist

* Updating screen_reader_live to accept focus. * Added two tests for isFocusable behavior. * Adding documentation for isFocusable prop.

1Copenut

Left one comment about adding same page link EuiScreenReaderOnly text

src-docs/src/components/guide_page/guide_page_chrome.js

kibanamachine · 2022-06-23T20:47:28Z

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

src/components/accessibility/screen_reader_live/screen_reader_live.tsx

src/components/accessibility/skip_link/skip_link.tsx

1Copenut · 2022-06-24T16:35:33Z

@constancecchen && @thompsongl I have a philosophical question RE this PR and the alternate take I proposed in #5991.

This PR (5995) uses Greg's EuiScreenReaderLive component and keys on URL strings to update the children prop to toggle local state. In my opinion this is the ideal for a design system component, to keep that toggle in the component's local state. What it means for our docs site (and I'm guessing Kibana), is that the toggle will only fire once on route change, and will always mean the first child div[role="status"] will be populated with the desired message.

PR 5995 only fires setToggle() once on route change, but fires it every time we click an in-page route. It doesn't announce the same page title a second time because focusRegionOnTextChange is not updated; we're not setting focus to the status region at the top of the source order.

I think this is okay, but wanted to get your thoughts. The alternate PR 5991 doesn't have this side effect because it uses a single status div and doesn't "juggle" two status live regions. 5991 depends on focus being set on a single div[role="status"] with text inside it to announce to screen readers. It's a subtle difference, but one I noticed while updating my SPA-routing-experience-body component to match this ^^ PR.

kibanamachine · 2022-06-24T16:52:19Z

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

src/components/accessibility/screen_reader_live/screen_reader_live.tsx

kibanamachine · 2022-06-24T19:29:01Z

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

- actually revert guide_page_chrome to original code - fix scroll_to_hash comments, syntax

- DRY out ButtonColor typing - remove unnecessary `= 'primary'` fallback - EuiButton already has this by default, so passing undefined is fine

src-docs/src/views/accessibility/accessibility_example.js

cee-chen · 2022-06-24T20:20:06Z

src/components/accessibility/screen_reader_live/screen_reader_live.test.tsx

+
+      const focusableDiv = component.find('div').at(0);
+
+      expect(focusableDiv.is(':focus')).toBe(true);


Awesome unit tests!

- the component already extends `EuiButtonProps` and already accepts `color` with all the necessary typing, which I only just realized 🤦

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

src/components/accessibility/screen_reader_live/screen_reader_live.tsx

kibanamachine · 2022-06-24T20:53:06Z

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

…live.tsx Co-authored-by: Constance <constancecchen@users.noreply.github.com>

…com/1Copenut/eui into feature/tpierce-live-region-docs-ii

cee-chen

🎉 This feels really robust and well tested - thanks for taking the extra time and effort on this Trevor!

kibanamachine · 2022-06-24T21:53:55Z

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

thompsongl

LGTM! Also confirmed that SR results for searchable EuiSelectable are unchanged.

elizabetdev

Thanks, @1Copenut.

The color="ghost" in EuiSkipLink looks good to me! 🎉

1Copenut added 2 commits June 23, 2022 14:50

Adding focus control to EuiScreenReaderLive

de9e5eb

* Updating screen_reader_live to accept focus. * Added two tests for isFocusable behavior. * Adding documentation for isFocusable prop.

Updating skip link tests.

458ddfa

1Copenut added documentation Issues or PRs that only affect documentation - will not need changelog entries accessibility accessibility - screen reader labels Jun 23, 2022

1Copenut requested review from cchaos, cee-chen, elizabetdev and thompsongl June 23, 2022 20:18

1Copenut self-assigned this Jun 23, 2022

Adding CHANGELOG entry.

cc7f2e9

1Copenut marked this pull request as ready for review June 23, 2022 20:20

1Copenut commented Jun 23, 2022

View reviewed changes

src-docs/src/components/guide_page/guide_page_chrome.js Outdated Show resolved Hide resolved

cee-chen reviewed Jun 23, 2022

View reviewed changes

src/components/accessibility/screen_reader_live/screen_reader_live.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Jun 23, 2022

View reviewed changes

src/components/accessibility/screen_reader_live/screen_reader_live.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Jun 23, 2022

View reviewed changes

src/components/accessibility/skip_link/skip_link.tsx Outdated Show resolved Hide resolved

Splitting useEffects, making skip link color dynamic.

29f3e4f

1Copenut requested a review from cee-chen June 24, 2022 16:26

cee-chen reviewed Jun 24, 2022

View reviewed changes

src/components/accessibility/screen_reader_live/screen_reader_live.tsx Outdated Show resolved Hide resolved

cee-chen added 2 commits June 24, 2022 11:49

Fix toggle announcing too much on sub section link clicks

e30d0ab

Improve subsection links screen reader experience

65dc9e1

Updating one test after in-page link improvement.

7288e1e

1Copenut mentioned this pull request Jun 24, 2022

Create a status component and use it in EUI Docs to announce page loads to screen readers #5991

Closed

11 tasks

Fixing main merge conflict.

61ba738

1Copenut requested a review from cee-chen June 24, 2022 20:01

Renaming a prop in unit tests.

1b2dfb9

cee-chen added 2 commits June 24, 2022 13:12

misc tweaks/fixes to previous constance commits

0df382f

- actually revert guide_page_chrome to original code - fix scroll_to_hash comments, syntax

[PR feedback] EuiSkipLink color prop

f9b8e56

- DRY out ButtonColor typing - remove unnecessary `= 'primary'` fallback - EuiButton already has this by default, so passing undefined is fine

cee-chen reviewed Jun 24, 2022

View reviewed changes

src-docs/src/views/accessibility/accessibility_example.js Outdated Show resolved Hide resolved

cee-chen reviewed Jun 24, 2022

View reviewed changes

cee-chen and others added 2 commits June 24, 2022 13:22

[PR feedback] Revert EuiSkipLink changes

8306354

- the component already extends `EuiButtonProps` and already accepts `color` with all the necessary typing, which I only just realized 🤦

Update src-docs/src/views/accessibility/accessibility_example.js

52cc406

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

cee-chen reviewed Jun 24, 2022

View reviewed changes

src/components/accessibility/screen_reader_live/screen_reader_live.tsx Outdated Show resolved Hide resolved

1Copenut and others added 5 commits June 24, 2022 16:00

Update src/components/accessibility/screen_reader_live/screen_reader_…

9a12d7b

…live.tsx Co-authored-by: Constance <constancecchen@users.noreply.github.com>

Add documentation section for focusRegionOnTextChange + example

1cd22d5

Merge branch 'feature/tpierce-live-region-docs-ii' of https://github.…

87cc233

…com/1Copenut/eui into feature/tpierce-live-region-docs-ii

Messed up a snippet

c4298ed

Fix documentation example on small screens

9215ef3

cee-chen approved these changes Jun 24, 2022

View reviewed changes

thompsongl approved these changes Jun 27, 2022

View reviewed changes

elizabetdev approved these changes Jun 28, 2022

View reviewed changes

1Copenut merged commit 6e72317 into elastic:main Jun 28, 2022

1Copenut deleted the feature/tpierce-live-region-docs-ii branch June 28, 2022 14:44

1Copenut mentioned this pull request Jun 28, 2022

Bumping EUI to v60.1.2. elastic/kibana#135373

Merged

1Copenut mentioned this pull request Jul 5, 2022

(Accessibility) Implement client routing solution for all products elastic/kibana#135760

Closed

2 tasks


		const focusableDiv = component.find('div').at(0);

		expect(focusableDiv.is(':focus')).toBe(true);

Conversation

1Copenut commented Jun 23, 2022 • edited by cee-chen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

1Copenut left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibanamachine commented Jun 23, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

1Copenut commented Jun 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibanamachine commented Jun 24, 2022

Uh oh!

Uh oh!

kibanamachine commented Jun 24, 2022

Uh oh!

Uh oh!

cee-chen Jun 24, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibanamachine commented Jun 24, 2022

Uh oh!

cee-chen left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jun 24, 2022

Uh oh!

thompsongl left a comment

Choose a reason for hiding this comment

Uh oh!

elizabetdev left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

1Copenut commented Jun 23, 2022 •

edited by cee-chen

Loading

1Copenut commented Jun 24, 2022 •

edited

Loading