Browse Source

Add more tests to the styles class.

Maciej Gołaszewski 6 years ago
parent
commit
b12e798f26

+ 94 - 24
packages/ckeditor5-engine/src/view/styles.js

@@ -91,9 +91,21 @@ export default class Styles {
 	}
 
 	/**
-	 * Inserts single style rule.
+	 * Inserts single style property.
 	 *
-	 * Supports shorthands.
+	 * Can insert one by one
+	 *
+	 *		styles.insertProperty( 'color', 'blue' );
+	 *		styles.insertProperty( 'margin-right', '1em' );
+	 *
+	 * or many styles at once:
+	 *
+	 *		styles.insertProperty( {
+	 *			color: 'blue',
+	 *			'margin-right': '1em'
+	 *		} );
+	 *
+	 * Supports shorthands.y
 	 *
 	 * @param {String|Object} nameOrObject
 	 * @param {String|Object} value
@@ -109,12 +121,34 @@ export default class Styles {
 		}
 	}
 
+	/**
+	 * Removes styles property.
+	 *
+	 * @param name
+	 */
 	removeProperty( name ) {
 		unset( this._styles, toPath( name ) );
-
 		delete this._styles[ name ];
 	}
 
+	/**
+	 * Return normalized style object;
+	 *
+	 *		const styles = new Styles();
+	 *		styles.setStyle( 'margin:1px 2px 3em;' );
+	 *
+	 *		console.log( styles.getNormalized( 'margin' ) );
+	 *		// will log:
+	 *		// {
+	 *		//     top: '1px',
+	 *		//     right: '2px',
+	 *		//     bottom: '3em',
+	 *		//     left: '2px'
+	 *		// }
+	 *
+	 * @param {String} name
+	 * @returns {Object|undefined}
+	 */
 	getNormalized( name ) {
 		if ( !name ) {
 			return merge( {}, this._styles );
@@ -129,24 +163,33 @@ export default class Styles {
 		}
 	}
 
+	/**
+	 * Returns a string containing normalized styles string or undefined if no style properties are set.
+	 *
+	 * @returns {String|undefined}
+	 */
 	getInlineStyle() {
-		const parsed = this._toFFFFFFFFFFStylesMap();
+		const entries = this._getStylesEntries();
 
-		if ( !parsed.length ) {
+		// Return undefined for empty styles map.
+		if ( !entries.length ) {
 			return;
 		}
 
-		return parsed
-			.filter( v => Array.isArray( v ) )// TODO: not needed?
-			.map( arr => arr.join( ':' ) )
-			.join( ';' ) + ';';
+		return entries.map( arr => arr.join( ':' ) ).join( ';' ) + ';';
 	}
 
+	/**
+	 * Returns property value string.
+	 *
+	 * @param {String} propertyName
+	 * @returns {String|undefined}
+	 */
 	getInlineProperty( propertyName ) {
 		const normalized = this.getNormalized( propertyName );
 
 		if ( !normalized ) {
-			// Try return directly
+			// Try return styles set directly - values that are not parsed.
 			return this._styles[ propertyName ];
 		}
 
@@ -157,24 +200,36 @@ export default class Styles {
 			if ( Array.isArray( propertyDescriptor ) ) {
 				return propertyDescriptor[ 1 ];
 			}
-		}
-		// String value
-		else {
+		} else {
 			return normalized;
 		}
 	}
 
+	/**
+	 * Returns style properties names as the would appear when using {@link #getInlineStyle()}
+	 *
+	 * @returns {Array.<String>}
+	 */
 	getStyleNames() {
-		const parsed = this._toFFFFFFFFFFStylesMap();
+		const entries = this._getStylesEntries();
 
-		return parsed.map( ( [ key ] ) => key );
+		return entries.map( ( [ key ] ) => key );
 	}
 
+	/**
+	 * Removes all styles.
+	 */
 	clear() {
 		this._styles = {};
 	}
 
-	_toFFFFFFFFFFStylesMap() {
+	/**
+	 * Returns normalized styles entries for further processing.
+	 *
+	 * @private
+	 * @returns {Array.<Array.<String, String>> ]}
+	 */
+	_getStylesEntries() {
 		const parsed = [];
 
 		const keys = Object.keys( this._styles ).sort();
@@ -188,10 +243,18 @@ export default class Styles {
 		return parsed;
 	}
 
+	/**
+	 * Appends style definition to the internal styles object.
+	 *
+	 * @param {String} nameOrPath
+	 * @param {String|Object} valueOrObject
+	 * @private
+	 */
 	_appendStyleValue( nameOrPath, valueOrObject ) {
-		if ( typeof valueOrObject === 'object' ) {
+		if ( isObject( valueOrObject ) ) {
 			if ( nameOrPath.includes( '.' ) ) {
 				const got = get( this._styles, nameOrPath );
+
 				set( this._styles, nameOrPath, merge( {}, got, valueOrObject ) );
 			} else {
 				this._styles[ nameOrPath ] = merge( {}, this._styles[ nameOrPath ], valueOrObject );
@@ -201,26 +264,33 @@ export default class Styles {
 		}
 	}
 
-	_parseProperty( key, value ) {
+	/**
+	 * Parses single style property.
+	 *
+	 * @param {String} name Name of style property.
+	 * @param {String} value Value of style property.
+	 * @private
+	 */
+	_parseProperty( name, value ) {
 		if ( isPlainObject( value ) ) {
-			this._appendStyleValue( toPath( key ), value );
+			this._appendStyleValue( toPath( name ), value );
 
 			return;
 		}
 
 		// Set directly to an object.
-		if ( setOnPathStyles.includes( key ) ) {
-			this._appendStyleValue( toPath( key ), value );
+		if ( setOnPathStyles.includes( name ) ) {
+			this._appendStyleValue( toPath( name ), value );
 
 			return;
 		}
 
-		if ( this.parsers.has( key ) ) {
-			const parser = this.parsers.get( key );
+		if ( this.parsers.has( name ) ) {
+			const parser = this.parsers.get( name );
 
 			this._styles = merge( {}, this._styles, parser( value ) );
 		} else {
-			this._appendStyleValue( key, value );
+			this._appendStyleValue( name, value );
 		}
 	}
 }

+ 0 - 64
packages/ckeditor5-engine/tests/view/element.js

@@ -9,8 +9,6 @@ import Element from '../../src/view/element';
 import Text from '../../src/view/text';
 import TextProxy from '../../src/view/textproxy';
 
-import encodedImage from './_utils/encodedimage.txt';
-
 describe( 'Element', () => {
 	describe( 'constructor()', () => {
 		it( 'should create element without attributes', () => {
@@ -872,68 +870,6 @@ describe( 'Element', () => {
 				expect( el.hasStyle( 'color' ) ).to.be.true;
 			} );
 		} );
-
-		describe( 'styles parsing edge cases and incorrect styles', () => {
-			it( 'should not crash and not add any styles if styles attribute was empty', () => {
-				const element = new Element( 'div', { style: '' } );
-				const styles = Array.from( element.getStyleNames() );
-
-				expect( styles ).to.deep.equal( [] );
-			} );
-
-			it( 'should be able to parse big styles definition', () => {
-				expect( () => {
-					// eslint-disable-next-line no-new
-					new Element( 'div', { style: `background-image:url('data:image/jpeg;base64,${ encodedImage }')` } );
-				} ).not.to.throw();
-			} );
-
-			it( 'should work with both types of quotes and ignore values inside quotes', () => {
-				let element;
-
-				element = new Element( 'div', { style: 'background-image:url("im;color:g.jpg")' } );
-				expect( element.getStyle( 'background-image' ) ).to.equal( 'url("im;color:g.jpg")' );
-
-				element = new Element( 'div', { style: 'background-image:url(\'im;color:g.jpg\')' } );
-				expect( element.getStyle( 'background-image' ) ).to.equal( 'url(\'im;color:g.jpg\')' );
-			} );
-
-			it( 'should not be confused by whitespaces', () => {
-				const element = new Element( 'div', { style: '\ncolor:\n red ' } );
-
-				expect( element.getStyle( 'color' ) ).to.equal( 'red' );
-			} );
-
-			it( 'should not be confused by duplicated semicolon', () => {
-				const element = new Element( 'div', { style: 'color: red;; display: inline' } );
-
-				expect( element.getStyle( 'color' ) ).to.equal( 'red' );
-				expect( element.getStyle( 'display' ) ).to.equal( 'inline' );
-			} );
-
-			it( 'should not throw when value is missing', () => {
-				const element = new Element( 'div', { style: 'color:; display: inline' } );
-
-				expect( element.getStyle( 'color' ) ).to.equal( '' );
-				expect( element.getStyle( 'display' ) ).to.equal( 'inline' );
-			} );
-
-			it( 'should not throw when colon is duplicated', () => {
-				const element = new Element( 'div', { style: 'color:: red; display: inline' } );
-
-				// The result makes no sense, but here we just check that the algorithm doesn't crash.
-				expect( element.getStyle( 'color' ) ).to.equal( ': red' );
-				expect( element.getStyle( 'display' ) ).to.equal( 'inline' );
-			} );
-
-			it( 'should not throw when random stuff passed', () => {
-				const element = new Element( 'div', { style: 'color: red;:; ;;" ":  display: inline; \'aaa;:' } );
-
-				// The result makes no sense, but here we just check that the algorithm doesn't crash.
-				expect( element.getStyle( 'color' ) ).to.equal( 'red' );
-				expect( element.getStyle( 'display' ) ).to.be.undefined;
-			} );
-		} );
 	} );
 
 	describe( 'findAncestor', () => {

+ 61 - 1
packages/ckeditor5-engine/tests/view/styles.js

@@ -2,7 +2,9 @@
  * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
+
 import StyleProxy from '../../src/view/styles';
+import encodedImage from './_utils/encodedimage.txt';
 
 describe( 'Styles', () => {
 	let styles;
@@ -38,6 +40,64 @@ describe( 'Styles', () => {
 
 			expect( styles.getNormalized() ).to.deep.equal( { margin: { bottom: '2em' } } );
 		} );
+
+		describe( 'styles parsing edge cases and incorrect styles', () => {
+			it( 'should not crash and not add any styles if styles attribute was empty', () => {
+				styles.setStyle( '' );
+
+				expect( styles.getStyleNames() ).to.deep.equal( [] );
+			} );
+
+			it( 'should be able to parse big styles definition', () => {
+				expect( () => {
+					styles.setStyle( `background-image:url('data:image/jpeg;base64,${ encodedImage }')` );
+				} ).not.to.throw();
+			} );
+
+			it( 'should work with both types of quotes and ignore values inside quotes', () => {
+				styles.setStyle( 'background-image:url("im;color:g.jpg")' );
+				expect( styles.getInlineProperty( 'background-image' ) ).to.equal( 'url("im;color:g.jpg")' );
+
+				styles.setStyle( 'background-image:url(\'im;color:g.jpg\')' );
+				expect( styles.getInlineProperty( 'background-image' ) ).to.equal( 'url(\'im;color:g.jpg\')' );
+			} );
+
+			it( 'should not be confused by whitespaces', () => {
+				styles.setStyle( '\ncolor:\n red ' );
+
+				expect( styles.getInlineProperty( 'color' ) ).to.equal( 'red' );
+			} );
+
+			it( 'should not be confused by duplicated semicolon', () => {
+				styles.setStyle( 'color: red;; display: inline' );
+
+				expect( styles.getInlineProperty( 'color' ) ).to.equal( 'red' );
+				expect( styles.getInlineProperty( 'display' ) ).to.equal( 'inline' );
+			} );
+
+			it( 'should not throw when value is missing', () => {
+				styles.setStyle( 'color:; display: inline' );
+
+				expect( styles.getInlineProperty( 'color' ) ).to.equal( '' );
+				expect( styles.getInlineProperty( 'display' ) ).to.equal( 'inline' );
+			} );
+
+			it( 'should not throw when colon is duplicated', () => {
+				styles.setStyle( 'color:: red; display: inline' );
+
+				// The result makes no sense, but here we just check that the algorithm doesn't crash.
+				expect( styles.getInlineProperty( 'color' ) ).to.equal( ': red' );
+				expect( styles.getInlineProperty( 'display' ) ).to.equal( 'inline' );
+			} );
+
+			it( 'should not throw when random stuff passed', () => {
+				styles.setStyle( 'color: red;:; ;;" ":  display: inline; \'aaa;:' );
+
+				// The result makes no sense, but here we just check that the algorithm doesn't crash.
+				expect( styles.getInlineProperty( 'color' ) ).to.equal( 'red' );
+				expect( styles.getInlineProperty( 'display' ) ).to.be.undefined;
+			} );
+		} );
 	} );
 
 	describe( 'getInlineStyle()', () => {
@@ -107,7 +167,7 @@ describe( 'Styles', () => {
 			expect( styles.getInlineProperty( 'color' ) ).to.equal( 'blue' );
 		} );
 
-		it( 'should work with objects', () => {
+		it( 'should set multiple styles by providing an object', () => {
 			styles.setStyle( 'color: red;' );
 			styles.insertProperty( { color: 'blue', margin: '1px' } );