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

Need solution for screen reader to read popover content without forcing ownFocus #729

Closed
aphelionz opened this issue Apr 29, 2018 · 3 comments · Fixed by #821
Closed

Need solution for screen reader to read popover content without forcing ownFocus #729

aphelionz opened this issue Apr 29, 2018 · 3 comments · Fixed by #821
Labels
accessibility - WCAG A Level A WCAG Accessibility Criteria accessibility

Comments

@aphelionz
Copy link
Contributor

aphelionz commented Apr 29, 2018

Steps to reproduce (assumes ChromeVox or similar))

  1. Open https://elastic.github.io/eui/#/layout/popover in a new tab.
  2. Tab to the first "Show popover" button
  3. Press enter
  4. Note the popover appears but is not read
  5. Tab to the next "Show popover" button under the "Trap focus" section
  6. Press enter

This reacts more like I would expect, however I see the need to have the ability to explicitly set ownFocus. The popovers still need to be read, though... I'm wondering if there's some sort of aria-describedby trick we can use here.

Category: #728: Elastic UI Popover Accessibility
Relevant WCAG Criteria: 3.3.2 Input Assistance: Labels or Instructions

@aphelionz aphelionz added accessibility accessibility - WCAG A Level A WCAG Accessibility Criteria labels Apr 29, 2018
snide added a commit to snide/eui that referenced this issue May 11, 2018
@cjcenizal
Copy link
Contributor

@bhavyarm pointed out to me that the aXe accessibility validator complains about the aria-controls attribute that's added to the button which triggers the popover. This attribute connects the button to the popover it opens, but because it's applied even when the popover doesn't exist in the DOM, it's invalid. I think the solution is to remove this attribute while the popover is closed.

@snide
Copy link
Contributor

snide commented May 11, 2018

@cjcenizal I'm in this one now, so I'll see what I can do. Also, I couldn't see any reason we wouldn't want to simply apply aria-live="assertive" to these. I can't think of any time you'd want to "click" them and not have the content read.

@cjcenizal
Copy link
Contributor

cjcenizal commented May 11, 2018

Per convo in Slack, my hunch is that there are 2 scenarios to account for:

  • Where the popover content is interactive (and ownFocus is set)
  • Where the content is read-only (and we want to use aria-live to read it out)

snide added a commit that referenced this issue May 14, 2018
* pagination labeling fixes #735
* fixes #621 adds menubar roles to side nav
* fixes #734, adds more descriptive aria labeling for pagination
* fixes #729, apply aria-live to popovers
* fixes #723, provides screen reader notice for bottom bar appearance
* fix ownFocus needing to be true for non-context menu sitations. apply aria labels depending upon ownFocus state
* fixes #616 adds home page label
* fixes #687, documentation button labeling
* fixes #617 applies aria label to theme selector
* fixes #746 applies proper label to select all checkboxes in table
* clone element and id no longer needed for popover
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility - WCAG A Level A WCAG Accessibility Criteria accessibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants