Accessible screen reader live region and notifications #359

cee-chen · 2019-07-19T23:13:28Z

Description

This is an an accessibility PR that provides more context and information to screen reader users. The goal is to announce the results/consequences of certain actions being taken to users, so that visually impaired users aren't left wondering what happened after pressing a button.

For example:

After clicking the "+ More" button in the filters sidebar, a screen reader user should know how many total filters are now being shown.
After pressing the search button, a screen reader user should know how many results are being shown and how many total results were returned.
After navigating to another page of results, a screen reader user should have the results/paging info read out to them (same as above)
After changing how many results are being shown per-page, a screen reader user should have the results/paging info read out to them (same as above)

Screencaps

Search results announcements:

+ More Filters annoucements:

List of changes

Adds a new a11yNotifications flag to our SearchDriver config.
- This will add a visually hidden live region to the end of document which will inform screen readers of app-wide changes in response to actions that users have taken.
- Placing this in our vanilla JS SearchDriver allows all users, not just developers using our React UI, to enjoy a more accessible experience.
- It's currently defaulted to false for a non-breaking 1.1 release, but will be flipped over to a true default for our upcoming 2.0 release.
Adds a new a11yNotificationMessages key to our SearchDriver config
- This allows developers to override our accessibility phrasing/messages, e.g. for localization.
- This also allows developers to add their own completely custom screen reader notifications if needed.
Adds a new a11yNotify action to SearchDriver / withSearch
- Per above, this allows developers to add their own custom screen reader notifications in their custom views/components.

Associated Github Issues

Fixes #325

PRs: See #320 and #355 for initial WIP work towards this final architecture.

- a11yNotifications flag/bool - a11yNotify action - this.a11yNotificationMessages

+ ability to pass in all custom SearchDriver config options to setupDriver test helper

+ DRY out PagingInfoContainer logic

so as to be non-breaking for any projects already shipping their own accessibility notices this will default to true in 2.0

ADVANCED.md

packages/react-search-ui/src/containers/__tests__/Facet.test.js

ADVANCED.md

examples/sandbox/src/App.js

packages/react-search-ui/src/containers/PagingInfo.js

packages/react-search-ui/src/containers/__tests__/Facet.test.js

packages/search-ui/src/SearchDriver.js

packages/search-ui/src/__tests__/A11yNotifications.ssr.test.js

packages/search-ui/src/__tests__/actions/a11yNotify.test.js

packages/search-ui/src/SearchDriver.js

ADVANCED.md

packages/react-search-ui/src/containers/Facet.js

packages/search-ui/src/test/helpers.js

packages/search-ui/src/A11yNotifications.js

packages/search-ui/src/actions/a11yNotify.js

packages/react-search-ui/src/containers/__tests__/Facet.test.js

#359 (comment)

…-ui into a11y-notifications

- Note that console.warn and console.error still seem to be output by jest no matter what :(

- Moving the message func to react-search-ui, so that non-react users don't inherit a UI message function they aren't using

cee-chen · 2019-07-23T20:41:47Z

Alrighty - all of @yakhinvadim's feedback should have been addressed at this point! I'd love a quick re-review of any added/changed functionality (don't hesitate to flag anything that you're skeptical of that was added after feedback, I truly don't mind).

As a heads up also once this PR has been finalized/merged, I'll probably go back and un-collapse a few comments that contain useful info that I think will be handy to have as historical reference :)

yakhinvadim

This is great! I have only one comment. Also, waiting for Jason's thoughts in this discussion: #359 (comment)

packages/react-search-ui/src/SearchProvider.js

yakhinvadim

LGTM 👍 Really happy we'll see this in the next release.

cee-chen added 7 commits July 17, 2019 15:16

🏗️ Add initial a11y notifications helper/manager

598441b

✨ Set up SearchDriver with a11y notification state

9ba4b60

- a11yNotifications flag/bool - a11yNotify action - this.a11yNotificationMessages

✅ Add tests for new SearchDriver logic

df72b65

+ ability to pass in all custom SearchDriver config options to setupDriver test helper

♻️ Update Facets +More action to use a11yNotify

dff5b7b

♻️ Refactor _updateSearchResults to use a11yNotify

29d4b83

+ DRY out PagingInfoContainer logic

📝 Add documentation

b880a95

📦 1.1 release - turn off a11yNotifications by default

da835da

so as to be non-breaking for any projects already shipping their own accessibility notices this will default to true in 2.0

cee-chen commented Jul 19, 2019

View reviewed changes

ADVANCED.md Show resolved Hide resolved

ADVANCED.md Outdated Show resolved Hide resolved

packages/react-search-ui/src/containers/__tests__/Facet.test.js Show resolved Hide resolved

JasonStoltz reviewed Jul 22, 2019

View reviewed changes

cee-chen and others added 2 commits July 22, 2019 11:15

👌 Address PR feedback

90ff38c

Merge branch 'master' into a11y-notifications

d66a57e

JasonStoltz previously approved these changes Jul 22, 2019

View reviewed changes

yakhinvadim reviewed Jul 22, 2019

View reviewed changes

cee-chen added 4 commits July 22, 2019 12:58

👌 Address PR feedback re: optionsCount naming

95927e6

#359 (comment)

👌 Address PR feedback re: SearchDriver test helpers

50fbb96

#359 (comment)

👌 Address documentation feedback

d37ac77

Merge branch 'a11y-notifications' of github.com:constancecchen/search…

20df0c7

…-ui into a11y-notifications

cee-chen dismissed JasonStoltz’s stale review via 20df0c7 July 22, 2019 20:02

cee-chen and others added 4 commits July 23, 2019 10:53

👌 Address PR feedback re: console error

bbb25a6

- Note that console.warn and console.error still seem to be output by jest no matter what :(

👌 Address boolean var naming feedback

b9df4f8

Merge branch 'master' into a11y-notifications

8a3d913

👌 Address PR feedback re: moveFilters location

70d8379

- Moving the message func to react-search-ui, so that non-react users don't inherit a UI message function they aren't using

cee-chen requested review from JasonStoltz and yakhinvadim July 23, 2019 20:38

yakhinvadim reviewed Jul 24, 2019

View reviewed changes

packages/react-search-ui/src/SearchProvider.js Outdated Show resolved Hide resolved

👌 Address PR feedback re: obj mutation

6dced84

yakhinvadim approved these changes Jul 24, 2019

View reviewed changes

JasonStoltz mentioned this pull request Jul 24, 2019

Make a11y notifications "true" by default #365

Closed

Merge branch 'master' into a11y-notifications

6968597

JasonStoltz merged commit 0cae2f5 into elastic:master Jul 24, 2019

cee-chen deleted the a11y-notifications branch July 24, 2019 19:38

JasonStoltz mentioned this pull request Oct 31, 2019

Make "hasA11yNotifications" true by default #394

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accessible screen reader live region and notifications #359

Accessible screen reader live region and notifications #359

cee-chen commented Jul 19, 2019 •

edited

Loading

cee-chen commented Jul 23, 2019

yakhinvadim left a comment •

edited

Loading

yakhinvadim left a comment

Accessible screen reader live region and notifications #359

Accessible screen reader live region and notifications #359

Conversation

cee-chen commented Jul 19, 2019 • edited Loading

Description

Screencaps

List of changes

Associated Github Issues

cee-chen commented Jul 23, 2019

yakhinvadim left a comment • edited Loading

Choose a reason for hiding this comment

yakhinvadim left a comment

Choose a reason for hiding this comment

cee-chen commented Jul 19, 2019 •

edited

Loading

yakhinvadim left a comment •

edited

Loading