Bladeren bron

Improved ContextualBalloon to deal with an async Views.

Oskar Wróbel 8 jaren geleden
bovenliggende
commit
dc42ffb09d
2 gewijzigde bestanden met toevoegingen van 88 en 118 verwijderingen
  1. 15 6
      packages/ckeditor5-ui/src/contextualballoon.js
  2. 73 112
      packages/ckeditor5-ui/tests/contextualballoon.js

+ 15 - 6
packages/ckeditor5-ui/src/contextualballoon.js

@@ -83,8 +83,9 @@ export default class ContextualBalloon extends Plugin {
 	 * Adds a new view to the stack and makes it visible.
 	 *
 	 * @param {Object} data Configuration of the view.
-	 * @param {module:ui/view~View} view Content of the balloon.
-	 * @param {module:utils/dom/position~Options} position Positioning options.
+	 * @param {module:ui/view~View} [data.view] Content of the balloon.
+	 * @param {module:utils/dom/position~Options} [data.position] Positioning options.
+	 * @returns {Promise} A Promise resolved when the child {@link module:ui/view~View#init} is done.
 	 */
 	add( data ) {
 		if ( this.hasView( data.view ) ) {
@@ -105,7 +106,7 @@ export default class ContextualBalloon extends Plugin {
 		// Add new view to the stack.
 		this._stack.set( data.view, data );
 		// And display it.
-		this._show( data.view );
+		return this._show( data.view );
 	}
 
 	/**
@@ -114,6 +115,7 @@ export default class ContextualBalloon extends Plugin {
 	 * When there is no view in the stack then balloon will hide.
 	 *
 	 * @param {module:ui/view~View} view A view to be removed from the balloon.
+	 * @returns {Promise} A Promise resolved when the preceding view is ready.
 	 */
 	remove( view ) {
 		if ( !this.hasView( view ) ) {
@@ -125,6 +127,9 @@ export default class ContextualBalloon extends Plugin {
 			throw new CKEditorError( 'contextualballoon-remove-view-not-exist: Cannot remove configuration of not existing view.' );
 		}
 
+		// A Promise resolved when the preceding view is ready.
+		let promise = Promise.resolve();
+
 		// When visible view is being removed.
 		if ( this.visibleView === view ) {
 			// We need to remove it from the view content.
@@ -139,7 +144,7 @@ export default class ContextualBalloon extends Plugin {
 			// If it is some other view.
 			if ( last ) {
 				// Just show it.
-				this._show( last.view );
+				promise = this._show( last.view );
 			} else {
 				// Hide the balloon panel.
 				this.view.hide();
@@ -148,6 +153,8 @@ export default class ContextualBalloon extends Plugin {
 			// Just remove given view from the stack.
 			this._stack.delete( view );
 		}
+
+		return promise;
 	}
 
 	/**
@@ -164,10 +171,12 @@ export default class ContextualBalloon extends Plugin {
 	 *
 	 * @private
 	 * @param {module:ui/view~View} view View to show in the balloon.
+	 * @returns {Promise} A Promise resolved when the child {@link module:ui/view~View#init} is done.
 	 */
 	_show( view ) {
-		this.view.content.add( view );
-		this.view.attachTo( this._getBalloonPosition() );
+		return this.view.content.add( view ).then( () => {
+			this.view.attachTo( this._getBalloonPosition() );
+		} );
 	}
 
 	/**

+ 73 - 112
packages/ckeditor5-ui/tests/contextualballoon.js

@@ -11,7 +11,7 @@ import Template from '../src/template';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 
-/* global document */
+/* global document, setTimeout */
 
 describe( 'ContextualBalloon', () => {
 	let editor, editorElement, balloon, viewA, viewB;
@@ -27,11 +27,20 @@ describe( 'ContextualBalloon', () => {
 			editor = newEditor;
 			balloon = editor.plugins.get( ContextualBalloon );
 
+			// We don't need to test attachTo method of BalloonPanel it's enough to check if was called with proper data.
+			sinon.stub( balloon.view, 'attachTo', () => {} );
+
 			viewA = new ViewA();
 			viewB = new ViewB();
 
-			// We don't need to test attachTo method of BalloonPanel it's enough to check if was called with proper data.
-			sinon.stub( balloon.view, 'attachTo', () => {} );
+			// Add viewA to the pane and init viewB.
+			return Promise.all( [
+				balloon.add( {
+					view: viewA,
+					position: { target: 'fake' }
+				} ),
+				viewB.init(),
+			] );
 		} );
 	} );
 
@@ -61,20 +70,10 @@ describe( 'ContextualBalloon', () => {
 
 	describe( 'hasView()', () => {
 		it( 'should return true when given view is in stack', () => {
-			balloon.add( {
-				view: viewA,
-				position: { target: 'fake' }
-			} );
-
 			expect( balloon.hasView( viewA ) ).to.true;
 		} );
 
 		it( 'should return true when given view is in stack but is not visible', () => {
-			balloon.add( {
-				view: viewA,
-				position: { target: 'fake' }
-			} );
-
 			balloon.add( {
 				view: viewB,
 				position: { target: 'fake' }
@@ -85,17 +84,26 @@ describe( 'ContextualBalloon', () => {
 		} );
 
 		it( 'should return false when given view is not in stack', () => {
-			expect( balloon.hasView( viewA ) ).to.false;
+			expect( balloon.hasView( viewB ) ).to.false;
 		} );
 	} );
 
 	describe( 'add()', () => {
-		it( 'should add view to the stack and display in balloon', () => {
-			balloon.add( {
-				view: viewA,
+		it( 'should return promise resolved when view is ready', ( done ) => {
+			const clock = sinon.useFakeTimers();
+			const view = new LongInitView();
+
+			const result = balloon.add( {
+				view: view,
 				position: { target: 'fake' }
 			} );
 
+			expect( result ).to.instanceof( Promise );
+			result.then( done );
+			clock.tick( 11 );
+		} );
+
+		it( 'should add view to the stack and display in balloon attached using given position options', () => {
 			expect( balloon.view.content.length ).to.equal( 1 );
 			expect( balloon.view.content.get( 0 ) ).to.deep.equal( viewA );
 			expect( balloon.view.attachTo.calledOnce ).to.true;
@@ -103,11 +111,6 @@ describe( 'ContextualBalloon', () => {
 		} );
 
 		it( 'should throw an error when try to add the same view more than once', () => {
-			balloon.add( {
-				view: viewA,
-				position: { target: 'fake' }
-			} );
-
 			expect( () => {
 				balloon.add( {
 					view: viewA,
@@ -117,11 +120,6 @@ describe( 'ContextualBalloon', () => {
 		} );
 
 		it( 'should add multiple views to he stack and display last one', () => {
-			balloon.add( {
-				view: viewA,
-				position: { target: 'fake' }
-			} );
-
 			balloon.add( {
 				view: viewB,
 				position: { target: 'fake' }
@@ -131,47 +129,31 @@ describe( 'ContextualBalloon', () => {
 			expect( balloon.view.content.get( 0 ) ).to.deep.equal( viewB );
 		} );
 
-		it( 'should add multiple views to the stack and keep balloon in the same position', () => {
-			balloon.add( {
-				view: viewA,
-				position: { target: 'fake', foo: 'bar' }
-			} );
-
-			balloon.add( {
+		it( 'should keep balloon at the same position after adding next view', () => {
+			return balloon.add( {
 				view: viewB,
-				position: { target: 'fake', bar: 'biz' }
-			} );
+				position: { target: 'other' }
+			} )
+			.then( () => {
+				expect( balloon.view.attachTo.calledTwice ).to.true;
 
-			expect( balloon.view.attachTo.calledTwice ).to.true;
-
-			expect( balloon.view.attachTo.firstCall.args[ 0 ] ).to.deep.equal( {
-				target: 'fake',
-				foo: 'bar'
-			} );
+				expect( balloon.view.attachTo.firstCall.args[ 0 ] ).to.deep.equal( {
+					target: 'fake'
+				} );
 
-			expect( balloon.view.attachTo.secondCall.args[ 0 ] ).to.deep.equal( {
-				target: 'fake',
-				foo: 'bar'
+				expect( balloon.view.attachTo.secondCall.args[ 0 ] ).to.deep.equal( {
+					target: 'fake'
+				} );
 			} );
 		} );
 	} );
 
 	describe( 'visibleView', () => {
 		it( 'should return data of currently visible view', () => {
-			balloon.add( {
-				view: viewA,
-				position: { target: 'fake' }
-			} );
-
 			expect( balloon.visibleView ).to.equal( viewA );
 		} );
 
 		it( 'should return data of currently visible view when there is more than one in the stack', () => {
-			balloon.add( {
-				view: viewA,
-				position: { target: 'fake' }
-			} );
-
 			balloon.add( {
 				view: viewB,
 				position: { target: 'fake' }
@@ -181,28 +163,26 @@ describe( 'ContextualBalloon', () => {
 		} );
 
 		it( 'should return `null` when the stack is empty', () => {
+			balloon.remove( viewA );
 			expect( balloon.visibleView ).to.null;
 		} );
 	} );
 
 	describe( 'remove()', () => {
+		it( 'should return promise', () => {
+			expect( balloon.remove( viewA ) ).to.instanceof( Promise );
+		} );
+
 		it( 'should remove given view and hide balloon when there is no other view to display', () => {
-			balloon.add( {
-				view: viewA,
-				position: { target: 'fake' }
-			} );
+			balloon.view.isVisible = true;
 
 			balloon.remove( viewA );
 
 			expect( balloon.visibleView ).to.null;
+			expect( balloon.view.isVisible ).to.false;
 		} );
 
-		it( 'should remove given view and set previous in the stack as visible when removed view was visible', () => {
-			balloon.add( {
-				view: viewA,
-				position: { target: 'fake' }
-			} );
-
+		it( 'should remove given view and set preceding in the stack as visible when removed view was visible', () => {
 			balloon.add( {
 				view: viewB,
 				position: { target: 'fake' }
@@ -213,12 +193,26 @@ describe( 'ContextualBalloon', () => {
 			expect( balloon.visibleView ).to.equal( viewA );
 		} );
 
-		it( 'should remove given view from the stack when view is not visible', () => {
+		it( 'should wait for init of preceding view when was is not ready', ( done ) => {
+			const clock = sinon.useFakeTimers();
+			const view = new LongInitView();
+
+			balloon.add( {
+				view: view,
+				position: { target: 'fake' }
+			} );
+
 			balloon.add( {
-				view: viewA,
+				view: viewB,
 				position: { target: 'fake' }
 			} );
 
+			balloon.remove( viewB ).then( done );
+
+			clock.tick( 11 );
+		} );
+
+		it( 'should remove given view from the stack when view is not visible', () => {
 			balloon.add( {
 				view: viewB,
 				position: { target: 'fake' }
@@ -231,18 +225,13 @@ describe( 'ContextualBalloon', () => {
 
 		it( 'should throw an error when there is no given view in the stack', () => {
 			expect( () => {
-				balloon.remove( viewA );
+				balloon.remove( viewB );
 			} ).to.throw( CKEditorError, /^contextualballoon-remove-view-not-exist/ );
 		} );
 	} );
 
 	describe( 'updatePosition()', () => {
 		it( 'should attach balloon to the target using the same position options as currently set', () => {
-			balloon.add( {
-				view: viewA,
-				position: { target: 'fake' }
-			} );
-
 			balloon.view.attachTo.reset();
 
 			balloon.updatePosition();
@@ -252,19 +241,10 @@ describe( 'ContextualBalloon', () => {
 		} );
 
 		it( 'should attach balloon to the target using the same position options as currently set when there is more than one view', () => {
-			balloon.add( {
-				view: viewA,
-				position: {
-					target: 'fake',
-					foo: 'bar'
-				}
-			} );
-
 			balloon.add( {
 				view: viewB,
 				position: {
-					target: 'fake',
-					bar: 'biz'
+					target: 'other'
 				}
 			} );
 
@@ -274,30 +254,13 @@ describe( 'ContextualBalloon', () => {
 
 			expect( balloon.view.attachTo.calledOnce );
 			expect( balloon.view.attachTo.firstCall.args[ 0 ] ).to.deep.equal( {
-				target: 'fake',
-				foo: 'bar'
-			} );
-		} );
-
-		it( 'should remove given view from the stack when view is not visible', () => {
-			balloon.add( {
-				view: viewA,
-				position: { target: 'fake' }
-			} );
-
-			balloon.add( {
-				view: viewB,
-				position: { target: 'fake' }
+				target: 'fake'
 			} );
-
-			balloon.remove( viewA );
-
-			expect( balloon.visibleView ).to.equal( viewB );
 		} );
 
 		it( 'should throw an error when there is no given view in the stack', () => {
 			expect( () => {
-				balloon.remove( viewA );
+				balloon.remove( viewB );
 			} ).to.throw( CKEditorError, /^contextualballoon-remove-view-not-exist/ );
 		} );
 	} );
@@ -311,22 +274,20 @@ describe( 'ContextualBalloon', () => {
 	} );
 } );
 
-class ViewA extends View {
+class GenericView extends View {
 	constructor( locale ) {
 		super( locale );
 
-		this.template = new Template( {
-			tag: 'div'
-		} );
+		this.template = new Template( { tag: 'div' } );
 	}
 }
 
-class ViewB extends View {
-	constructor( locale ) {
-		super( locale );
+class ViewA extends GenericView {}
 
-		this.template = new Template( {
-			tag: 'div'
-		} );
+class ViewB extends GenericView {}
+
+class LongInitView extends GenericView {
+	init() {
+		setTimeout( () => super.init(), 10 );
 	}
 }