瀏覽代碼

Merge branch 'master' into t/138

Piotrek Koszuliński 9 年之前
父節點
當前提交
b426959332

+ 13 - 10
packages/ckeditor5-ui/src/dropdown/dropdownview.js

@@ -24,13 +24,24 @@ export default class DropdownView extends View {
 	constructor( locale, buttonView, panelView ) {
 		super( locale );
 
+		// Extend button's template before it's registered as a child of the dropdown because
+		// by doing so, its #element is rendered and any post–render template extension will
+		// not be reflected in DOM.
+		Template.extend( buttonView.template, {
+			attributes: {
+				class: [
+					'ck-dropdown__button'
+				]
+			}
+		} );
+
 		/**
 		 * Button of this dropdown view.
 		 *
 		 * @readonly
 		 * @member {ui.button.ButtonView} #buttonView
 		 */
-		this.addChildren( this.buttonView = buttonView );
+		this.buttonView = buttonView;
 
 		/**
 		 * Panel of this dropdown view.
@@ -38,7 +49,7 @@ export default class DropdownView extends View {
 		 * @readonly
 		 * @member {module:ui/dropdown/dropdownpanelview~DropdownPanelView} #panelView
 		 */
-		this.addChildren( this.panelView = panelView );
+		this.panelView = panelView;
 
 		/**
 		 * Controls whether the dropdown view is open, which also means its
@@ -80,14 +91,6 @@ export default class DropdownView extends View {
 			]
 		} );
 
-		Template.extend( buttonView.template, {
-			attributes: {
-				class: [
-					'ck-dropdown__button'
-				]
-			}
-		} );
-
 		// Toggle the the dropdown when it's button has been clicked.
 		this.listenTo( buttonView, 'execute', () => this.isOpen = !this.isOpen );
 

+ 2 - 2
packages/ckeditor5-ui/src/labeledinput/labeledinputview.js

@@ -51,14 +51,14 @@ export default class LabeledInputView extends View {
 		 *
 		 * @member {module:ui/label/labelview~LabelView} #labelView
 		 */
-		this.addChildren( this.labelView = this._createLabelView( id ) );
+		this.labelView = this._createLabelView( id );
 
 		/**
 		 * The input view.
 		 *
 		 * @member {module:ui/view~View} #inputView
 		 */
-		this.addChildren( this.inputView = this._createInputView( InputView, id ) );
+		this.inputView = this._createInputView( InputView, id );
 
 		this.template = new Template( {
 			tag: 'div',

+ 53 - 12
packages/ckeditor5-ui/src/view.js

@@ -145,6 +145,8 @@ export default class View {
 			return null;
 		}
 
+		this._addTemplateChildren();
+
 		return ( this._element = this.template.render() );
 	}
 
@@ -223,22 +225,17 @@ export default class View {
 	 *				this.childA = new SomeChildView( locale );
 	 *				this.childB = new SomeChildView( locale );
 	 *
+	 *				this.template = new Template( { tag: 'p' } );
+	 *
 	 *				// Register children.
 	 *				this.addChildren( [ this.childA, this.childB ] );
+	 *			}
 	 *
-	 *				this.template = new Template( {
-	 *					tag: 'p',
-	 *
-	 *					children: [
-	 *						// This is where the `childA` will render.
-	 *						this.childA,
-	 *
-	 *						{ tag: 'b' },
+	 *			init() {
+	 *				this.element.appendChild( this.childA.element );
+	 *				this.element.appendChild( this.childB.element );
 	 *
-	 *						// This is where the `childB` will render.
-	 *						this.childB
-	 *					]
-	 *				} );
+	 *				return super.init();
 	 *			}
 	 *		}
 	 *
@@ -249,6 +246,28 @@ export default class View {
 	 *			document.body.appendChild( view.element );
 	 *		} );
 	 *
+	 * **Note**: There's no need to add child views if they're used in the
+	 * {@link #template} explicitly:
+	 *
+	 *		class SampleView extends View {
+	 *			constructor( locale ) {
+	 *				super( locale );
+	 *
+	 *				this.childA = new SomeChildView( locale );
+	 *				this.childB = new SomeChildView( locale );
+	 *
+	 *				this.template = new Template( {
+	 *					tag: 'p',
+	 *
+ 	 *					// These children will be added automatically. There's no
+ 	 *					// need to call {@link #addChildren} for any of them.
+	 *					children: [ this.childA, this.childB ]
+	 *				} );
+	 *			}
+	 *
+	 *			...
+	 *		}
+	 *
 	 * @param {module:ui/view~View|Iterable.<module:ui/view~View>} children Children views to be registered.
 	 */
 	addChildren( children ) {
@@ -309,6 +328,28 @@ export default class View {
 
 		return Promise.all( promises );
 	}
+
+	/**
+	 * Recursively traverses {@link #template} in search of {@link module:ui/view~View}
+	 * instances and automatically registers them using {@link #addChildren} method.
+	 *
+	 * @protected
+	 */
+	_addTemplateChildren() {
+		const search = ( def ) => {
+			if ( def.children ) {
+				for ( let defOrView of def.children ) {
+					if ( defOrView instanceof View ) {
+						this.addChildren( defOrView );
+					} else {
+						search( defOrView );
+					}
+				}
+			}
+		};
+
+		search( this.template );
+	}
 }
 
 mix( View, DomEmmiterMixin );

+ 6 - 17
packages/ckeditor5-ui/src/viewcollection.js

@@ -34,14 +34,14 @@ export default class ViewCollection extends Collection {
 
 		// Handle {@link module:ui/view~View#element} in DOM when a new view is added to the collection.
 		this.on( 'add', ( evt, view, index ) => {
-			if ( this.ready && view.element && this._parentElement ) {
+			if ( view.element && this._parentElement ) {
 				this._parentElement.insertBefore( view.element, this._parentElement.children[ index ] );
 			}
 		} );
 
 		// Handle {@link module:ui/view~View#element} in DOM when a view is removed from the collection.
 		this.on( 'remove', ( evt, view ) => {
-			if ( this.ready && view.element && this._parentElement ) {
+			if ( view.element && this._parentElement ) {
 				view.element.remove();
 			}
 		} );
@@ -97,21 +97,10 @@ export default class ViewCollection extends Collection {
 			throw new CKEditorError( 'ui-viewcollection-init-reinit: This ViewCollection has already been initialized.' );
 		}
 
-		const promises = [];
-
-		for ( let view of this ) {
-			// Do not render unbound children. They're already in DOM by explicit declaration
-			// in Template definition.
-			if ( this._parentElement && view.element ) {
-				this._parentElement.appendChild( view.element );
-			}
-
-			promises.push( view.init() );
-		}
-
-		return Promise.all( promises ).then( () => {
-			this.ready = true;
-		} );
+		return Promise.all( this.map( v => v.init() ) )
+			.then( () => {
+				this.ready = true;
+			} );
 	}
 
 	/**

+ 42 - 6
packages/ckeditor5-ui/tests/view.js

@@ -61,7 +61,7 @@ describe( 'View', () => {
 		} );
 	} );
 
-	describe( 'createCollection', () => {
+	describe( 'createCollection()', () => {
 		beforeEach( () => {
 			setTestViewClass();
 			setTestViewInstance();
@@ -81,7 +81,7 @@ describe( 'View', () => {
 		} );
 	} );
 
-	describe( 'addChildren', () => {
+	describe( 'addChildren()', () => {
 		beforeEach( () => {
 			setTestViewClass();
 			setTestViewInstance();
@@ -192,6 +192,44 @@ describe( 'View', () => {
 		it( 'is null when there is no template', () => {
 			expect( new View().element ).to.be.null;
 		} );
+
+		it( 'registers child views found in the template', () => {
+			const view = new View();
+			const viewA = new View();
+			const viewB = new View();
+			const viewC = new View();
+
+			viewA.template = new Template( { tag: 'a' } );
+			viewB.template = new Template( { tag: 'b' } );
+			viewC.template = new Template( { tag: 'c' } );
+
+			view.template = new Template( {
+				tag: 'div',
+				children: [
+					viewA,
+					viewB,
+					{
+						tag: 'p',
+						children: [
+							viewC
+						]
+					},
+					{
+						text: 'foo'
+					}
+				]
+			} );
+
+			expect( view._unboundChildren ).to.have.length( 0 );
+
+			// Render the view.
+			view.element;
+
+			expect( view._unboundChildren ).to.have.length( 3 );
+			expect( view._unboundChildren.get( 0 ) ).to.equal( viewA );
+			expect( view._unboundChildren.get( 1 ) ).to.equal( viewB );
+			expect( view._unboundChildren.get( 2 ) ).to.equal( viewC );
+		} );
 	} );
 
 	describe( 'destroy()', () => {
@@ -216,8 +254,8 @@ describe( 'View', () => {
 		it( 'clears #_unboundChildren', () => {
 			const cached = view._unboundChildren;
 
-			view.addChildren( new View(), new View() );
-			expect( cached ).to.have.length.above( 1 );
+			view.addChildren( [ new View(), new View() ] );
+			expect( cached ).to.have.length.above( 2 );
 
 			return view.destroy().then( () => {
 				expect( cached ).to.have.length( 0 );
@@ -350,6 +388,4 @@ function createViewWithChildren() {
 	} );
 
 	setTestViewInstance();
-
-	view.addChildren( childA, childB );
 }

+ 123 - 24
packages/ckeditor5-ui/tests/viewcollection.js

@@ -3,7 +3,7 @@
  * For licensing, see LICENSE.md.
  */
 
-/* global document */
+/* global document, setTimeout */
 
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import Collection from '@ckeditor/ckeditor5-utils/src/collection';
@@ -35,39 +35,90 @@ describe( 'ViewCollection', () => {
 			expect( new ViewCollection( locale ).locale ).to.equal( locale );
 		} );
 
-		it( 'manages view#element of the children in DOM', () => {
-			const parentElement = document.createElement( 'p' );
-			collection.setParent( parentElement );
+		describe( 'child view management in DOM', () => {
+			it( 'manages view#element of the children in DOM', () => {
+				const parentElement = document.createElement( 'p' );
+				collection.setParent( parentElement );
 
-			const viewA = new View();
+				const viewA = new View();
 
-			expect( () => {
-				collection.add( viewA );
-				collection.remove( viewA );
-			} ).to.not.throw();
+				expect( () => {
+					collection.add( viewA );
+					collection.remove( viewA );
+				} ).to.not.throw();
 
-			expect( () => {
-				collection.ready = true;
+				expect( () => {
+					collection.ready = true;
+					collection.add( viewA );
+					collection.remove( viewA );
+				} ).to.not.throw();
+
+				viewA.element = document.createElement( 'b' );
 				collection.add( viewA );
+
+				expect( normalizeHtml( parentElement.outerHTML ) ).to.equal( '<p><b></b></p>' );
+
+				const viewB = new View();
+				viewB.element = document.createElement( 'i' );
+
+				collection.add( viewB, 0 );
+				expect( normalizeHtml( parentElement.outerHTML ) ).to.equal( '<p><i></i><b></b></p>' );
+
 				collection.remove( viewA );
-			} ).to.not.throw();
+				expect( normalizeHtml( parentElement.outerHTML ) ).to.equal( '<p><i></i></p>' );
 
-			viewA.element = document.createElement( 'b' );
-			collection.add( viewA );
+				collection.remove( viewB );
+				expect( normalizeHtml( parentElement.outerHTML ) ).to.equal( '<p></p>' );
+			} );
 
-			expect( normalizeHtml( parentElement.outerHTML ) ).to.equal( '<p><b></b></p>' );
+			// #145
+			it( 'always keeps the collection synchronized with DOM', () => {
+				const view = new View();
+				const viewA = getView( 'A' );
+				const viewB = getView( 'B' );
+
+				function getView( text ) {
+					const view = new View();
+
+					view.template = new Template( {
+						tag: 'li',
+						children: [
+							{
+								text: text
+							}
+						]
+					} );
 
-			const viewB = new View();
-			viewB.element = document.createElement( 'i' );
+					return view;
+				}
 
-			collection.add( viewB, 0 );
-			expect( normalizeHtml( parentElement.outerHTML ) ).to.equal( '<p><i></i><b></b></p>' );
+				// Fill the collection with children.
+				collection.add( viewA );
+				collection.add( viewB );
+
+				// Put the collection in the template.
+				view.template = new Template( {
+					tag: 'ul',
+					children: collection
+				} );
 
-			collection.remove( viewA );
-			expect( normalizeHtml( parentElement.outerHTML ) ).to.equal( '<p><i></i></p>' );
+				// Render view#template along with collection of children.
+				view.element;
 
-			collection.remove( viewB );
-			expect( normalizeHtml( parentElement.outerHTML ) ).to.equal( '<p></p>' );
+				const viewC = getView( 'C' );
+
+				// Modify the collection, while the view#element has already
+				// been rendered but the view has **not** been inited yet.
+				collection.add( viewC );
+				collection.remove( 1 );
+
+				// Finally init the view. Check what's in there.
+				return view.init().then( () => {
+					expect( view.element.childNodes ).to.have.length( 2 );
+					expect( view.element.childNodes[ 0 ] ).to.equal( viewA.element );
+					expect( view.element.childNodes[ 1 ] ).to.equal( viewC.element );
+				} );
+			} );
 		} );
 	} );
 
@@ -76,6 +127,54 @@ describe( 'ViewCollection', () => {
 			expect( collection.init() ).to.be.instanceof( Promise );
 		} );
 
+		it( 'should return a composition of child init() promises', () => {
+			const spyA = sinon.spy().named( 'A' );
+			const spyB = sinon.spy().named( 'B' );
+			const spyC = sinon.spy().named( 'C' );
+
+			const childA = {
+				init: () => {
+					return new Promise( ( resolve ) => {
+						setTimeout( () => {
+							spyA();
+							resolve();
+						}, 20 );
+					} );
+				}
+			};
+
+			const childB = {
+				init: () => {
+					return new Promise( ( resolve ) => {
+						setTimeout( () => {
+							spyB();
+							resolve();
+						}, 10 );
+					} );
+				}
+			};
+
+			const childC = {
+				init: () => {
+					return new Promise( ( resolve ) => {
+						setTimeout( () => {
+							spyC();
+							resolve();
+						}, 0 );
+					} );
+				}
+			};
+
+			collection.add( childA );
+			collection.add( childB );
+			collection.add( childC );
+
+			return collection.init()
+				.then( () => {
+					sinon.assert.callOrder( spyC, spyB, spyA );
+				} );
+		} );
+
 		it( 'should throw if already initialized', () => {
 			return collection.init()
 				.then( () => {
@@ -182,7 +281,7 @@ describe( 'ViewCollection', () => {
 		} );
 	} );
 
-	describe( 'setParent', () => {
+	describe( 'setParent()', () => {
 		it( 'sets #_parentElement', () => {
 			const el = {};
 			expect( collection._parentElement ).to.be.null;