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

Components: Close popover by escape keypress #2697

Merged
merged 4 commits into from
Sep 25, 2017
Merged

Conversation

aduth
Copy link
Member

@aduth aduth commented Sep 7, 2017

Related: #2323

This pull request seeks to enhance the Popover component to handle escape keypress and focus return for closing the focused dialog. This also helps consolidate existing but inconsistently-applied approaches for this behavior.

Testing instructions:

Verify that a focused popover can be closed by clicking outside or pressing escape. When pressing escape, observe that focus returns to the element which had been focused prior to opening the dialog.

  1. Navigate to Gutenberg > New Post
  2. Click Insert in the header menu
  3. Press Escape
  4. Note that the Insert button is focused

(Repeat for Visibility, Schedule popovers)

Caveats:

When tabbing through to the end of a popover's tabbable elements, we should do one or the other of:

  • Circle back to beginning of tabbables in popover (constrain focus, as in a modal)
  • Direct focus to next in sequence from where popover had been mounted

Click outside is handled internally by the Popover component
onClickOutside passes event which is relied upon by PostVisibility, but we don't want to guarantee this with signature of onClose
@aduth aduth added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Feature] UI Components Impacts or related to the UI component system labels Sep 7, 2017
@codecov
Copy link

codecov bot commented Sep 7, 2017

Codecov Report

Merging #2697 into master will increase coverage by 0.02%.
The diff coverage is 41.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2697      +/-   ##
==========================================
+ Coverage   33.64%   33.67%   +0.02%     
==========================================
  Files         185      185              
  Lines        5584     5591       +7     
  Branches      973      975       +2     
==========================================
+ Hits         1879     1883       +4     
- Misses       3138     3139       +1     
- Partials      567      569       +2
Impacted Files Coverage Δ
components/popover/detect-outside.js 25% <ø> (ø) ⬆️
editor/inserter/index.js 0% <0%> (ø) ⬆️
editor/inserter/menu.js 49.59% <100%> (+1.18%) ⬆️
editor/sidebar/post-schedule/index.js 66.66% <100%> (+1.66%) ⬆️
editor/sidebar/post-visibility/index.js 45.23% <25%> (-0.92%) ⬇️
components/popover/index.js 87.64% <37.5%> (-5.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6ceb8b...9c539d0. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] UI Components Impacts or related to the UI component system [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant