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

[Enterprise Search] Improve flash messages screen reader UX #103412

Merged
merged 3 commits into from
Jun 29, 2021

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Jun 26, 2021

Summary

I hadn't realized this earlier (totally my bad), but apparently role="region" was causing "Flash messages" to get read out loud (and in duplicate, which is fun) between every page navigation... even when it was empty and had no messages. 🤦‍♀️

flash messages

I tested in MacOS VO in Firefox with just aria-live and that seems to be enough for screen readers to pick up the messages without also needing role, and stops "Flash message" from getting read out on every page. That being said, I could be wrong on that or missing other screen readers - @myasonik, do you mind jumping in on if this is okay to do? I didn't see any axe-devtools warnings from this change FWIW.

Checklist

  • Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)

@cee-chen cee-chen added release_note:skip Skip the PR/issue when compiling release notes v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 26, 2021
@cee-chen cee-chen requested review from myasonik and a team June 26, 2021 03:28
Copy link
Contributor

@byronhulcher byronhulcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed your repro and it works as you describe. I trust you to work out with @myasonik if this is the right way to go, but I don't want you blocked by ent-search-frontend approval

@cee-chen
Copy link
Contributor Author

@elasticmachine merge upstream

@cee-chen
Copy link
Contributor Author

cee-chen commented Jun 28, 2021

Thanks Byron! I'll hang tight on merging until I get confirmation from Michail on Monday 👍

cee-chen added 2 commits June 28, 2021 14:05
- just `aria-live` is enough for screen readers to read it out, and `role` was causing "Flash messages" to get read out loud repeatedly between page navigation even when empty which was annoying and not good
@cee-chen
Copy link
Contributor Author

Just chatted with Michail on Slack and he additionally recommended role="alert". I re-tested on VO and the live area is still reporting errors well without announcing anything extra on page navigation 💯 No new axe-devtools errors, so I think this is a winner!

@cee-chen cee-chen enabled auto-merge (squash) June 28, 2021 21:07
@cee-chen
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 2.1MB 2.1MB -412.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@cee-chen cee-chen merged commit 6336494 into elastic:master Jun 29, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 29, 2021
…103412)

* Remove role region on flash messages

- just `aria-live` is enough for screen readers to read it out, and `role` was causing "Flash messages" to get read out loud repeatedly between page navigation even when empty which was annoying and not good

* Further a11y attribute recommendations from @myasonik
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 29, 2021
…#103596)

* Remove role region on flash messages

- just `aria-live` is enough for screen readers to read it out, and `role` was causing "Flash messages" to get read out loud repeatedly between page navigation even when empty which was annoying and not good

* Further a11y attribute recommendations from @myasonik

Co-authored-by: Constance <constancecchen@users.noreply.github.com>
@cee-chen cee-chen deleted the misc-pass-2 branch July 7, 2021 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants