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: Trigger focus return by tracking focus events #2321

Merged
merged 1 commit into from
Aug 10, 2017

Conversation

aduth
Copy link
Member

@aduth aduth commented Aug 9, 2017

Partially addresses #2306

This pull request seeks to refactor the withFocusReturn higher-order component to track an instance property reflecting whether the rendered element currently has focus. This proves to be more reliable when the disposable component is portaled to another location in the DOM (#2306), since the previous element.contains test will not match said content. Instead, the implementation here leverages event bubbling (and event bubbling of portaled content) to track at all times whether the wrapped component has focus. Then, the focus return condition is simply a matter of: Is the component unmounting while it has focus?

Testing instructions:

Verify that focus is returned to initial active element after dismissing a withFocusReturn component:

  1. Navigate to Gutenberg > New Post
  2. Open an inserter
  3. Press Escape
  4. Note that focus is returned 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 Aug 9, 2017
@aduth aduth requested a review from afercia August 9, 2017 21:40
@codecov
Copy link

codecov bot commented Aug 9, 2017

Codecov Report

Merging #2321 into master will increase coverage by 0.07%.
The diff coverage is 90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2321      +/-   ##
==========================================
+ Coverage   25.34%   25.42%   +0.07%     
==========================================
  Files         152      152              
  Lines        4754     4760       +6     
  Branches      807      808       +1     
==========================================
+ Hits         1205     1210       +5     
- Misses       2996     2997       +1     
  Partials      553      553
Impacted Files Coverage Δ
components/higher-order/with-focus-return/index.js 91.66% <90%> (-8.34%) ⬇️

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 659b1bb...011903f. Read the comment docs.

Copy link
Contributor

@afercia afercia left a comment

Choose a reason for hiding this comment

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

Tested on the Inserter, works great. Thanks!

@aduth aduth merged commit 3c55b72 into master Aug 10, 2017
@aduth aduth deleted the fix/popover-focus branch August 10, 2017 15:54
aduth added a commit that referenced this pull request Mar 13, 2018
Existed prior to refactoring to track events (#2321, #931).

Causes conflict where intentional tabs away from the mounted element would revert focus back to the element (twice tab to dismiss a tooltip'd element)
aduth added a commit that referenced this pull request Mar 16, 2018
Existed prior to refactoring to track events (#2321, #931).

Causes conflict where intentional tabs away from the mounted element would revert focus back to the element (twice tab to dismiss a tooltip'd element)
aduth added a commit that referenced this pull request Mar 21, 2018
Existed prior to refactoring to track events (#2321, #931).

Causes conflict where intentional tabs away from the mounted element would revert focus back to the element (twice tab to dismiss a tooltip'd element)
aduth added a commit that referenced this pull request Mar 26, 2018
Existed prior to refactoring to track events (#2321, #931).

Causes conflict where intentional tabs away from the mounted element would revert focus back to the element (twice tab to dismiss a tooltip'd element)
ceyhun pushed a commit that referenced this pull request Jun 17, 2020
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.

2 participants