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: Remove focus return handling from Popover #3056

Merged
merged 2 commits into from Oct 27, 2017
Merged

Conversation

aduth
Copy link
Member

@aduth aduth commented Oct 18, 2017

Related: #2911, #2888

This pull request seeks to resolve an issue where it requires two subsequent tabs to move between buttons in the block toolbar. This is caused by the withFocusReturn applied to Popover forcefully moving focus back to a button after the tooltip is to be dismissed. With #2888 and #2911 the intended behavior of focus returns on toggle button popovers is now managed within the Dropdown component. If Popover were to need to manage this, it should at the very least be excluded on the basis of the focusOnOpen prop (which Tooltip explicitly sets as false).

Testing instructions:

Verify that tabbing through block toolbar options requires a single tab to move through buttons.
Verify there is no regressions in the behavior of focus return for Dropdown components (e.g. pressing escape while inserter is open should return focus to the inserter button).

@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 Oct 18, 2017
@codecov
Copy link

codecov bot commented Oct 27, 2017

Codecov Report

Merging #3056 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3056      +/-   ##
==========================================
+ Coverage   31.56%   31.59%   +0.03%     
==========================================
  Files         221      221              
  Lines        6311     6314       +3     
  Branches     1121     1122       +1     
==========================================
+ Hits         1992     1995       +3     
  Misses       3631     3631              
  Partials      688      688
Impacted Files Coverage Δ
components/popover/detect-outside.js 25% <ø> (ø) ⬆️
components/popover/index.js 83.83% <100%> (+0.5%) ⬆️

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 d951c5f...989f84b. Read the comment docs.

@aduth aduth merged commit 15e8b96 into master Oct 27, 2017
@aduth aduth deleted the fix/toolbar-tab branch October 27, 2017 13:53
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.

None yet

1 participant