Skip to content

Conversation

@meganaconley
Copy link
Contributor

Description

Add accessibility features to the Popover control prop. This way, any element can be passed as a control, but it will still work as a button.

  • tabIndex: puts normally non-tabbable elements into the normal tab flow
  • aria-haspopup: indicates the button triggers a popup
  • role="button": screenreader will announce element as button
  • onKeyPress handler: if the control is not a button, adds enter and space as a way to trigger the popover.

@CLAassistant
Copy link

CLAassistant commented Nov 14, 2019

CLA assistant check
All committers have signed the CLA.

@netlify
Copy link

netlify bot commented Nov 14, 2019

@meganaconley meganaconley requested a review from a team November 14, 2019 17:31
Copy link
Member

@prsdthkr prsdthkr left a comment

Choose a reason for hiding this comment

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

Generally the onKeyPress works well with simple Popover controls. It gets a bit tricky with complex controls like ComboboxInput, LocalizationEditor, SearchInput because they have additional input and button within.

@jbadan
Copy link
Contributor

jbadan commented Nov 15, 2019

Once the Popover is open - how do I navigate inside of it with keyboard? Or is that out of scope for this pr.
I forgot to turn keycastr on but I tabbed to the button dropdown and then was using the arrow keys - not the mouse like it looks. Bad gif sorry!

2019-11-15 13 51 52

@jacobdevera
Copy link
Contributor

@jbadan keyboard navigation for Popover is through tabbing in #793

I would think that a basic Popover should navigate via tabbing since it can contain any content, but it makes sense that certain compositions (Dropdown, ComboboxInput, etc.) would use arrow keys. Probably out of scope for now.

@meganaconley meganaconley requested review from a team and prsdthkr November 18, 2019 21:59
Copy link
Member

@prsdthkr prsdthkr left a comment

Choose a reason for hiding this comment

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

✅ QA looks good to me on Chrome with VoiceOver.

@meganaconley meganaconley requested a review from a team November 19, 2019 18:07
@meganaconley meganaconley merged commit 6ac4e11 into master Nov 19, 2019
@meganaconley meganaconley deleted the feat/accessible-popover-control branch November 19, 2019 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants