Merge pull request #282 from ckeditor/t/281

Fix: The `clickOutsideHandler` helper should use `mousedown` instead of `mouseup` event. Closes #281.

Author: oskarwrobel

src/bindings/clickoutsidehandler.js

@@ -22,7 +22,7 @@
  * @param {Function} options.callback Function fired after clicking outside of specified elements.
  */
 export default function clickOutsideHandler( { emitter, activator, callback, contextElements } ) {
-	emitter.listenTo( document, 'mouseup', ( evt, { target } ) => {
+	emitter.listenTo( document, 'mousedown', ( evt, { target } ) => {
 		if ( !activator() ) {
 			return;
 		}

tests/bindings/clickoutsidehandler.js

@@ -38,23 +38,23 @@ describe( 'clickOutsideHandler', () => {
 		document.body.removeChild( contextElement2 );
 	} );
 
-	it( 'should fired callback after clicking out of context element when listener is active', () => {
+	it( 'should execute upon #mousedown outside of the contextElements (activator is active)', () => {
 		activator.returns( true );
 
-		document.body.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) );
+		document.body.dispatchEvent( new Event( 'mousedown', { bubbles: true } ) );
 
 		sinon.assert.calledOnce( actionSpy );
 	} );
 
-	it( 'should not fired callback after clicking out of context element when listener is not active', () => {
+	it( 'should not execute upon #mousedown outside of the contextElements (activator is inactive)', () => {
 		activator.returns( false );
 
-		document.body.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) );
+		document.body.dispatchEvent( new Event( 'mousedown', { bubbles: true } ) );
 
 		sinon.assert.notCalled( actionSpy );
 	} );
 
-	it( 'should not fired callback after clicking on context element when listener is active', () => {
+	it( 'should not execute upon #mousedown from one of the contextElements (activator is active)', () => {
 		activator.returns( true );
 
 		contextElement1.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) );
@@ -64,7 +64,7 @@ describe( 'clickOutsideHandler', () => {
 		sinon.assert.notCalled( actionSpy );
 	} );
 
-	it( 'should not fired callback after clicking on context element when listener is not active', () => {
+	it( 'should not execute upon #mousedown from one of the contextElements (activator is inactive)', () => {
 		activator.returns( false );
 
 		contextElement1.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) );
@@ -74,7 +74,7 @@ describe( 'clickOutsideHandler', () => {
 		sinon.assert.notCalled( actionSpy );
 	} );
 
-	it( 'should listen when model initial `ifActive` value was `true`', () => {
+	it( 'should execute if the activator function returns `true`', () => {
 		const spy = testUtils.sinon.spy();
 
 		activator.returns( true );
@@ -86,12 +86,12 @@ describe( 'clickOutsideHandler', () => {
 			callback: spy
 		} );
 
-		document.body.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) );
+		document.body.dispatchEvent( new Event( 'mousedown', { bubbles: true } ) );
 
 		sinon.assert.calledOnce( spy );
 	} );
 
-	it( 'should not listen when model initial `ifActive` value was `false`', () => {
+	it( 'should not execute if the activator function returns `false`', () => {
 		const spy = testUtils.sinon.spy();
 
 		activator.returns( false );
@@ -103,30 +103,40 @@ describe( 'clickOutsideHandler', () => {
 			callback: spy
 		} );
 
-		document.body.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) );
+		document.body.dispatchEvent( new Event( 'mousedown', { bubbles: true } ) );
 
 		sinon.assert.notCalled( spy );
 	} );
 
-	it( 'should react on model `ifActive` property change', () => {
+	it( 'should react to the activator\'s return value change', () => {
 		activator.returns( true );
 
-		document.body.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) );
+		document.body.dispatchEvent( new Event( 'mousedown', { bubbles: true } ) );
 
 		sinon.assert.calledOnce( actionSpy );
 
 		activator.returns( false );
 
-		document.body.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) );
+		document.body.dispatchEvent( new Event( 'mousedown', { bubbles: true } ) );
 
 		// Still called once, was not called second time.
 		sinon.assert.calledOnce( actionSpy );
 
 		activator.returns( true );
 
-		document.body.dispatchEvent( new Event( 'mouseup', { bubbles: true } ) );
+		document.body.dispatchEvent( new Event( 'mousedown', { bubbles: true } ) );
 
 		// Called one more time.
 		sinon.assert.calledTwice( actionSpy );
 	} );
+
+	it( 'should not execute if one of contextElements contains the DOM event target', () => {
+		const target = document.createElement( 'div' );
+		activator.returns( true );
+
+		contextElement2.appendChild( target );
+		target.dispatchEvent( new Event( 'mousedown', { bubbles: true } ) );
+
+		sinon.assert.notCalled( actionSpy );
+	} );
 } );