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

[Discover] Add ownFocus to Discover field search popover for a11y #83399

Closed

Conversation

kertal
Copy link
Member

@kertal kertal commented Nov 16, 2020

Summary

This PR improves Discover field search popover to be accessible by adding ownFocus to the EuiPopover. What doesn't work is adding focus to the first input element, seems to be an issue of EuiButtonGroup

So when you set the ownFocus to true but no initialFocus is the type Select is automatically focused:
Bildschirmfoto 2020-11-16 um 10 45 44

When you set the ownFocus and try to set initialFocus to the first button group, no element is focused, but after using tab key the first EuiButtonGroup is focused:

Bildschirmfoto 2020-11-16 um 10 45 04

Fixes #75190

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@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
discover 420.6KB 420.7KB +93.0B

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

@myasonik
Copy link
Contributor

Being unable to successfully set initialFocus to EuiButtonGroup is definitely a bug (I saw you created elastic/eui#4266 which is perfect).

But leaving it off is also totally fine. Focus is moved to the popover container itself without initialFocus set which is a generally expected default. So you only need to set an initialFocus if you really want to.

So, if you want, you could leave it at that, merge the PR, and close the issue.

If you want initialFocus to be set to the EuiButtonGroup then you'll need to wait for EUI to fix the issue.

@myasonik
Copy link
Contributor

Oh! Actually, you could close this PR all together as EUI recently merged a PR changing the default of ownFocus to true for all EuiPopovers which would close the issue. (Though this would still not setup an initialFocus.)

@kertal
Copy link
Member Author

kertal commented Nov 16, 2020

thx @myasonik, will close this PR since it doesn't make sense to add code, to later remove it.

@kertal kertal closed this Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(Accessibility) Dicsover Keyboard Accessibility "Filter by type" component
3 participants