Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #246 from ckeditor/t/245
Browse files Browse the repository at this point in the history 
Fix: Made the `FocusTracker#isFocused` synchronous thanks to `FocusEvent#relatedTarget`. Closes #245.
  • Loading branch information
@oskarwrobel
oskarwrobel authored Jul 2, 2018
2 parents 66ead86 + b3724b6 commit 7467c4e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 49 deletions.
53 changes: 24 additions & 29 deletions src/focustracker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
* For licensing, see LICENSE.md.
*/

/* global setTimeout, clearTimeout */

/**
* @module utils/focustracker
*/
Expand Down Expand Up @@ -53,14 +51,6 @@ export default class FocusTracker {
* @member {Set.<HTMLElement>}
*/
this._elements = new Set();

/**
* Event loop timeout.
*
* @private
* @member {Number}
*/
this._nextEventLoopTimeout = null;
}

/**
Expand All @@ -74,7 +64,7 @@ export default class FocusTracker {
}

this.listenTo( element, 'focus', () => this._focus( element ), { useCapture: true } );
this.listenTo( element, 'blur', () => this._blur(), { useCapture: true } );
this.listenTo( element, 'blur', ( evt, domEvt ) => this._blur( domEvt ), { useCapture: true } );
this._elements.add( element );
}

Expand All @@ -101,35 +91,40 @@ export default class FocusTracker {
* @param {HTMLElement} element Element which has been focused.
*/
_focus( element ) {
clearTimeout( this._nextEventLoopTimeout );

this.focusedElement = element;
this.isFocused = true;
}

/**
* Clears currently focused element and set {@link #isFocused} as `false`.
* This method uses `setTimeout` to change order of fires `blur` and `focus` events.
*
* @private
* @fires blur
* @param {FocusEvent} domEvt The native DOM FocusEvent instance.
*/
_blur() {
clearTimeout( this._nextEventLoopTimeout );
_blur( domEvt ) {
const relatedTarget = domEvt.relatedTarget;
const isInnerBlur = Array.from( this._elements ).some( element => {
// element.contains( element ) -> true
return element.contains( relatedTarget );
} );

// If the blur was caused by focusing an element which is either:
//
// * registered in this focus tracker,
// * a child of an element registered in this focus tracker
//
// then don't fire the event to prevent rapid #isFocused changes. The focus remains within
// the registered elements; announcing this change is pointless.
//
// Note: In DOM, the native blur always precedes the following focus.
// https://github.com/ckeditor/ckeditor5-utils/issues/245
if ( isInnerBlur ) {
return;
}

this._nextEventLoopTimeout = setTimeout( () => {
this.focusedElement = null;
this.isFocused = false;
}, 0 );
this.focusedElement = null;
this.isFocused = false;
}

/**
* @event focus
*/

/**
* @event blur
*/
}

mix( FocusTracker, DomEmitterMixin );
Expand Down
37 changes: 17 additions & 20 deletions tests/focustracker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* For licensing, see LICENSE.md.
*/

/* global document, Event */
/* global document, FocusEvent */

import FocusTracker from '../src/focustracker';
import CKEditorError from '../src/ckeditorerror';
Expand Down Expand Up @@ -60,7 +60,7 @@ describe( 'FocusTracker', () => {

expect( focusTracker.isFocused ).to.false;

containerFirstInput.dispatchEvent( new Event( 'focus' ) );
containerFirstInput.dispatchEvent( new FocusEvent( 'focus' ) );

expect( focusTracker.isFocused ).to.true;
} );
Expand All @@ -69,8 +69,7 @@ describe( 'FocusTracker', () => {
focusTracker.add( containerFirstInput );
focusTracker.isFocused = true;

containerFirstInput.dispatchEvent( new Event( 'blur' ) );
testUtils.sinon.clock.tick( 0 );
containerFirstInput.dispatchEvent( new FocusEvent( 'blur' ) );

expect( focusTracker.isFocused ).to.false;
} );
Expand All @@ -82,7 +81,7 @@ describe( 'FocusTracker', () => {

expect( focusTracker.isFocused ).to.false;

containerFirstInput.dispatchEvent( new Event( 'focus' ) );
containerFirstInput.dispatchEvent( new FocusEvent( 'focus' ) );

expect( focusTracker.isFocused ).to.true;
} );
Expand All @@ -91,8 +90,7 @@ describe( 'FocusTracker', () => {
focusTracker.add( container );
focusTracker.isFocused = true;

containerFirstInput.dispatchEvent( new Event( 'blur' ) );
testUtils.sinon.clock.tick( 0 );
containerFirstInput.dispatchEvent( new FocusEvent( 'blur' ) );

expect( focusTracker.isFocused ).to.false;
} );
Expand All @@ -102,15 +100,17 @@ describe( 'FocusTracker', () => {

focusTracker.add( container );

containerFirstInput.dispatchEvent( new Event( 'focus' ) );
containerFirstInput.dispatchEvent( new FocusEvent( 'focus' ) );

focusTracker.listenTo( focusTracker, 'change:isFocused', changeSpy );

expect( focusTracker.isFocused ).to.true;

containerFirstInput.dispatchEvent( new Event( 'blur' ) );
containerSecondInput.dispatchEvent( new Event( 'focus' ) );
testUtils.sinon.clock.tick( 0 );
containerFirstInput.dispatchEvent( new FocusEvent( 'blur', {
relatedTarget: containerSecondInput
} ) );

containerSecondInput.dispatchEvent( new FocusEvent( 'focus' ) );

expect( focusTracker.isFocused ).to.true;
expect( changeSpy.notCalled ).to.true;
Expand All @@ -121,10 +121,9 @@ describe( 'FocusTracker', () => {
focusTracker.add( container );
focusTracker.isFocused = true;

container.dispatchEvent( new Event( 'blur' ) );
containerFirstInput.dispatchEvent( new Event( 'blur' ) );
containerSecondInput.dispatchEvent( new Event( 'focus' ) );
testUtils.sinon.clock.tick( 0 );
container.dispatchEvent( new FocusEvent( 'blur' ) );
containerFirstInput.dispatchEvent( new FocusEvent( 'blur' ) );
containerSecondInput.dispatchEvent( new FocusEvent( 'focus' ) );

expect( focusTracker.isFocused ).to.be.true;
} );
Expand All @@ -142,7 +141,7 @@ describe( 'FocusTracker', () => {
focusTracker.add( containerFirstInput );
focusTracker.remove( containerFirstInput );

containerFirstInput.dispatchEvent( new Event( 'focus' ) );
containerFirstInput.dispatchEvent( new FocusEvent( 'focus' ) );

expect( focusTracker.isFocused ).to.false;
} );
Expand All @@ -152,20 +151,18 @@ describe( 'FocusTracker', () => {
focusTracker.remove( containerFirstInput );
focusTracker.isFocused = true;

containerFirstInput.dispatchEvent( new Event( 'blur' ) );
testUtils.sinon.clock.tick( 0 );
containerFirstInput.dispatchEvent( new FocusEvent( 'blur' ) );

expect( focusTracker.isFocused ).to.true;
} );

it( 'should blur element before removing when is focused', () => {
focusTracker.add( containerFirstInput );
containerFirstInput.dispatchEvent( new Event( 'focus' ) );
containerFirstInput.dispatchEvent( new FocusEvent( 'focus' ) );

expect( focusTracker.isFocused ).to.true;

focusTracker.remove( containerFirstInput );
testUtils.sinon.clock.tick( 0 );

expect( focusTracker.isFocused ).to.false;
} );
Expand Down

0 comments on commit 7467c4e

Please sign in to comment.