Преглед на файлове

Moved the logic of adding child controllers from Controller#(add|remove|get)Child to ControllerCollection methods.

Aleksander Nowodzinski преди 10 години
родител
ревизия
7788245309

+ 35 - 74
packages/ckeditor5-ui/src/ui/controller.js

@@ -51,6 +51,40 @@ CKEDITOR.define( [
 			this.collections = new Collection( {
 				idProperty: 'name'
 			} );
+
+			// Listen to {@link ControllerCollection#add} and {@link ControllerCollection#remove}
+			// of newly added Collection to synchronize this controller's view and children
+			// controllers' views in the future.
+			this.collections.on( 'add', ( evt, collection ) => {
+				// Set the {@link ControllerCollection#parent} to this controller.
+				// It allows the collection to determine the {@link #ready} state of this controller
+				// and accordingly initialize a child controller when added.
+				collection.parent = this;
+
+				this.listenTo( collection, 'add', ( evt, childController, index ) => {
+					// Child view is added to corresponding region in this controller's view
+					// when a new Controller joins the collection.
+					if ( this.ready && childController.view ) {
+						this.view.addChild( collection.name, childController.view, index );
+					}
+				} );
+
+				this.listenTo( collection, 'remove', ( evt, childController ) => {
+					// Child view is removed from corresponding region in this controller's view
+					// when a new Controller is removed from the the collection.
+					if ( this.ready && childController.view ) {
+						this.view.removeChild( collection.name, childController.view );
+					}
+				} );
+			} );
+
+			this.collections.on( 'remove', ( evt, collection ) => {
+				// Release the collection. Once removed from {@link #collections}, it can be
+				// moved to another controller.
+				collection.parent = null;
+
+				this.stopListening( collection );
+			} );
 		}
 
 		/**
@@ -81,74 +115,6 @@ CKEDITOR.define( [
 		}
 
 		/**
-		 * Adds a child controller to one of the {@link #collections}.
-		 * If this controller instance is ready, the child view will be initialized when added.
-		 * If this controller and child controller have views, the child view will be added
-		 * to corresponding region in this controller's view.
-		 *
-		 * @param {String} collectionName One of {@link #collections} the child should be added to.
-		 * @param {Controller} childController A child controller.
-		 * @param {Number} [index] Index at which the child will be added to the collection.
-		 * @returns {Promise} A Promise resolved when the child is added.
-		 */
-		addChild( collectionName, childController, index ) {
-			const collection = this.collections.get( collectionName );
-
-			// ChildController.init() returns Promise.
-			let promise = Promise.resolve();
-
-			collection.add( childController, index );
-
-			if ( this.ready ) {
-				if ( childController.view ) {
-					this.view.addChild( collectionName, childController.view, index );
-				}
-
-				if ( !childController.ready ) {
-					promise = promise.then( () => {
-						return childController.init();
-					} );
-				}
-			}
-
-			return promise;
-		}
-
-		/**
-		 * Removes a child controller from one of the {@link #collections}.
-		 * If this controller and child controller have views, the child view will be removed
-		 * from corresponding region in this controller's view.
-		 *
-		 * @param {String} collectionName One of {@link #collections} the child should be removed from.
-		 * @param {Controller} childController A child controller.
-		 * @returns {Controller} A child controller instance after removal.
-		 */
-		removeChild( collectionName, childController ) {
-			const collection = this.collections.get( collectionName );
-
-			collection.remove( childController );
-
-			if ( this.ready && childController.view ) {
-				this.view.removeChild( collectionName, childController.view );
-			}
-
-			return childController;
-		}
-
-		/**
-		 * Returns a child controller from one of the {@link #collections} at given `index`.
-		 *
-		 * @param {String} collectionName One of {@link #collections} the child should be retrieved from.
-		 * @param {Number} [index] An index of desired controller.
-		 * @returns {Controller} A child controller instance.
-		 */
-		getChild( collectionName, index ) {
-			const collection = this.collections.get( collectionName );
-
-			return collection.get( index );
-		}
-
-		/**
 		 * Destroys the controller instance. The process includes:
 		 *
 		 * 1. Destruction of the child {@link #view}.
@@ -162,12 +128,7 @@ CKEDITOR.define( [
 
 			for ( collection of this.collections ) {
 				for ( childController of collection ) {
-					if ( this.view && childController.view ) {
-						this.view.removeChild( collection.name, childController.view );
-					}
-
 					promises.push( childController.destroy() );
-
 					collection.remove( childController );
 				}
 
@@ -215,7 +176,7 @@ CKEDITOR.define( [
 
 			for ( collection of this.collections ) {
 				for ( childController of collection ) {
-					if ( this.view, childController.view ) {
+					if ( this.view && childController.view ) {
 						this.view.addChild( collection.name, childController.view );
 					}
 

+ 34 - 0
packages/ckeditor5-ui/src/ui/controllercollection.js

@@ -33,9 +33,43 @@ CKEDITOR.define( [
 				throw new CKEditorError( 'ui-controllercollection-no-name: ControllerCollection must be initialized with a name.' );
 			}
 
+			/**
+			 * Name of this collection.
+			 *
+			 * @property {String}
+			 */
 			this.name = name;
+
+			/**
+			 * Parent controller of this collection.
+			 *
+			 * @property {Controller}
+			 */
+			this.parent = null;
 		}
 
+		/**
+		 * Adds a child controller to the collection. If {@link #parent} {@link Controller}
+		 * instance is ready, the child view is initialized when added.
+		 *
+		 * @param {Controller} controller A child controller.
+		 * @param {Number} [index] Index at which the child will be added to the collection.
+		 * @returns {Promise} A Promise resolved when the child {@link Controller#init} is done.
+		 */
+		add( controller, index ) {
+			super.add( controller, index );
+
+			// ChildController.init() returns Promise.
+			let promise = Promise.resolve();
+
+			if ( this.parent && this.parent.ready && !controller.ready ) {
+				promise = promise.then( () => {
+					return controller.init();
+				} );
+			}
+
+			return promise;
+		}
 	}
 
 	return ControllerCollection;

+ 96 - 180
packages/ckeditor5-ui/tests/ui/controller.js

@@ -19,7 +19,7 @@ const modules = bender.amd.require( 'ckeditor',
 );
 
 let View, Controller, Model, CKEditorError, Collection, ControllerCollection;
-let ParentView;
+let ParentController, ParentView;
 
 bender.tools.createSinonSandbox();
 
@@ -82,19 +82,20 @@ describe( 'Controller', () => {
 
 		it( 'should initialize child controllers in own collections', () => {
 			const parentController = new Controller();
-			parentController.collections.add( new ControllerCollection( 'buttons' ) );
+			const buttonCollection = new ControllerCollection( 'buttons' );
+			parentController.collections.add( buttonCollection );
 
 			const childController1 = new Controller();
 			const childController2 = new Controller();
 			const spy1 = bender.sinon.spy( childController1, 'init' );
 			const spy2 = bender.sinon.spy( childController2, 'init' );
 
-			parentController.addChild( 'buttons', childController1 );
-			parentController.addChild( 'buttons', childController2 );
+			buttonCollection.add( childController1 );
+			buttonCollection.add( childController2 );
 
 			return parentController.init().then( () => {
-				expect( parentController.getChild( 'buttons', 0 ) ).to.be.equal( childController1 );
-				expect( parentController.getChild( 'buttons', 1 ) ).to.be.equal( childController2 );
+				expect( buttonCollection.get( 0 ) ).to.be.equal( childController1 );
+				expect( buttonCollection.get( 1 ) ).to.be.equal( childController2 );
 
 				sinon.assert.calledOnce( spy1 );
 				sinon.assert.calledOnce( spy2 );
@@ -102,192 +103,97 @@ describe( 'Controller', () => {
 		} );
 	} );
 
-	describe( 'addChild', () => {
-		beforeEach( defineParentViewClass );
-
-		it( 'should add a child controller to given collection and return promise', () => {
-			const parentController = new Controller();
-			const childController = new Controller();
-			const collection = new ControllerCollection( 'x' );
-
-			parentController.collections.add( collection );
-
-			const returned = parentController.addChild( 'x', childController );
-
-			expect( returned ).to.be.an.instanceof( Promise );
-			expect( collection.get( 0 ) ).to.be.equal( childController );
-		} );
-
-		it( 'should add a child controller at given position', () => {
-			const parentController = new Controller();
-			const childController1 = new Controller();
-			const childController2 = new Controller();
-			const collection = new ControllerCollection( 'x' );
-
-			parentController.collections.add( collection );
-
-			parentController.addChild( 'x', childController1 );
-			parentController.addChild( 'x', childController2, 0 );
-
-			expect( collection.get( 0 ) ).to.be.equal( childController2 );
-			expect( collection.get( 1 ) ).to.be.equal( childController1 );
-		} );
-
-		it( 'should add a child controller which has no view', () => {
-			const parentController = new Controller( null, new ParentView() );
-			const childController = new Controller();
-
-			parentController.collections.add( new ControllerCollection( 'x' ) );
-
-			return parentController.init()
-				.then( () => {
-					return parentController.addChild( 'x', childController );
-				} )
-				.then( () => {
-					expect( parentController.getChild( 'x', 0 ) ).to.be.equal( childController );
-				} );
-		} );
-
-		it( 'should append child controller\'s view to parent controller\'s view', () => {
-			const parentView = new ParentView();
-			const parentController = new Controller( null, parentView );
-			const childController = new Controller( null, new View() );
-
-			const spy1 = bender.sinon.spy( parentView, 'addChild' );
-
-			parentController.collections.add( new ControllerCollection( 'x' ) );
-			parentController.addChild( 'x', childController );
-
-			sinon.assert.notCalled( spy1 );
-
-			parentController.removeChild( 'x', childController );
-
-			return parentController.init()
-				.then( () => {
-					return parentController.addChild( 'x', childController );
-				} )
-				.then( () => {
-					sinon.assert.calledOnce( spy1 );
-					sinon.assert.calledWithExactly( spy1, 'x', childController.view, undefined );
-				} );
-		} );
-
-		it( 'should append child controller\'s view to parent controller\'s view at given index', () => {
-			const parentController = new Controller( null, new ParentView() );
-
-			const childView1 = new View();
-			const childController1 = new Controller( null, childView1 );
-			const childView2 = new View();
-			const childController2 = new Controller( null, childView2 );
-
-			parentController.collections.add( new ControllerCollection( 'x' ) );
-
-			return parentController.init()
-				.then( () => {
-					return parentController.addChild( 'x', childController1 ).then( () => {
-						return parentController.addChild( 'x', childController2, 0 );
+	describe( 'collections', () => {
+		describe( 'add', () => {
+			beforeEach( defineParentViewClass );
+			beforeEach( defineParentControllerClass );
+
+			it( 'should add a child controller which has no view', () => {
+				const parentController = new ParentController( null, new ParentView() );
+				const collection = parentController.collections.get( 'x' );
+				const childController = new Controller();
+
+				return parentController.init()
+					.then( () => {
+						return collection.add( childController );
+					} )
+					.then( () => {
+						expect( collection.get( 0 ) ).to.be.equal( childController );
 					} );
-				} )
-				.then( () => {
-					expect( parentController.view.getChild( 'x', 0 ) ).to.be.equal( childView2 );
-					expect( parentController.view.getChild( 'x', 1 ) ).to.be.equal( childView1 );
-				} );
-		} );
-
-		it( 'should initialize child controller if parent is ready', () => {
-			const parentController = new Controller( null, new ParentView() );
-			const childController = new Controller( null, new View() );
-			const spy = bender.sinon.spy( childController, 'init' );
-
-			parentController.collections.add( new ControllerCollection( 'x' ) );
-			parentController.addChild( 'x', childController );
-			parentController.removeChild( 'x', childController );
-
-			sinon.assert.notCalled( spy );
-
-			return parentController.init()
-				.then( () => {
-					return parentController.addChild( 'x', childController );
-				} )
-				.then( () => {
-					sinon.assert.calledOnce( spy );
-				} );
-		} );
-
-		it( 'should not initialize child controller twice', () => {
-			const parentController = new Controller( null, new ParentView() );
-			const childController = new Controller( null, new View() );
-			const spy = bender.sinon.spy( childController, 'init' );
-
-			parentController.collections.add( new ControllerCollection( 'x' ) );
+			} );
 
-			return parentController.init()
-				.then( () => {
-					return childController.init();
-				} )
-				.then( () => {
-					return parentController.addChild( 'x', childController );
-				} )
-				.then( () => {
-					sinon.assert.calledOnce( spy );
-				} );
-		} );
-	} );
+			it( 'should append child controller\'s view to parent controller\'s view', () => {
+				const parentView = new ParentView();
+				const parentController = new ParentController( null, parentView );
+				const collection = parentController.collections.get( 'x' );
+				const childController = new Controller( null, new View() );
+				const spy1 = bender.sinon.spy( parentView, 'addChild' );
 
-	describe( 'removeChild', () => {
-		beforeEach( defineParentViewClass );
+				collection.add( childController );
 
-		it( 'should remove child controller and return it', () => {
-			const parentController = new Controller();
-			const childController = new Controller();
-			const collection = new ControllerCollection( 'x' );
+				sinon.assert.notCalled( spy1 );
 
-			parentController.collections.add( collection );
+				collection.remove( childController );
 
-			parentController.addChild( 'x', childController );
-			const returned = parentController.removeChild( 'x', childController );
+				return parentController.init()
+					.then( () => {
+						return collection.add( childController );
+					} )
+					.then( () => {
+						sinon.assert.calledOnce( spy1 );
+						sinon.assert.calledWithExactly( spy1, 'x', childController.view, 0 );
+					} );
+			} );
 
-			expect( returned ).to.be.equal( childController );
-			expect( collection.length ).to.be.equal( 0 );
+			it( 'should append child controller\'s view to parent controller\'s view at given index', () => {
+				const parentController = new ParentController( null, new ParentView() );
+				const collection = parentController.collections.get( 'x' );
+
+				const childView1 = new View();
+				const childController1 = new Controller( null, childView1 );
+				const childView2 = new View();
+				const childController2 = new Controller( null, childView2 );
+
+				return parentController.init()
+					.then( () => {
+						return collection.add( childController1 ).then( () => {
+							return collection.add( childController2, 0 );
+						} );
+					} )
+					.then( () => {
+						expect( parentController.view.getChild( 'x', 0 ) ).to.be.equal( childView2 );
+						expect( parentController.view.getChild( 'x', 1 ) ).to.be.equal( childView1 );
+					} );
+			} );
 		} );
 
-		it( 'should remove child controller\'s view from parent controller\'s view', () => {
-			const parentView = new ParentView();
-			const parentController = new Controller( null, parentView );
-			const childController = new Controller( null, new View() );
-
-			const spy = bender.sinon.spy( parentView, 'removeChild' );
+		describe( 'remove', () => {
+			beforeEach( defineParentViewClass );
 
-			parentController.collections.add( new ControllerCollection( 'x' ) );
-			parentController.addChild( 'x', childController );
+			it( 'should remove child controller\'s view from parent controller\'s view', () => {
+				const parentView = new ParentView();
+				const parentController = new ParentController( null, parentView );
+				const collection = parentController.collections.get( 'x' );
+				const childController = new Controller( null, new View() );
+				const spy = bender.sinon.spy( parentView, 'removeChild' );
 
-			sinon.assert.notCalled( spy );
+				collection.add( childController );
 
-			return parentController.init()
-				.then( () => {
-					parentController.removeChild( 'x', childController );
-					sinon.assert.calledOnce( spy );
-					sinon.assert.calledWithExactly( spy, 'x', childController.view );
-				} );
-		} );
-	} );
+				sinon.assert.notCalled( spy );
 
-	describe( 'getChild', () => {
-		beforeEach( defineParentViewClass );
-
-		it( 'should get child controller by index', () => {
-			const parentController = new Controller();
-			const childController = new Controller();
-
-			parentController.collections.add( new ControllerCollection( 'x' ) );
-			parentController.addChild( 'x', childController );
-
-			expect( parentController.getChild( 'x', 0 ) ).to.be.equal( childController );
+				return parentController.init()
+					.then( () => {
+						collection.remove( childController );
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'x', childController.view );
+					} );
+			} );
 		} );
 	} );
 
 	describe( 'destroy', () => {
 		beforeEach( defineParentViewClass );
+		beforeEach( defineParentControllerClass );
 
 		it( 'should destroy the controller', () => {
 			const view = new View();
@@ -323,13 +229,13 @@ describe( 'Controller', () => {
 		} );
 
 		it( 'should destroy child controllers in collections with their views', () => {
-			const parentController = new Controller( null, new ParentView() );
+			const parentController = new ParentController( null, new ParentView() );
+			const collection = parentController.collections.get( 'x' );
 			const childView = new View();
 			const childController = new Controller( null, childView );
 			const spy = bender.sinon.spy( childView, 'destroy' );
 
-			parentController.collections.add( new ControllerCollection( 'x' ) );
-			parentController.addChild( 'x', childController );
+			collection.add( childController );
 
 			return parentController.init()
 				.then( () => {
@@ -344,11 +250,11 @@ describe( 'Controller', () => {
 		} );
 
 		it( 'should destroy child controllers in collections when they have no views', () => {
-			const parentController = new Controller( null, new ParentView() );
+			const parentController = new ParentController( null, new ParentView() );
+			const collection = parentController.collections.get( 'x' );
 			const childController = new Controller( null, null );
 
-			parentController.collections.add( new ControllerCollection( 'x' ) );
-			parentController.addChild( 'x', childController );
+			collection.add( childController );
 
 			return parentController.init()
 				.then( () => {
@@ -382,3 +288,13 @@ function defineParentViewClass() {
 		}
 	};
 }
+
+function defineParentControllerClass() {
+	ParentController = class extends Controller {
+		constructor( ...args ) {
+			super( ...args );
+
+			this.collections.add( new ControllerCollection( 'x' ) );
+		}
+	};
+}

+ 99 - 3
packages/ckeditor5-ui/tests/ui/controllercollection.js

@@ -7,18 +7,114 @@
 
 'use strict';
 
-const modules = bender.amd.require( 'ckeditor', 'ui/controllercollection' );
+const modules = bender.amd.require( 'ckeditor',
+	'ui/controllercollection',
+	'ui/controller',
+	'ui/view'
+);
 
 bender.tools.createSinonSandbox();
 
+let ControllerCollection, Controller, View;
+let ParentView;
+
 describe( 'ControllerCollection', () => {
+	beforeEach( updateModuleReference );
+	beforeEach( defineParentViewClass );
+
 	describe( 'constructor', () => {
 		it( 'should throw when no name is passed', () => {
-			const ControllerCollection = modules[ 'ui/controllercollection' ];
-
 			expect( () => {
 				new ControllerCollection();
 			} ).to.throw( /^ui-controllercollection-no-name/ );
 		} );
 	} );
+
+	describe( 'add', () => {
+		it( 'should add a child controller and return promise', () => {
+			const parentController = new Controller();
+			const childController = new Controller();
+			const collection = new ControllerCollection( 'x' );
+
+			parentController.collections.add( collection );
+
+			const returned = collection.add( childController );
+
+			expect( returned ).to.be.an.instanceof( Promise );
+			expect( collection.get( 0 ) ).to.be.equal( childController );
+		} );
+
+		it( 'should add a child controller at given position', () => {
+			const parentController = new Controller();
+			const childController1 = new Controller();
+			const childController2 = new Controller();
+			const collection = new ControllerCollection( 'x' );
+
+			parentController.collections.add( collection );
+
+			collection.add( childController1 );
+			collection.add( childController2, 0 );
+
+			expect( collection.get( 0 ) ).to.be.equal( childController2 );
+			expect( collection.get( 1 ) ).to.be.equal( childController1 );
+		} );
+
+		it( 'should initialize child controller if parent is ready', () => {
+			const parentController = new Controller( null, new ParentView() );
+			const childController = new Controller( null, new View() );
+			const spy = bender.sinon.spy( childController, 'init' );
+			const collection = new ControllerCollection( 'x' );
+
+			parentController.collections.add( collection );
+			collection.add( childController );
+			collection.remove( childController );
+
+			sinon.assert.notCalled( spy );
+
+			return parentController.init()
+				.then( () => {
+					return collection.add( childController );
+				} )
+				.then( () => {
+					sinon.assert.calledOnce( spy );
+				} );
+		} );
+
+		it( 'should not initialize child controller twice', () => {
+			const parentController = new Controller( null, new ParentView() );
+			const childController = new Controller( null, new View() );
+			const spy = bender.sinon.spy( childController, 'init' );
+			const collection = new ControllerCollection( 'x' );
+
+			parentController.collections.add( collection );
+
+			return parentController.init()
+				.then( () => {
+					return childController.init();
+				} )
+				.then( () => {
+					return collection.add( childController );
+				} )
+				.then( () => {
+					sinon.assert.calledOnce( spy );
+				} );
+		} );
+	} );
 } );
+
+function updateModuleReference() {
+	View = modules[ 'ui/view' ];
+	Controller = modules[ 'ui/controller' ];
+	ControllerCollection = modules[ 'ui/controllercollection' ];
+}
+
+function defineParentViewClass() {
+	ParentView = class extends View {
+		constructor() {
+			super();
+
+			this.el = document.createElement( 'span' );
+			this.register( 'x', true );
+		}
+	};
+}