Browse Source

Code refactoring: Cleared up how template bindings are created and discovered.

Aleksander Nowodzinski 9 years ago
parent
commit
504cf0f0e6
2 changed files with 168 additions and 72 deletions
  1. 156 63
      packages/ckeditor5-ui/src/template.js
  2. 12 9
      packages/ckeditor5-ui/tests/template.js

+ 156 - 63
packages/ckeditor5-ui/src/template.js

@@ -13,8 +13,6 @@ import View from './view.js';
 import cloneDeepWith from '../utils/lib/lodash/cloneDeepWith.js';
 import isObject from '../utils/lib/lodash/isObject.js';
 
-const bindToSymbol = Symbol( 'bindTo' );
-const bindIfSymbol = Symbol( 'bindIf' );
 const xhtmlNs = 'http://www.w3.org/1999/xhtml';
 
 /**
@@ -202,12 +200,11 @@ export default class Template {
 			 * @return {ui.TemplateBinding}
 			 */
 			to( eventNameOrFunctionOrAttribute, callback ) {
-				return {
-					type: bindToSymbol,
+				return new TemplateToBinding( {
 					eventNameOrFunction: eventNameOrFunctionOrAttribute,
 					attribute: eventNameOrFunctionOrAttribute,
 					observable, emitter, callback
-				};
+				} );
 			},
 
 			/**
@@ -242,10 +239,9 @@ export default class Template {
 			 * @return {ui.TemplateBinding}
 			 */
 			if( attribute, valueIfTrue, callback ) {
-				return {
-					type: bindIfSymbol,
+				return new TemplateIfBinding( {
 					observable, emitter, attribute, valueIfTrue, callback
-				};
+				} );
 			}
 		};
 	}
@@ -382,7 +378,7 @@ export default class Template {
 		// Check if this Text Node is bound to Observable. Cases:
 		//		{ text: [ Template.bind( ... ).to( ... ) ] }
 		//		{ text: [ 'foo', Template.bind( ... ).to( ... ), ... ] }
-		if ( hasBinding( this.text ) ) {
+		if ( hasTemplateBinding( this.text ) ) {
 			this._bindToObservable( this.text, textNode, getTextUpdater( textNode ) );
 		}
 
@@ -420,7 +416,7 @@ export default class Template {
 			// 		{ class: [ Template.bind( ... ).to( ... ) ] }
 			// 		{ class: [ 'bar', Template.bind( ... ).to( ... ), 'baz' ] }
 			// 		{ class: { ns: 'abc', value: Template.bind( ... ).to( ... ) } }
-			if ( hasBinding( attrValue ) ) {
+			if ( hasTemplateBinding( attrValue ) ) {
 				this._bindToObservable(
 					// Normalize attributes with additional data like namespace:
 					//		{ class: { ns: 'abc', value: [ ... ] } }
@@ -492,7 +488,7 @@ export default class Template {
 			// style: {
 			//	color: bind.to( 'attribute' )
 			// }
-			if ( hasBinding( styleValue ) ) {
+			if ( hasTemplateBinding( styleValue ) ) {
 				this._bindToObservable( [ styleValue ], el, getStyleUpdater( el, styleName ) );
 			}
 
@@ -538,17 +534,9 @@ export default class Template {
 		for ( let key in this.eventListeners ) {
 			const [ domEvtName, domSelector ] = key.split( '@' );
 
-			for ( let schemaItem of this.eventListeners[ key ] ) {
-				schemaItem.emitter.listenTo( el, domEvtName, ( evt, domEvt ) => {
-					if ( !domSelector || domEvt.target.matches( domSelector ) ) {
-						if ( typeof schemaItem.eventNameOrFunction == 'function' ) {
-							schemaItem.eventNameOrFunction( domEvt );
-						} else {
-							schemaItem.observable.fire( schemaItem.eventNameOrFunction, domEvt );
-						}
-					}
-				} );
-			}
+			this.eventListeners[ key ].forEach( schemaItem => {
+				schemaItem.activateDomEventListener( el, domEvtName, domSelector );
+			} );
 		}
 	}
 
@@ -572,25 +560,155 @@ export default class Template {
 			// Once only the actual binding are left, let the emitter listen to observable change:attribute event.
 			// TODO: Reduce the number of listeners attached as many bindings may listen
 			// to the same observable attribute.
-			.forEach( ( { emitter, observable, attribute } ) => {
-				emitter.listenTo( observable, 'change:' + attribute, () => {
-					syncBinding( ...arguments );
-				} );
-			} );
+			.forEach( templateBinding => templateBinding.activateAttributeListener( ...arguments ) );
 
 		// Set initial values.
-		syncBinding( ...arguments );
+		syncValueSchemaValue( ...arguments );
 	}
 }
 
 mix( Template, EmitterMixin );
 
+/**
+ * Describes a binding created by {@link ui.Template#bind} interface.
+ *
+ * @protected
+ * @memberOf ui
+ */
+export class TemplateBinding {
+	/**
+	 * Creates an instance of the {@link ui.TemplateBinding} class.
+	 *
+	 * @param {ui.TemplateBindingDefinition} def The definition of the binding.
+	 */
+	constructor( def ) {
+		Object.assign( this, def );
+
+		/**
+		 * An observable instance of the binding. It provides the attribute
+		 * with the value or passes the event when a corresponding DOM event is fired.
+		 *
+		 * @member {utils.ObservableMixin} ui.TemplateBinding#observable
+		 */
+
+		/**
+		 * An {@link utils.EmitterMixin} instance used by the binding
+		 * to (either):
+		 *
+		 * * Listen to the attribute change in the {@link ui.TemplateBinding#observable}.
+		 * * Listen to the event in the DOM.
+		 *
+		 * @member {utils.EmitterMixin} ui.TemplateBinding#emitter
+		 */
+
+		/**
+		 * The name of the attribute of {@link ui.TemplateBinding#observable} which is observed.
+		 *
+		 * @member {String} ui.TemplateBinding#attribute
+		 */
+
+		/**
+		 * A custom function to process the value of {@link ui.TemplateBinding#attribute}.
+		 *
+		 * @member {Function} [ui.TemplateBinding#callback]
+		 */
+	}
+
+	/**
+	 * Returns the value of the binding, which is the value of {@link ui.TemplateBinding#attribute} in
+	 * {@link ui.TemplateBinding#observable}.
+	 *
+	 * @param {Node} [node] A native DOM node, passed to the custom {@link ui.TemplateBinding#callback}.
+	 * @returns {*} The value of {@link ui.TemplateBinding#attribute} in {@link ui.TemplateBinding#observable}.
+	 */
+	getValue( domNode ) {
+		const value = this.observable[ this.attribute ];
+
+		return this.callback ? this.callback( value, domNode ) : value;
+	}
+
+	/**
+	 * Activates the listener for the changes of {@link ui.TemplateBinding#attribute} in
+	 * {@link ui.TemplateBinding#observable}, which then updates the DOM with the aggregated
+	 * value of {@link ui.TemplateValueSchema}.
+	 *
+	 * For instance, the `class` attribute of the `Template` element can be be bound to
+	 * the observable `foo` attribute in `ObservableMixin` instance.
+	 *
+	 * @param {ui.TemplateValueSchema} valueSchema A full schema to generate an attribute or text in DOM.
+	 * @param {Node} node A native DOM node, which attribute or text is to be updated.
+	 * @param {Function} updater A DOM updater function used to update native DOM attribute or text.
+	 */
+	activateAttributeListener( valueSchema, node, updater ) {
+		this.emitter.listenTo( this.observable, 'change:' + this.attribute, () => {
+			syncValueSchemaValue( valueSchema, node, updater );
+		} );
+	}
+}
+
+/**
+ * Describes either
+ *
+ * * A binding to {@link utils.ObservableMixin},
+ * * A native DOM event binding,
+ *
+ * created by {@link ui.Template.bind#to} method.
+ *
+ * @protected
+ * @memberOf ui
+ */
+export class TemplateToBinding extends TemplateBinding {
+	/**
+	 * Activates the listener for the native DOM event, which when fired, is propagated by
+	 * the {@link ui.TemplateBinding#emitter}.
+	 *
+	 * @param {HTMLElement} element An element on which listening to the native DOM event.
+	 * @param {String} domEvtName A name of the native DOM event.
+	 * @param {String} [domSelector] A selector in DOM to filter delegated events.
+	 */
+	activateDomEventListener( el, domEvtName, domSelector ) {
+		this.emitter.listenTo( el, domEvtName, ( evt, domEvt ) => {
+			if ( !domSelector || domEvt.target.matches( domSelector ) ) {
+				if ( typeof this.eventNameOrFunction == 'function' ) {
+					this.eventNameOrFunction( domEvt );
+				} else {
+					this.observable.fire( this.eventNameOrFunction, domEvt );
+				}
+			}
+		} );
+	}
+}
+
+/**
+ * Describes a binding to {@link utils.ObservableMixin} created by {@link ui.Template.bind#if} method.
+ *
+ * @protected
+ * @memberOf ui
+ */
+export class TemplateIfBinding extends TemplateBinding {
+	/**
+	 * @inheritDoc
+	 */
+	getValue( domNode ) {
+		const value = super.getValue( domNode );
+
+		return isFalsy( value ) ? false : ( this.valueIfTrue || true );
+	}
+
+	/**
+	 * The value of the DOM attribute/text to be set if the {@link ui.TemplateBinding#attribute} in
+	 * {@link ui.TemplateBinding#observable} is `true`.
+	 *
+	 * @member {String} [ui.TemplateIfBinding#valueIfTrue]
+	 */
+}
+
 // Checks whether given {@link ui.TemplateValueSchema} contains a
 // {@link ui.TemplateBinding}.
 //
 // @param {ui.TemplateValueSchema} valueSchema
 // @returns {Boolean}
-function hasBinding( valueSchema ) {
+function hasTemplateBinding( valueSchema ) {
 	if ( !valueSchema ) {
 		return false;
 	}
@@ -602,8 +720,8 @@ function hasBinding( valueSchema ) {
 	}
 
 	if ( Array.isArray( valueSchema ) ) {
-		return valueSchema.some( hasBinding );
-	} else if ( valueSchema.observable ) {
+		return valueSchema.some( hasTemplateBinding );
+	} else if ( valueSchema instanceof TemplateBinding ) {
 		return true;
 	}
 
@@ -617,23 +735,11 @@ function hasBinding( valueSchema ) {
 // @param {ui.TemplateValueSchema} valueSchema
 // @param {Node} node DOM Node updated when {@link utils.ObservableMixin} changes.
 // @return {Array}
-function getBindingValue( valueSchema, domNode ) {
+function getValueSchemaValue( valueSchema, domNode ) {
 	return valueSchema.map( schemaItem => {
 		// Process {@link ui.TemplateBinding} bindings.
-		if ( isObject( schemaItem ) ) {
-			let { observable, callback, type } = schemaItem;
-			let modelValue = observable[ schemaItem.attribute ];
-
-			// Process the value with the callback.
-			if ( callback ) {
-				modelValue = callback( modelValue, domNode );
-			}
-
-			if ( type === bindIfSymbol ) {
-				return isFalsy( modelValue ) ? false : ( schemaItem.valueIfTrue || true );
-			} else {
-				return modelValue;
-			}
+		if ( schemaItem instanceof TemplateBinding ) {
+			return schemaItem.getValue( domNode );
 		}
 
 		// All static values like strings, numbers, and "falsy" values (false, null, undefined, '', etc.) just pass.
@@ -647,12 +753,12 @@ function getBindingValue( valueSchema, domNode ) {
 // @param {ui.TemplateValueSchema} valueSchema
 // @param {Node} node DOM Node updated when {@link utils.ObservableMixin} changes.
 // @param {Function} domUpdater A function which updates DOM (like attribute or text).
-function syncBinding( valueSchema, domNode, domUpdater ) {
-	let value = getBindingValue( valueSchema, domNode );
+function syncValueSchemaValue( valueSchema, domNode, domUpdater ) {
+	let value = getValueSchemaValue( valueSchema, domNode );
 
 	// Check if valueSchema is a single Template.bind.if, like:
 	//		{ class: Template.bind.if( 'foo' ) }
-	if ( valueSchema.length == 1 && valueSchema[ 0 ].type == bindIfSymbol ) {
+	if ( valueSchema.length == 1 && valueSchema[ 0 ] instanceof TemplateIfBinding ) {
 		value = value[ 0 ];
 	} else {
 		value = value.reduce( arrayValueReducer, '' );
@@ -736,7 +842,7 @@ function clone( def ) {
 		// 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.type || value instanceof View ) ) {
+		if ( value && ( value instanceof TemplateBinding || value instanceof View ) ) {
 			return value;
 		}
 	} );
@@ -1115,16 +1221,3 @@ function isFalsy( value ) {
  * @typedef ui.TemplateListenerSchema
  * @type {Object|String|Array}
  */
-
-/**
- * Describes Model binding created via {@link ui.Template#bind}.
- *
- * @typedef ui.TemplateBinding
- * @type Object
- * @property {utils.ObservableMixin} observable
- * @property {utils.EmitterMixin} emitter
- * @property {Symbol} type
- * @property {String} attribute
- * @property {String} [valueIfTrue]
- * @property {Function} [callback]
- */

+ 12 - 9
packages/ckeditor5-ui/tests/template.js

@@ -8,6 +8,7 @@
 
 import testUtils from '/tests/core/_utils/utils.js';
 import Template from '/ckeditor5/ui/template.js';
+import { TemplateToBinding, TemplateIfBinding } from '/ckeditor5/ui/template.js';
 import View from '/ckeditor5/ui/view.js';
 import Model from '/ckeditor5/ui/model.js';
 import CKEditorError from '/ckeditor5/utils/ckeditorerror.js';
@@ -59,19 +60,19 @@ describe( 'Template', () => {
 			expect( tpl.attributes.a[ 0 ] ).to.equal( 'foo' );
 			expect( tpl.attributes.b[ 0 ] ).to.equal( 'bar' );
 			expect( tpl.attributes.b[ 1 ] ).to.equal( 'baz' );
-			expect( tpl.attributes.c[ 0 ].value[ 0 ].type ).to.be.a( 'symbol' );
+			expect( tpl.attributes.c[ 0 ].value[ 0 ] ).to.be.instanceof( TemplateToBinding );
 
 			expect( tpl.children ).to.have.length( 4 );
 			expect( tpl.children.get( 0 ).text[ 0 ] ).to.equal( 'content' );
-			expect( tpl.children.get( 1 ).text[ 0 ].type ).to.be.a( 'symbol' );
+			expect( tpl.children.get( 1 ).text[ 0 ] ).to.be.instanceof( TemplateToBinding );
 			expect( tpl.children.get( 2 ).text[ 0 ] ).to.equal( 'abc' );
 			expect( tpl.children.get( 3 ).text[ 0 ] ).to.equal( 'a' );
 			expect( tpl.children.get( 3 ).text[ 1 ] ).to.equal( 'b' );
 
-			expect( tpl.eventListeners[ 'a@span' ][ 0 ].type ).to.be.a( 'symbol' );
-			expect( tpl.eventListeners[ 'b@span' ][ 0 ].type ).to.be.a( 'symbol' );
-			expect( tpl.eventListeners[ 'c@span' ][ 0 ].type ).to.be.a( 'symbol' );
-			expect( tpl.eventListeners[ 'c@span' ][ 1 ].type ).to.be.a( 'symbol' );
+			expect( tpl.eventListeners[ 'a@span' ][ 0 ] ).to.be.instanceof( TemplateToBinding );
+			expect( tpl.eventListeners[ 'b@span' ][ 0 ] ).to.be.instanceof( TemplateToBinding );
+			expect( tpl.eventListeners[ 'c@span' ][ 0 ] ).to.be.instanceof( TemplateToBinding );
+			expect( tpl.eventListeners[ 'c@span' ][ 1 ] ).to.be.instanceof( TemplateToBinding );
 
 			// Note that Template mixes EmitterMixin.
 			expect( tpl.on ).to.be.a( 'function' );
@@ -961,12 +962,13 @@ describe( 'Template', () => {
 			} );
 
 			describe( 'to', () => {
-				it( 'returns an object which describes the binding', () => {
+				it( 'returns an instance of TemplateToBinding', () => {
 					const spy = testUtils.sinon.spy();
 					const binding = bind.to( 'foo', spy );
 
+					expect( binding ).to.be.instanceof( TemplateToBinding );
 					expect( spy.called ).to.be.false;
-					expect( binding ).to.have.keys( [ 'type', 'observable', 'eventNameOrFunction', 'emitter', 'attribute', 'callback' ] );
+					expect( binding ).to.have.keys( [ 'observable', 'eventNameOrFunction', 'emitter', 'attribute', 'callback' ] );
 					expect( binding.observable ).to.equal( observable );
 					expect( binding.callback ).to.equal( spy );
 					expect( binding.attribute ).to.equal( 'foo' );
@@ -1186,8 +1188,9 @@ describe( 'Template', () => {
 					const spy = testUtils.sinon.spy();
 					const binding = bind.if( 'foo', 'whenTrue', spy );
 
+					expect( binding ).to.be.instanceof( TemplateIfBinding );
 					expect( spy.called ).to.be.false;
-					expect( binding ).to.have.keys( [ 'type', 'observable', 'emitter', 'attribute', 'callback', 'valueIfTrue' ] );
+					expect( binding ).to.have.keys( [ 'observable', 'emitter', 'attribute', 'callback', 'valueIfTrue' ] );
 					expect( binding.observable ).to.equal( observable );
 					expect( binding.callback ).to.equal( spy );
 					expect( binding.attribute ).to.equal( 'foo' );