8
0
Эх сурвалжийг харах

Merge pull request #12 from ckeditor/t/11

Destroy the controller's children correctly.
Piotrek Koszuliński 9 жил өмнө
parent
commit
34e34c6bcd

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

@@ -133,12 +133,13 @@ export default class Controller {
 		for ( collection of this.collections ) {
 		for ( collection of this.collections ) {
 			for ( childController of collection ) {
 			for ( childController of collection ) {
 				promises.push( childController.destroy() );
 				promises.push( childController.destroy() );
-				collection.remove( childController );
 			}
 			}
 
 
-			this.collections.remove( collection );
+			collection.clear();
 		}
 		}
 
 
+		this.collections.clear();
+
 		if ( this.view ) {
 		if ( this.view ) {
 			promises.push( Promise.resolve().then( () => {
 			promises.push( Promise.resolve().then( () => {
 				return this.view.destroy();
 				return this.view.destroy();

+ 61 - 0
packages/ckeditor5-ui/tests/controller.js

@@ -341,6 +341,67 @@ describe( 'Controller', () => {
 					expect( childController.collections ).to.be.null;
 					expect( childController.collections ).to.be.null;
 				} );
 				} );
 		} );
 		} );
+
+		// See #11
+		it( 'should correctly destroy multiple controller collections', () => {
+			const parentController = new Controller();
+			const controllerCollectionCollection = parentController.collections; // Yep... it's correct :D.
+			const childControllers = [];
+			const collections = [ 'a', 'b', 'c' ].map( name => {
+				const collection = new ControllerCollection( name );
+				const childController = new Controller();
+
+				childController.destroy = sinon.spy();
+
+				parentController.collections.add( collection );
+				collection.add( childController );
+				childControllers.push( childController );
+
+				return collection;
+			} );
+
+			return parentController.init()
+				.then( () => {
+					return parentController.destroy();
+				} )
+				.then( () => {
+					expect( controllerCollectionCollection ).to.have.lengthOf( 0, 'parentController.collections is empty' );
+					expect( collections.map( collection => collection.length ) )
+						.to.deep.equal( [ 0, 0, 0 ], 'all collections are empty' );
+					expect( childControllers.map( controller => controller.destroy.calledOnce ) )
+						.to.deep.equal( [ true, true, true ], 'all child controllers were destroyed' );
+				} );
+		} );
+
+		// See #11
+		it( 'should correctly destroy collections with multiple child controllers', () => {
+			const parentController = new Controller();
+			const controllerCollectionCollection = parentController.collections; // Yep... it's correct :D.
+			const controllerCollection = new ControllerCollection( 'foo' );
+			const childControllers = [];
+
+			parentController.collections.add( controllerCollection );
+
+			for ( let i = 0; i < 3; i++ ) {
+				const childController = new Controller();
+
+				childController.destroy = sinon.spy();
+
+				childControllers.push( childController );
+				parentController.add( 'foo', childController );
+			}
+
+			return parentController.init()
+				.then( () => {
+					return parentController.destroy();
+				} )
+				.then( () => {
+					expect( controllerCollectionCollection ).to.have.lengthOf( 0, 'parentController.collections is empty' );
+					expect( controllerCollection ).to.have.lengthOf( 0, 'child controller collection is empty' );
+					expect( childControllers.map( controller => controller.destroy.calledOnce ) )
+						.to.deep.equal( [ true, true, true ], 'all child controllers were destroyed' );
+				} );
+		} );
 	} );
 	} );
 } );
 } );
 
 

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

@@ -208,7 +208,7 @@ describe( 'View', () => {
 		} );
 		} );
 	} );
 	} );
 
 
-	describe( 'el', () => {
+	describe( 'element', () => {
 		beforeEach( createViewInstanceWithTemplate );
 		beforeEach( createViewInstanceWithTemplate );
 
 
 		it( 'invokes out of #template', () => {
 		it( 'invokes out of #template', () => {
@@ -231,6 +231,10 @@ describe( 'View', () => {
 
 
 			expect( view.element ).to.be.an.instanceof( HTMLElement );
 			expect( view.element ).to.be.an.instanceof( HTMLElement );
 		} );
 		} );
+
+		it( 'is null when there is no template', () => {
+			expect( new View().element ).to.be.null;
+		} );
 	} );
 	} );
 
 
 	describe( 'attributeBinder', () => {
 	describe( 'attributeBinder', () => {