Explorar el Código

Merge pull request #143 from ckeditor/t/118b

t/118b: Removed view#(add|remove|get)Child methods.
Piotrek Koszuliński hace 10 años
padre
commit
3e6f2953c6

+ 3 - 3
packages/ckeditor5-ui/src/ui/controller.js

@@ -64,7 +64,7 @@ export default class Controller extends Model {
 				// 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.view.regions.get( collection.name ).views.add( childController.view, index );
 				}
 			} );
 
@@ -72,7 +72,7 @@ export default class Controller extends Model {
 				// 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.view.regions.get( collection.name ).views.remove( childController.view );
 				}
 			} );
 		} );
@@ -176,7 +176,7 @@ export default class Controller extends Model {
 		for ( collection of this.collections ) {
 			for ( childController of collection ) {
 				if ( this.view && childController.view ) {
-					this.view.addChild( collection.name, childController.view );
+					this.view.regions.get( collection.name ).views.add( childController.view );
 				}
 
 				promises.push( childController.init() );

+ 2 - 108
packages/ckeditor5-ui/src/ui/view.js

@@ -119,112 +119,6 @@ export default class View extends Model {
 		this._initRegions();
 	}
 
-	/**
-	 * Adds a child view to one of the {@link #regions} (see {@link #register}) in DOM
-	 * at given, optional index position.
-	 *
-	 * @param {String} regionName One of {@link #regions} the child should be added to.
-	 * @param {View} childView A child view.
-	 * @param {Number} [index] Index at which the child will be added to the region.
-	 */
-	addChild( regionName, childView, index ) {
-		if ( !regionName ) {
-			/**
-			 * The name of the region is required.
-			 *
-			 * @error ui-view-addchild-badrname
-			 */
-			throw new CKEditorError( 'ui-view-addchild-badrname' );
-		}
-
-		const region = this.regions.get( regionName );
-
-		if ( !region ) {
-			/**
-			 * No such region of given name.
-			 *
-			 * @error ui-view-addchild-noreg
-			 */
-			throw new CKEditorError( 'ui-view-addchild-noreg' );
-		}
-
-		if ( !childView ) {
-			/**
-			 * No child view passed.
-			 *
-			 * @error ui-view-addchild-no-view
-			 */
-			throw new CKEditorError( 'ui-view-addchild-no-view' );
-		}
-
-		region.views.add( childView, index );
-	}
-
-	/**
-	 * Removes a child view from one of the {@link #regions} (see {@link #register}) in DOM.
-	 *
-	 * @param {String} regionName One of {@link #regions} the view should be removed from.
-	 * @param {View} childVIew A child view.
-	 * @returns {View} A child view instance after removal.
-	 */
-	removeChild( regionName, childView ) {
-		if ( !regionName ) {
-			/**
-			 * The name of the region is required.
-			 *
-			 * @error ui-view-removechild-badrname
-			 */
-			throw new CKEditorError( 'ui-view-removechild-badrname' );
-		}
-
-		const region = this.regions.get( regionName );
-
-		if ( !region ) {
-			/**
-			 * No such region of given name.
-			 *
-			 * @error ui-view-removechild-noreg
-			 */
-			throw new CKEditorError( 'ui-view-removechild-noreg' );
-		}
-
-		if ( !childView ) {
-			/**
-			 * The view must be an instance of View.
-			 *
-			 * @error ui-view-removechild-no-view
-			 */
-			throw new CKEditorError( 'ui-view-removechild-no-view' );
-		}
-
-		region.views.remove( childView );
-
-		return childView;
-	}
-
-	/**
-	 * Returns a child view from one of the {@link #regions}
-	 * (see {@link #register}) at given `index`.
-	 *
-	 * @param {String} regionName One of {@link #regions} the child should be retrieved from.
-	 * @param {Number} [index] An index of desired view.
-	 * @returns {View} A view instance.
-	 */
-	getChild( regionName, index ) {
-		const region = this.regions.get( regionName );
-
-		if ( !region ) {
-			/**
-			 * No such region of given name.
-			 *
-			 * @error ui-view-getchild-noreg
-			 */
-			throw new CKEditorError( 'ui-view-getchild-noreg' );
-		}
-
-		return region.views.get( index );
-	}
-
 	/**
 	 * Registers a region in {@link #regions}.
 	 *
@@ -356,8 +250,8 @@ export default class View extends Model {
 		this.stopListening();
 
 		for ( let region of this.regions ) {
-			while ( ( childView = this.getChild( region.name, 0 ) ) ) {
-				this.removeChild( region.name, childView );
+			while ( ( childView = region.views.get( 0 ) ) ) {
+				region.views.remove( childView );
 			}
 
 			this.regions.remove( region ).destroy();

+ 16 - 8
packages/ckeditor5-ui/tests/ui/controller.js

@@ -13,6 +13,7 @@ import Controller from '/ckeditor5/core/ui/controller.js';
 import ControllerCollection from '/ckeditor5/core/ui/controllercollection.js';
 import CKEditorError from '/ckeditor5/core/ckeditorerror.js';
 import Model from '/ckeditor5/core/model.js';
+import EventInfo from '/ckeditor5/core/eventinfo.js';
 
 let ParentController, ParentView;
 
@@ -120,11 +121,13 @@ describe( 'Controller', () => {
 				const parentController = new ParentController( null, parentView );
 				const collection = parentController.collections.get( 'x' );
 				const childController = new Controller( null, new View() );
-				const spy1 = testUtils.sinon.spy( parentView, 'addChild' );
+				const spy = testUtils.sinon.spy();
+
+				parentView.regions.get( 'x' ).views.on( 'add', spy );
 
 				collection.add( childController );
 
-				sinon.assert.notCalled( spy1 );
+				sinon.assert.notCalled( spy );
 
 				collection.remove( childController );
 
@@ -133,8 +136,9 @@ describe( 'Controller', () => {
 						return collection.add( childController );
 					} )
 					.then( () => {
-						sinon.assert.calledOnce( spy1 );
-						sinon.assert.calledWithExactly( spy1, 'x', childController.view, 0 );
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy,
+							sinon.match.instanceOf( EventInfo ), childController.view, 0 );
 					} );
 			} );
 
@@ -154,8 +158,10 @@ describe( 'Controller', () => {
 						} );
 					} )
 					.then( () => {
-						expect( parentController.view.getChild( 'x', 0 ) ).to.be.equal( childView2 );
-						expect( parentController.view.getChild( 'x', 1 ) ).to.be.equal( childView1 );
+						const region = parentController.view.regions.get( 'x' );
+
+						expect( region.views.get( 0 ) ).to.be.equal( childView2 );
+						expect( region.views.get( 1 ) ).to.be.equal( childView1 );
 					} );
 			} );
 		} );
@@ -168,7 +174,8 @@ describe( 'Controller', () => {
 				const parentController = new ParentController( null, parentView );
 				const collection = parentController.collections.get( 'x' );
 				const childController = new Controller( null, new View() );
-				const spy = testUtils.sinon.spy( parentView, 'removeChild' );
+				const spy = testUtils.sinon.spy();
+				parentView.regions.get( 'x' ).views.on( 'remove', spy );
 
 				collection.add( childController );
 
@@ -178,7 +185,8 @@ describe( 'Controller', () => {
 					.then( () => {
 						collection.remove( childController );
 						sinon.assert.calledOnce( spy );
-						sinon.assert.calledWithExactly( spy, 'x', childController.view );
+						sinon.assert.calledWithExactly( spy,
+							sinon.match.instanceOf( EventInfo ), childController.view );
 					} );
 			} );
 		} );

+ 1 - 131
packages/ckeditor5-ui/tests/ui/view.js

@@ -124,136 +124,6 @@ describe( 'View', () => {
 		} );
 	} );
 
-	describe( 'addChild', () => {
-		beforeEach( () => {
-			setTestViewClass(
-				() => ( { tag: 'p', text: 'x' } ),
-				function() {
-					this.register( 'x', el => el );
-				}
-			);
-
-			setTestViewInstance();
-		} );
-
-		it( 'should throw when no region name', () => {
-			expect( () => {
-				view.addChild();
-			} ).to.throw( CKEditorError, /ui-view-addchild-badrname/ );
-		} );
-
-		it( 'should throw when region does not exist', () => {
-			expect( () => {
-				view.addChild( 'nonexistent' );
-			} ).to.throw( CKEditorError, /ui-view-addchild-noreg/ );
-		} );
-
-		it( 'should throw when no child view passed', () => {
-			expect( () => {
-				view.addChild( 'x' );
-			} ).to.throw( CKEditorError, /ui-view-addchild-no-view/ );
-		} );
-
-		it( 'should add a child to the region views', () => {
-			const childView = new View();
-
-			view.addChild( 'x', childView );
-
-			expect( view.getChild( 'x', 0 ) ).to.be.equal( childView );
-		} );
-
-		it( 'should add a child to the region views at the right index', () => {
-			const childView1 = new View();
-			const childView2 = new View();
-
-			view.addChild( 'x', childView1 );
-			view.addChild( 'x', childView2, 0 );
-
-			expect( view.getChild( 'x', 0 ) ).to.be.equal( childView2 );
-			expect( view.getChild( 'x', 1 ) ).to.be.equal( childView1 );
-		} );
-	} );
-
-	describe( 'removeChild', () => {
-		beforeEach( () => {
-			setTestViewClass(
-				() => ( { tag: 'p', text: 'x' } ),
-				function() {
-					this.register( 'x', el => el );
-				}
-			);
-
-			setTestViewInstance();
-		} );
-
-		it( 'should throw when no region name', () => {
-			expect( () => {
-				view.removeChild();
-			} ).to.throw( CKEditorError, /ui-view-removechild-badrname/ );
-		} );
-
-		it( 'should throw when child view passed', () => {
-			expect( () => {
-				view.removeChild( 'x' );
-			} ).to.throw( CKEditorError, /ui-view-removechild-no-view/ );
-		} );
-
-		it( 'should throw when region does not exist', () => {
-			expect( () => {
-				view.removeChild( 'nonexistent', new View() );
-			} ).to.throw( CKEditorError, /ui-view-removechild-noreg/ );
-		} );
-
-		it( 'should remove a child from the region views', () => {
-			const childView1 = new View();
-			const childView2 = new View();
-
-			view.addChild( 'x', childView1 );
-			view.addChild( 'x', childView2, 0 );
-
-			expect( view.getChild( 'x', 0 ) ).to.be.equal( childView2 );
-			expect( view.getChild( 'x', 1 ) ).to.be.equal( childView1 );
-
-			view.removeChild( 'x', childView2 );
-
-			expect( view.getChild( 'x', 0 ) ).to.be.equal( childView1 );
-
-			view.removeChild( 'x', childView1 );
-
-			expect( view.regions.get( 'x' ).views.length ).to.be.equal( 0 );
-		} );
-	} );
-
-	describe( 'getChild', () => {
-		beforeEach( () => {
-			setTestViewClass(
-				() => ( { tag: 'p', text: 'x' } ),
-				function() {
-					this.register( 'x', el => el );
-				}
-			);
-
-			setTestViewInstance();
-		} );
-
-		it( 'should throw when region does not exist', () => {
-			expect( () => {
-				view.getChild( 'nonexistent', 0 );
-			} ).to.throw( CKEditorError, /ui-view-getchild-noreg/ );
-		} );
-
-		it( 'should get a child from the region views', () => {
-			const childView1 = new View();
-			const childView2 = new View();
-
-			view.addChild( 'x', childView1 );
-			view.addChild( 'x', childView2, 0 );
-
-			expect( view.getChild( 'x', 0 ) ).to.be.equal( childView2 );
-			expect( view.getChild( 'x', 1 ) ).to.be.equal( childView1 );
-		} );
-	} );
-
 	describe( 'register', () => {
 		beforeEach( () => {
 			setTestViewClass();
@@ -711,7 +581,7 @@ describe( 'View', () => {
 			const regionViewsRef = region.views;
 
 			view.register( region, true );
-			view.addChild( 'x', new View() );
+			view.regions.get( 'x' ).views.add( new View() );
 			view.destroy();
 
 			expect( regionsRef.length ).to.be.equal( 0 );