Skip to content

Commit

Permalink
Check to ensure focus has intentionally left the wrapped component in…
Browse files Browse the repository at this point in the history
… `withFocusOutside` HOC (#17051)

* Adds check to determine whether event occured within element trapping focus

Previously there was no check to see whether the blur event occured within the wrapped component. If it occurs within then we do not want to trigger the `handleFocusOutside` callback because (by definition) the focus has not moved outside the wrapped element.

This may have been a regression introduced in #16878

* Fix handling blur event when document is not actually focused

Previously we fixed the wrong problem. The actual issue is that previously we were handling document blur events as true “focus outside” events when in fact they should be ignored as if the document is not focused then presumably the user did not intent the blur.

* Update to test for document loss of focus scenario
  • Loading branch information
getdave authored and gziolo committed Aug 29, 2019
1 parent 0690500 commit 9f0d249
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@ export default createHigherOrderComponent(
}

this.blurCheckTimeout = setTimeout( () => {
// If document is not focused then focus should remain
// inside the wrapped component and therefore we cancel
// this blur event thereby leaving focus in place.
// https://developer.mozilla.org/en-US/docs/Web/API/Document/hasFocus.
if ( ! document.hasFocus() ) {
event.preventDefault();
return;
}
if ( 'function' === typeof this.node.handleFocusOutside ) {
this.node.handleFocusOutside( event );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import ReactDOM from 'react-dom';
let wrapper, onFocusOutside;

describe( 'withFocusOutside', () => {
let origHasFocus;

const EnhancedComponent = withFocusOutside(
class extends Component {
handleFocusOutside() {
Expand Down Expand Up @@ -51,12 +53,21 @@ describe( 'withFocusOutside', () => {
};

beforeEach( () => {
// Mock document.hasFocus() to always be true for testing
// note: we overide this for some tests.
origHasFocus = document.hasFocus;
document.hasFocus = () => true;

onFocusOutside = jest.fn();
wrapper = TestUtils.renderIntoDocument(
getTestComponent( EnhancedComponent, { onFocusOutside } )
);
} );

afterEach( () => {
document.hasFocus = origHasFocus;
} );

it( 'should not call handler if focus shifts to element within component', () => {
simulateEvent( 'focus' );
simulateEvent( 'blur' );
Expand Down Expand Up @@ -91,6 +102,19 @@ describe( 'withFocusOutside', () => {
expect( onFocusOutside ).toHaveBeenCalled();
} );

it( 'should not call handler if focus shifts outside the component when the document does not have focus', () => {
// Force document.hasFocus() to return false to simulate the window/document losing focus
// See https://developer.mozilla.org/en-US/docs/Web/API/Document/hasFocus.
document.hasFocus = () => false;

simulateEvent( 'focus' );
simulateEvent( 'blur' );

jest.runAllTimers();

expect( onFocusOutside ).not.toHaveBeenCalled();
} );

it( 'should cancel check when unmounting while queued', () => {
simulateEvent( 'focus' );
simulateEvent( 'input' );
Expand Down

0 comments on commit 9f0d249

Please sign in to comment.