浏览代码

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

t/117b: Allowed Template instance as a child of another template
Piotr Jasiun 9 年之前
父节点
当前提交
40fb39f5dd
共有 2 个文件被更改,包括 85 次插入2 次删除
  1. 14 2
      packages/ckeditor5-ui/src/template.js
  2. 71 0
      packages/ckeditor5-ui/tests/template.js

+ 14 - 2
packages/ckeditor5-ui/src/template.js

@@ -784,10 +784,13 @@ function clone( def ) {
 		// cloneDeepWith algorithm. There's no point in cloning Observable/DomEmitterMixins
 		// along with the definition.
 		//
+		// Don't clone Template instances if provided as a child. They're simply #render()ed
+		// and nothing should interfere.
+		//
 		// Also don't clone View instances if provided as a child of the Template. The template
 		// instance will be extracted from the View during the normalization and there's no need
 		// to clone it.
-		if ( value && ( value instanceof TemplateBinding || isView( value ) || isViewCollection( value ) ) ) {
+		if ( value && ( value instanceof TemplateBinding || isTemplate( value ) || isView( value ) || isViewCollection( value ) ) ) {
 			return value;
 		}
 	} );
@@ -831,7 +834,7 @@ function normalize( def ) {
 				children.add( def.children );
 			} else {
 				for ( let child of def.children ) {
-					if ( isView( child ) ) {
+					if ( isTemplate( child ) || isView( child ) ) {
 						children.add( child );
 					} else {
 						children.add( new Template( child ) );
@@ -1063,6 +1066,14 @@ function isView( item ) {
 	return item instanceof View;
 }
 
+// Checks if the item is an instance of {@link module:ui/template~Template}
+//
+// @private
+// @param {*} value Value to be checked.
+function isTemplate( item ) {
+	return item instanceof Template;
+}
+
 // Checks if the item is an instance of {@link module:ui/viewcollection~ViewCollection}
 //
 // @private
@@ -1089,6 +1100,7 @@ function isViewCollection( item ) {
  *				},
  *				'also-static–text',
  *				<{@link module:ui/view~View} instance>
+ *				<{@link module:ui/template~Template} instance>
  *				...
  *			],
  *			attributes: {

+ 71 - 0
packages/ckeditor5-ui/tests/template.js

@@ -483,6 +483,77 @@ describe( 'Template', () => {
 
 				expect( collection._parentElement ).to.equal( rendered );
 			} );
+
+			// #117
+			it( 'renders template children', () => {
+				const childTplA = new Template( {
+					tag: 'a'
+				} );
+
+				const childTplB = new Template( {
+					tag: 'b'
+				} );
+
+				const view = new View();
+
+				view.set( {
+					foo: 'bar',
+					bar: 'foo'
+				} );
+
+				const bind = Template.bind( view, view );
+
+				// Yes, this TC is crazy in some sort of way. It's all about template
+				// normalization and deep cloning and the like.
+				//
+				// To **really** prove the code is safe there must be a View instance,
+				// which has some child bound to its attribute and... there must be a
+				// Template instance (below), which also has a child bound to the same attribute.
+				//
+				// I know, the view instance and its template aren't even rendered.
+				//
+				// The truth is that madness behind this test case is so deep there are no
+				// words to explain it. But what actually matters is that it proves the Template
+				// class is free of "Maximum call stack size exceeded" error in certain
+				// situations.
+				view.template = new Template( {
+					tag: 'span',
+
+					children: [
+						{
+							text: bind.to( 'bar' )
+						}
+					]
+				} );
+
+				const childTplC = new Template( {
+					tag: 'i',
+
+					children: [
+						{
+							text: bind.to( 'bar' )
+						}
+					]
+				} );
+
+				const tpl = new Template( {
+					tag: 'p',
+					children: [
+						childTplA,
+						childTplB,
+						childTplC
+					]
+				} );
+
+				// Make sure child instances weren't cloned.
+				expect( tpl.children.get( 0 ) ).to.equal( childTplA );
+				expect( tpl.children.get( 1 ) ).to.equal( childTplB );
+				expect( tpl.children.get( 2 ) ).to.equal( childTplC );
+
+				expect( normalizeHtml( tpl.render().outerHTML ) ).to.equal(
+					'<p><a></a><b></b><i>foo</i></p>'
+				);
+			} );
 		} );
 
 		describe( 'bindings', () => {