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

Improves screen reader features of combo boxes in edit role screen #153808

Merged
merged 7 commits into from
Mar 30, 2023

Conversation

jeramysoucy
Copy link
Contributor

@jeramysoucy jeramysoucy commented Mar 27, 2023

Summary

Related to #27749

While the EuiComboBox rebuild is in progress, this PR addresses missing aria properties and fixes option announcements for the run as user combo. It additionally adds an aria-label for the Spaces Navigation EuiSelectable that I found was missing.

Testing

  • Add all sample data
  • Edit a non-reserved role
  • Turn on a screen reader (NVDA is recommended, but VoiceOver will do)
  • Tab through the controls on the Edit role screen. When you arrive in a combo box you may or may not hear an announcement declaring the name of the combo box (this is expected, especially with VoiceOver).
  • With focus in a combo box press the down arrow key. Verify the options are announced as you traverse them with the down and up arrow keys.
  • If there was success in option announcement, press escape key and verify that the name of the combo box is announced. You may have to press escape twice when using NVDA.

Note: It seems to be the consensus that this is best we can do with the existing implementation of EuiComboBox, which is partly why it is being rebuilt with the EuiSelectable component.

@jeramysoucy jeramysoucy added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! release_note:skip Skip the PR/issue when compiling release notes accessibility: keyboard navigation Accessibility keyboard navigation and focus v8.8.0 and removed accessibility: keyboard navigation Accessibility keyboard navigation and focus labels Mar 27, 2023
@jeramysoucy jeramysoucy changed the title Fixes screen reader features of combo boxes in edit role screen Improves screen reader features of combo boxes in edit role screen Mar 28, 2023
@jeramysoucy jeramysoucy marked this pull request as ready for review March 29, 2023 12:12
@jeramysoucy jeramysoucy requested a review from a team as a code owner March 29, 2023 12:12
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@kc13greiner kc13greiner self-requested a review March 29, 2023 14:30
Copy link
Contributor

@kc13greiner kc13greiner left a comment

Choose a reason for hiding this comment

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

Just a couple questions to help me understand!

Copy link
Contributor

@kc13greiner kc13greiner left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the info!

@jeramysoucy jeramysoucy enabled auto-merge (squash) March 30, 2023 14:50
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #13 / dashboard dashboard maps by value editing a map and adding to map library updates the existing panel when adding to dashboard
  • [job] [logs] FTR Configs #13 / dashboard dashboard maps by value editing a map by value "before all" hook for "retains the same number of panels"

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
security 557.1KB 557.3KB +189.0B
spaces 174.6KB 174.7KB +97.0B
total +286.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 433 436 +3

Total ESLint disabled count

id before after diff
securitySolution 513 516 +3

History

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

@jeramysoucy jeramysoucy merged commit b3d78e8 into elastic:main Mar 30, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Mar 30, 2023
@jeramysoucy jeramysoucy self-assigned this Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants