8
0
Просмотр исходного кода

Refactored renderign of style attribute.

Oskar Wrobel 9 лет назад
Родитель
Сommit
3c3cf802b2
2 измененных файлов с 91 добавлено и 160 удалено
  1. 44 97
      packages/ckeditor5-ui/src/template.js
  2. 47 63
      packages/ckeditor5-ui/tests/template.js

+ 44 - 97
packages/ckeditor5-ui/src/template.js

@@ -405,12 +405,6 @@ export default class Template {
 			attrValue = attributes[ attrName ];
 			attrNs = attrValue[ 0 ].ns || null;
 
-			// Attribute style might have specific format so needs to be parsed in a specific way.
-			if ( attrName == 'style' ) {
-				this._renderAttributeStyle( attrValue[ 0 ].value || attrValue[ 0 ], el );
-				continue;
-			}
-
 			// Activate binding if one is found. Cases:
 			// 		{ class: [ Template.bind.to( ... ) ] }
 			// 		{ class: [ 'bar', Template.bind.to( ... ), 'baz' ] }
@@ -425,6 +419,15 @@ export default class Template {
 				);
 			}
 
+			// Attribute style has a specific format so needs to be parsed in a specific way
+			// 		{ style: {
+			// 			width: '100px',
+			// 			height: Template.bind.to( ... )
+			// 		}
+			else if ( attrName == 'style' ) {
+				this._renderStyleAttribute( attrValue[ 0 ].value || attrValue[ 0 ], el );
+			}
+
 			// Otherwise simply set the attribute.
 			// 		{ class: [ 'foo' ] }
 			// 		{ class: [ 'all', 'are', 'static' ] }
@@ -444,106 +447,39 @@ export default class Template {
 	}
 
 	/**
-	 * Render attribute `style`.
+	 * Render `style` attribute.
 	 *
-	 * Attribute style value could be an {Object} with static or binded to model properties:
-	 *
-	 *		new Model( {
-	 *			modelProperty: 'value'
-	 *		} );
+	 * Value of style attribute is an {Object} with static or bound to model properties:
 	 *
 	 *		attributes: {
 	 * 			style: {
 	 * 				property: value,
-	 * 				otherProperty: bind.to( 'modelProperty' ),
-	 * 			}
-	 * 		}
-	 *
-	 * 	or a {String} if whole style attribute is binded to model property:
-	 *
-	 * 		new Model( {
-	 *			style: 'value'
-	 *		} );
-	 *
-	 * 		attributes: {
-	 *			style: bind.to( 'style' )
-	 * 		}
-	 *
-	 * Note: Multiple-word properties are always defined in camelCase format:
-	 *
-	 * 		attributes: {
-	 * 			style: {
-	 * 				backgroundColor: ...,
-	 * 				borderWidth: ...
+	 * 				otherProperty: bind.to( ... ),
 	 * 			}
 	 * 		}
 	 *
 	 * Note: Attribute `style` is rendered without setting namespace because:
 	 * 1. It seems to be not necessary
-	 * 2. We are using more efficient methods for setting styles `el.style.property` and `el.style.cssText` instead of
-	 * `setAttributeNS()`
+	 * 2. We are using more efficient way for updating style `el.style.property = value;` instead of
+	 * `setAttributeNS( 'style', value );`
 	 *
 	 * @private
 	 * @param {ui.TemplateDefinition.attributes.styles} styles Styles definition.
 	 * @param {HTMLElement} el Element which is rendered.
 	 */
-	_renderAttributeStyle( styles, el ) {
-		// If whole attribute is binded we don't parse value.
-		// We render attribute in similar way to the rest of attributes and we take care of proper value format.
-		if ( styles.observable ) {
-			const { emitter, observable, attribute } = styles;
-
-			// Validate value format for initial value.
-			if ( typeof observable[ attribute ] != 'string' ) {
-				throw new CKEditorError(
-					'template-renderAttributeStyle-invalid-format: Value for Whole binded property must be a string',
-					{ value: observable[ attribute ] }
-				);
-			}
-
-			emitter.listenTo( observable, `change:${ attribute }`, ( eventInfo, property, value ) => {
-				// Validate value format for updated value.
-				if ( typeof value != 'string' ) {
-					throw new CKEditorError(
-						'template-renderAttributeStyle-invalid-format: Value for Whole binded property must be a string',
-						{ value }
-					);
-				}
-
-				el.style.cssText = value;
-			} );
-
-			el.style.cssText = observable[ attribute ];
-
-			return;
-		}
+	_renderStyleAttribute( styles, el ) {
+		for ( let styleName in styles ) {
+			const styleValue = styles[ styleName ];
 
-		// Iterate through every single style.
-		const initialStyles = Object.keys( styles ).map( ( style ) => {
-			// If style value is not observable.
-			if ( !styles[ style ].observable ) {
-				// Just return style as `property-name: value`.
-				// Note that style property has to be transformed from camelCase to dash
-				// for setting initial value by `el.style.cssText`.
-				return `${ camelCaseToDash( style ) }:${ styles[ style ] }`;
+			// style: { color: bind.to( 'attribute' ) }
+			if ( hasBinding( styleValue ) ) {
+				this._bindToObservable( [ styleValue ], el, getStyleUpdater( el, styleName ) );
 			}
-
-			// If style value is observable.
-			const { emitter, observable, attribute } = styles[ style ];
-
-			// Add listener.
-			emitter.listenTo( observable, `change:${ attribute }`, ( eventInfo, property, value ) => {
-				// And update only this style which has been changed.
-				el.style[ style ] = value;
-			} );
-
-			// A the end return style as `property-name: value`.
-			// Note that style property has to be transformed from camelCase to dash
-			// for setting initial value by `setAttributeNS()`.
-			return `${ camelCaseToDash( style ) }:${ styles[ style ].observable[ attribute ] }`;
-		} );
-
-		el.style.cssText = `${ initialStyles.join( ';' ) }`;
+			// style: { color: 'red' }
+			else {
+				el.style[ styleName ] = styleValue;
+			}
+		}
 	}
 
 	/**
@@ -751,6 +687,25 @@ function getAttributeUpdater( el, attrName, ns = null ) {
 	};
 }
 
+// Returns an object consisting of `set` and `remove` functions, which
+// can be used in the context of CSSStyleDeclaration to set or remove an style.
+// @see ui.View#_bindToObservable
+//
+// @param {Node} node DOM Node to be modified.
+// @param {String} styleName Name of the style to be modified.
+// @returns {Object}
+function getStyleUpdater( el, styleName ) {
+	return {
+		set( value ) {
+			el.style[ styleName ] = value;
+		},
+
+		remove() {
+			el.style[ styleName ] = null;
+		}
+	};
+}
+
 // Clones definition of the template.
 //
 // @param {ui.TemplateDefinition} def
@@ -998,14 +953,6 @@ function extendTemplateDefinition( def, extDef ) {
 	}
 }
 
-// Transform camelCase to dash. `someString` to `some-string`.
-//
-// @param {String} value String to transform.
-// @returns {String} Transformed string.
-function camelCaseToDash( value ) {
-	return value.replace( /([a-z])([A-Z])/g, '$1-$2' ).toLowerCase();
-}
-
 /**
  * A definition of {@link ui.Template}.
  * See: {@link ui.TemplateValueSchema}.

+ 47 - 63
packages/ckeditor5-ui/tests/template.js

@@ -317,98 +317,82 @@ describe( 'Template', () => {
 		} );
 
 		describe( '`style` attribute', () => {
-			it( 'renders with dynamic and static properties', () => {
-				const observable = new Model( {
-					width: '100px',
-					color: 'yellow'
+			let observable, emitter, bind;
+
+			beforeEach( () => {
+				observable = new Model( {
+					width: '10px',
+					backgroundColor: 'yellow'
 				} );
 
-				const emitter = Object.create( EmitterMixin );
-				const bind = Template.bind( observable, emitter );
-				const el = new Template( {
+				emitter = Object.create( EmitterMixin );
+				bind = Template.bind( observable, emitter );
+			} );
+
+			it( 'renders with static and bound properties', () => {
+				setElement( {
 					tag: 'p',
 					attributes: {
 						style: {
 							width: bind.to( 'width' ),
-							height: '200px',
-							backgroundColor: bind.to( 'color' )
+							height: '10px',
+							backgroundColor: bind.to( 'backgroundColor' )
 						}
 					}
-				} ).render();
+				} );
 
-				expect( el.style.width ).to.be.equal( '100px' );
-				expect( el.style.height ).to.be.equal( '200px' );
-				expect( el.style.backgroundColor ).to.be.equal( 'yellow' );
+				expect( el.outerHTML ).to.equal( '<p style="width: 10px; height: 10px; background-color: yellow;"></p>' );
 
-				observable.width = '50px';
-				observable.color = 'green';
+				observable.width = '20px';
+				observable.backgroundColor = 'green';
 
-				expect( el.style.width ).to.be.equal( '50px' );
-				expect( el.style.height ).to.be.equal( '200px' );
-				expect( el.style.backgroundColor ).to.be.equal( 'green' );
+				expect( el.outerHTML ).to.equal( '<p style="width: 20px; height: 10px; background-color: green;"></p>' );
 			} );
 
-			it( 'renders fully binded', () => {
-				const observable = new Model( {
-					style: 'width: 100px'
-				} );
-
-				const emitter = Object.create( EmitterMixin );
-				const bind = Template.bind( observable, emitter );
-				const el = new Template( {
+			it( 'renders with empty properties', () => {
+				setElement( {
 					tag: 'p',
 					attributes: {
-						style: bind.to( 'style' )
+						style: {
+							width: '',
+							backgroundColor: bind.to( 'backgroundColor' )
+						}
 					}
-				} ).render();
-
-				expect( el.style.width ).to.be.equal( '100px' );
+				} );
 
-				observable.style = 'width:20px;height:50px';
+				observable.backgroundColor = '';
 
-				expect( el.style.width ).to.be.equal( '20px' );
-				expect( el.style.height ).to.be.equal( '50px' );
+				expect( el.outerHTML ).to.be.equal( '<p></p>' );
 			} );
 
-			it( 'throws when wrong format for initial value of fully binded', () => {
-				const observable = new Model( {
-					style: {
-						width: '100px'
+			it( 'renders with falsy values', () => {
+				setElement( {
+					tag: 'p',
+					attributes: {
+						style: {
+							width: null,
+							height: false
+						}
 					}
 				} );
 
-				const emitter = Object.create( EmitterMixin );
-				const bind = Template.bind( observable, emitter );
-
-				expect( () => {
-					new Template( {
-						tag: 'p',
-						attributes: {
-							style: bind.to( 'style' )
-						}
-					} ).render();
-				} ).to.throw( CKEditorError, /template-renderAttributeStyle-invalid-format/ );
+				expect( el.outerHTML ).to.be.equal( '<p style="background-color: yellow;"></p>' );
 			} );
 
-			it( 'throws when wrong format for updated value of fully binded', () => {
-				const observable = new Model( {
-					style: 'width: 100px'
-				} );
-
-				const emitter = Object.create( EmitterMixin );
-				const bind = Template.bind( observable, emitter );
-				new Template( {
+			it( 'renders with empty properties', () => {
+				setElement( {
 					tag: 'p',
 					attributes: {
-						style: bind.to( 'style' )
+						style: {
+							width: '',
+							backgroundColor: bind.to( 'backgroundColor' )
+						}
 					}
-				} ).render();
+				} );
 
-				expect( () => {
-					observable.style = {
-						width: '100px'
-					};
-				} ).to.throw( CKEditorError, /template-renderAttributeStyle-invalid-format/ );
+				observable.backgroundColor = '';
+
+				expect( el.outerHTML ).to.be.equal( '<p></p>' );
 			} );
 		} );
 	} );