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

Added role='dialog' to EuiFlyout and documented correct way to make it announce itself with aria attributes #916

Merged
merged 2 commits into from
Jun 11, 2018

Conversation

cjcenizal
Copy link
Contributor

Fixes #725

@cjcenizal
Copy link
Contributor Author

jenkins, test this

@cchaos
Copy link
Contributor

cchaos commented Jun 11, 2018

It now reads the description twice:

https://d.pr/free/v/3smVQ5

@cjcenizal
Copy link
Contributor Author

Thanks @cchaos. Looks like aria-describedby was the culprit so I removed it since it seems to be optional. Can you take another look?

@cchaos
Copy link
Contributor

cchaos commented Jun 11, 2018

So the only thing I see now is that when I click the show button it starts reading all the contents of the flyout. Then if I tab to any tabbable contents within the flyout, it will start re-reading all the contents of the flyout.

@cjcenizal
Copy link
Contributor Author

I'm not 100% sure but I think that might be "normal" behavior. The Bootstrap modal has the same issue, where you click on the button and the entire modal content gets read, and then you tab into the modal and the whole thing gets read again. Tabbing through the buttons inside of the modal is fine though, and that's the same with the flyout. So I think this is OK.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Yeah it looks like in the modal patterns here: https://a11yproject.com/patterns#modal-windows it does the same thing. This looks good for this, though we may want to consider writing up some more docs on using aria-labelledby and aria-describedby specifically for flyouts.

@cjcenizal cjcenizal merged commit 6cc98e2 into elastic:master Jun 11, 2018
@cjcenizal cjcenizal deleted the flyout-accessibility branch June 11, 2018 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants