Sfoglia il codice sorgente

Fix border styles normalization code. Add tests for utils methods.

Maciej Gołaszewski 6 anni fa
parent
commit
19989b5e12

+ 4 - 4
packages/ckeditor5-engine/src/view/styles.js

@@ -93,7 +93,7 @@ export class StylesConverter {
 	 * @param {Object|String} normalizedValue
 	 * @returns {Array.<Array.<String, String>>}
 	 */
-	getReduceForm( styleName, normalizedValue ) {
+	getReducedForm( styleName, normalizedValue ) {
 		const data = {
 			value: normalizedValue
 		};
@@ -321,7 +321,7 @@ export default class Styles {
 	 * @returns {String|undefined}
 	 */
 	getInlineProperty( propertyName ) {
-		const normalized = stylesConverter.getNormalized( propertyName, this._styles );
+		const normalized = this.converter.getNormalized( propertyName, this._styles );
 
 		if ( !normalized ) {
 			// Try return styles set directly - values that are not parsed.
@@ -329,7 +329,7 @@ export default class Styles {
 		}
 
 		if ( isObject( normalized ) ) {
-			const styles = stylesConverter.getReduceForm( propertyName, normalized );
+			const styles = this.converter.getReducedForm( propertyName, normalized );
 
 			const propertyDescriptor = styles.find( ( [ property ] ) => property === propertyName );
 
@@ -374,7 +374,7 @@ export default class Styles {
 		for ( const key of keys ) {
 			const normalized = this.converter.getNormalized( key, this._styles );
 
-			parsed.push( ...this.converter.getReduceForm( key, normalized ) );
+			parsed.push( ...this.converter.getReducedForm( key, normalized ) );
 		}
 
 		return parsed;

+ 63 - 65
packages/ckeditor5-engine/src/view/styles/borderstyles.js

@@ -3,14 +3,14 @@
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
 
-import { getTopRightBottomLeftValues, getTopRightBottomLeftValueReducer, isColor, isLength, isLineStyle } from './utils';
+import { getParts, getTopRightBottomLeftValueReducer, getTopRightBottomLeftValues, isLength, isLineStyle } from './utils';
 
 /**
  * @module engine/view/styles
  */
 export default class BorderStyles {
 	static attach( stylesConverter ) {
-		stylesConverter.on( 'normalize:border', normalizeBorder );
+		stylesConverter.on( 'normalize:border', borderNormalizer );
 
 		// Border-position shorthands.
 		stylesConverter.on( 'normalize:border-top', getBorderPositionNormalizer( 'top' ) );
@@ -40,10 +40,10 @@ export default class BorderStyles {
 		stylesConverter.on( 'normalize:border-left-style', getBorderPropertyPositionNormalizer( 'style', 'left' ) );
 		stylesConverter.on( 'normalize:border-left-width', getBorderPropertyPositionNormalizer( 'width', 'left' ) );
 
-		stylesConverter.on( 'extract:border-top', borderPositionExtractor( 'top' ) );
-		stylesConverter.on( 'extract:border-right', borderPositionExtractor( 'right' ) );
-		stylesConverter.on( 'extract:border-bottom', borderPositionExtractor( 'bottom' ) );
-		stylesConverter.on( 'extract:border-left', borderPositionExtractor( 'left' ) );
+		stylesConverter.on( 'extract:border-top', getBorderPositionExtractor( 'top' ) );
+		stylesConverter.on( 'extract:border-right', getBorderPositionExtractor( 'right' ) );
+		stylesConverter.on( 'extract:border-bottom', getBorderPositionExtractor( 'bottom' ) );
+		stylesConverter.on( 'extract:border-left', getBorderPositionExtractor( 'left' ) );
 
 		stylesConverter.on( 'extract:border-top-color', ( evt, data ) => ( data.path = 'border.color.top' ) );
 		stylesConverter.on( 'extract:border-right-color', ( evt, data ) => ( data.path = 'border.color.right' ) );
@@ -63,25 +63,15 @@ export default class BorderStyles {
 		stylesConverter.on( 'reduce:border-color', getTopRightBottomLeftValueReducer( 'border-color' ) );
 		stylesConverter.on( 'reduce:border-style', getTopRightBottomLeftValueReducer( 'border-style' ) );
 		stylesConverter.on( 'reduce:border-width', getTopRightBottomLeftValueReducer( 'border-width' ) );
-		stylesConverter.on( 'reduce:border-top',
-			( evt, data ) => ( data.reduced = getBorderPositionReducer( 'top' )( data.value ) ) );
-		stylesConverter.on( 'reduce:border-right',
-			( evt, data ) => ( data.reduced = getBorderPositionReducer( 'right' )( data.value ) ) );
-		stylesConverter.on( 'reduce:border-bottom',
-			( evt, data ) => ( data.reduced = getBorderPositionReducer( 'bottom' )( data.value ) ) );
-		stylesConverter.on( 'reduce:border-left',
-			( evt, data ) => ( data.reduced = getBorderPositionReducer( 'left' )( data.value ) ) );
-		stylesConverter.on( 'reduce:border', getBorderReducer );
+		stylesConverter.on( 'reduce:border-top', getBorderPositionReducer( 'top' ) );
+		stylesConverter.on( 'reduce:border-right', getBorderPositionReducer( 'right' ) );
+		stylesConverter.on( 'reduce:border-bottom', getBorderPositionReducer( 'bottom' ) );
+		stylesConverter.on( 'reduce:border-left', getBorderPositionReducer( 'left' ) );
+		stylesConverter.on( 'reduce:border', borderReducer );
 	}
 }
 
-function toBorderPropertyShorthand( value, property ) {
-	return {
-		[ property ]: getTopRightBottomLeftValues( value )
-	};
-}
-
-function normalizeBorder( evt, data ) {
+function borderNormalizer( evt, data ) {
 	const { color, style, width } = normalizeBorderShorthand( data.value );
 
 	data.path = 'border';
@@ -122,6 +112,12 @@ function getBorderPropertyNormalizer( propertyName ) {
 	};
 }
 
+function toBorderPropertyShorthand( value, property ) {
+	return {
+		[ property ]: getTopRightBottomLeftValues( value )
+	};
+}
+
 function getBorderPropertyPositionNormalizer( property, side ) {
 	return ( evt, data ) => {
 		data.path = 'border';
@@ -133,41 +129,41 @@ function getBorderPropertyPositionNormalizer( property, side ) {
 	};
 }
 
-function borderPositionExtractor( which ) {
+function getBorderPositionExtractor( which ) {
 	return ( evt, data ) => {
-		const border = data.styles.border;
+		data.value = extractBorderPosition( data.styles.border, which, data );
+	};
+}
 
-		const value = [];
+function extractBorderPosition( border, which ) {
+	const value = {};
 
-		if ( border.width && border.width[ which ] ) {
-			value.push( border.width[ which ] );
-		}
+	if ( border.width && border.width[ which ] ) {
+		value.width = border.width[ which ];
+	}
 
-		if ( border.style && border.style[ which ] ) {
-			value.push( border.style[ which ] );
-		}
+	if ( border.style && border.style[ which ] ) {
+		value.style = border.style[ which ];
+	}
 
-		if ( border.color && border.color[ which ] ) {
-			value.push( border.color[ which ] );
-		}
+	if ( border.color && border.color[ which ] ) {
+		value.color = border.color[ which ];
+	}
 
-		data.value = value.join( ' ' );
-	};
+	return value;
 }
 
 function normalizeBorderShorthand( string ) {
 	const result = {};
 
-	for ( const part of string.split( ' ' ) ) {
-		if ( isLength( part ) ) {
-			result.width = part;
-		}
+	const parts = getParts( string );
 
-		if ( isLineStyle( part ) ) {
+	for ( const part of parts ) {
+		if ( isLength( part ) || /thin|medium|thick/.test( part ) ) {
+			result.width = part;
+		} else if ( isLineStyle( part ) ) {
 			result.style = part;
-		}
-
-		if ( isColor( part ) ) {
+		} else {
 			result.color = part;
 		}
 	}
@@ -175,37 +171,39 @@ function normalizeBorderShorthand( string ) {
 	return result;
 }
 
-function getBorderReducer( evt, data ) {
+function borderReducer( evt, data ) {
 	const ret = [];
 
-	ret.push( ...getBorderPositionReducer( 'top' )( data.value ) );
-	ret.push( ...getBorderPositionReducer( 'right' )( data.value ) );
-	ret.push( ...getBorderPositionReducer( 'bottom' )( data.value ) );
-	ret.push( ...getBorderPositionReducer( 'left' )( data.value ) );
+	ret.push( ...reduceBorderPosition( extractBorderPosition( data.value, 'top' ), 'top' ) );
+	ret.push( ...reduceBorderPosition( extractBorderPosition( data.value, 'right' ), 'right' ) );
+	ret.push( ...reduceBorderPosition( extractBorderPosition( data.value, 'bottom' ), 'bottom' ) );
+	ret.push( ...reduceBorderPosition( extractBorderPosition( data.value, 'left' ), 'left' ) );
 
 	data.reduced = ret;
 }
 
 function getBorderPositionReducer( which ) {
-	return value => {
-		const reduced = [];
+	return ( evt, data ) => ( data.reduced = reduceBorderPosition( data.value, which ) );
+}
 
-		if ( value && value.width && value.width[ which ] !== undefined ) {
-			reduced.push( value.width[ which ] );
-		}
+function reduceBorderPosition( value, which ) {
+	const reduced = [];
 
-		if ( value && value.style && value.style[ which ] !== undefined ) {
-			reduced.push( value.style[ which ] );
-		}
+	if ( value && value.width !== undefined ) {
+		reduced.push( value.width );
+	}
 
-		if ( value && value.color && value.color[ which ] !== undefined ) {
-			reduced.push( value.color[ which ] );
-		}
+	if ( value && value.style !== undefined ) {
+		reduced.push( value.style );
+	}
 
-		if ( reduced.length ) {
-			return [ [ 'border-' + which, reduced.join( ' ' ) ] ];
-		}
+	if ( value && value.color !== undefined ) {
+		reduced.push( value.color );
+	}
 
-		return [];
-	};
+	if ( reduced.length ) {
+		return [ [ `border-${ which }`, reduced.join( ' ' ) ] ];
+	}
+
+	return [];
 }

+ 31 - 28
packages/ckeditor5-engine/src/view/styles/utils.js

@@ -7,12 +7,40 @@
  * @module engine/view/styles/utils
  */
 
+export function isColor( string ) {
+	return /(^[#0-9A-Fa-f]{3,9}$|rgba?\(|hsla?\(|^currentColor$)/.test( string );
+}
+
+export function isLineStyle( string ) {
+	return /^(none|hidden|dotted|dashed|solid|double|groove|ridge|inset|outset)$/.test( string );
+}
+
+export function isLength( string ) {
+	return /^[+-]?[0-9]?[.]?[0-9]+([a-z]+|%)$/.test( string );
+}
+
+export function isRepeat( string ) {
+	return /^(repeat-x|repeat-y|repeat|space|round|no-repeat)$/.test( string );
+}
+
+export function isPosition( string ) {
+	return /^(center|top|bottom|left|right)$/.test( string );
+}
+
+export function isAttachment( string ) {
+	return /^(fixed|scroll|local)$/.test( string );
+}
+
+export function isURL( string ) {
+	return /^url\(/.test( string );
+}
+
 export function getTopRightBottomLeftValues( value = '' ) {
 	if ( value === '' ) {
 		return { top: undefined, right: undefined, bottom: undefined, left: undefined };
 	}
 
-	const values = value.split( ' ' );
+	const values = getParts( value );
 
 	const top = values[ 0 ];
 	const bottom = values[ 2 ] || top;
@@ -22,18 +50,6 @@ export function getTopRightBottomLeftValues( value = '' ) {
 	return { top, bottom, right, left };
 }
 
-export function isColor( string ) {
-	return /^([#0-9A-Fa-f]{3,8}|[a-zA-Z]+)$/.test( string ) && !isLineStyle( string );
-}
-
-export function isLineStyle( string ) {
-	return /^(none|hidden|dotted|dashed|solid|double|groove|ridge|inset|outset)$/.test( string );
-}
-
-export function isLength( string ) {
-	return /^[+-]?[0-9]?[.]?[0-9]+([a-z]+|%)$/.test( string );
-}
-
 export function getTopRightBottomLeftValueReducer( styleShorthand ) {
 	return ( evt, data ) => {
 		const { top, right, bottom, left } = ( data.value || {} );
@@ -87,19 +103,6 @@ export function getPositionShorthandNormalizer( longhand ) {
 	};
 }
 
-export function isRepeat( string ) {
-	return /^(repeat-x|repeat-y|repeat|space|round|no-repeat)$/.test( string );
-}
-
-export function isPosition( string ) {
-	return /^(center|top|bottom|left|right)$/.test( string );
+export function getParts( string ) {
+	return string.replace( /, /g, ',' ).split( ' ' ).map( string => string.replace( /,/g, ', ' ) );
 }
-
-export function isAttachment( string ) {
-	return /^(fixed|scroll|local)$/.test( string );
-}
-
-export function isURL( string ) {
-	return /^url\(/.test( string );
-}
-

+ 189 - 5
packages/ckeditor5-engine/tests/view/styles/borderstyles.js

@@ -199,6 +199,58 @@ describe( 'Border styles normalization', () => {
 		expect( styles.getInlineProperty( 'border-left' ) ).to.equal( '1px' );
 	} );
 
+	describe( 'normalized values getters', () => {
+		it( 'should output border-*-color', () => {
+			styles.setStyle( 'border:1px solid #f00;' );
+
+			[ 'top', 'right', 'bottom', 'left' ].forEach( position => {
+				expect( styles.getNormalized( `border-${ position }-color` ) ).to.equal( '#f00' );
+			} );
+		} );
+
+		it( 'should output border-*-width', () => {
+			styles.setStyle( 'border:1px solid #f00;' );
+
+			[ 'top', 'right', 'bottom', 'left' ].forEach( position => {
+				expect( styles.getNormalized( `border-${ position }-width` ) ).to.equal( '1px' );
+			} );
+		} );
+
+		it( 'should output border-*-style', () => {
+			styles.setStyle( 'border:1px solid #f00;' );
+
+			[ 'top', 'right', 'bottom', 'left' ].forEach( position => {
+				expect( styles.getNormalized( `border-${ position }-style` ) ).to.equal( 'solid' );
+			} );
+		} );
+	} );
+
+	describe( 'border reducers', () => {
+		it( 'should output border-top', () => {
+			styles.setStyle( 'border:1px solid #f00' );
+
+			expect( styles.getInlineProperty( 'border-top' ) ).to.equal( '1px solid #f00' );
+		} );
+
+		it( 'should output border-right', () => {
+			styles.setStyle( 'border:1px solid #f00' );
+
+			expect( styles.getInlineProperty( 'border-right' ) ).to.equal( '1px solid #f00' );
+		} );
+
+		it( 'should output border-bottom', () => {
+			styles.setStyle( 'border:1px solid #f00' );
+
+			expect( styles.getInlineProperty( 'border-bottom' ) ).to.equal( '1px solid #f00' );
+		} );
+
+		it( 'should output border-left', () => {
+			styles.setStyle( 'border:1px solid #f00' );
+
+			expect( styles.getInlineProperty( 'border-left' ) ).to.equal( '1px solid #f00' );
+		} );
+	} );
+
 	describe( 'border-color', () => {
 		it( 'should set all border colors (1 value defined)', () => {
 			styles.setStyle( 'border-color:cyan;' );
@@ -261,6 +313,50 @@ describe( 'Border styles normalization', () => {
 				width: { top: '1px', right: '1px', bottom: '1px', left: '1px' }
 			} );
 		} );
+
+		it( 'should parse #RGB color value', () => {
+			styles.setStyle( 'border:#f00;' );
+
+			expect( styles.getNormalized( 'border-color' ) ).to.deep.equal( {
+				top: '#f00',
+				right: '#f00',
+				bottom: '#f00',
+				left: '#f00'
+			} );
+		} );
+
+		it( 'should parse #RGBA color value', () => {
+			styles.setStyle( 'border:#f00A;' );
+
+			expect( styles.getNormalized( 'border-color' ) ).to.deep.equal( {
+				top: '#f00A',
+				right: '#f00A',
+				bottom: '#f00A',
+				left: '#f00A'
+			} );
+		} );
+
+		it( 'should parse rgb() color value', () => {
+			styles.setStyle( 'border:rgb(0, 30%,35);' );
+
+			expect( styles.getNormalized( 'border-color' ) ).to.deep.equal( {
+				top: 'rgb(0, 30%, 35)',
+				right: 'rgb(0, 30%, 35)',
+				bottom: 'rgb(0, 30%, 35)',
+				left: 'rgb(0, 30%, 35)'
+			} );
+		} );
+
+		it( 'should parse hsl() color value', () => {
+			styles.setStyle( 'border:hsl(0, 100%, 50%);' );
+
+			expect( styles.getNormalized( 'border-color' ) ).to.deep.equal( {
+				top: 'hsl(0, 100%, 50%)',
+				right: 'hsl(0, 100%, 50%)',
+				bottom: 'hsl(0, 100%, 50%)',
+				left: 'hsl(0, 100%, 50%)'
+			} );
+		} );
 	} );
 
 	describe( 'border-style', () => {
@@ -315,6 +411,39 @@ describe( 'Border styles normalization', () => {
 				}
 			} );
 		} );
+
+		it( 'should parse none value', () => {
+			styles.setStyle( 'border:none;' );
+
+			expect( styles.getNormalized( 'border-style' ) ).to.deep.equal( {
+				top: 'none',
+				right: 'none',
+				bottom: 'none',
+				left: 'none'
+			} );
+		} );
+
+		it( 'should parse line-style value', () => {
+			styles.setStyle( 'border:solid;' );
+
+			expect( styles.getNormalized( 'border-style' ) ).to.deep.equal( {
+				top: 'solid',
+				right: 'solid',
+				bottom: 'solid',
+				left: 'solid'
+			} );
+		} );
+
+		it( 'should not parse non line-style value', () => {
+			styles.setStyle( 'border:blue' );
+
+			expect( styles.getNormalized( 'border-style' ) ).to.deep.equal( {
+				top: undefined,
+				right: undefined,
+				bottom: undefined,
+				left: undefined
+			} );
+		} );
 	} );
 
 	describe( 'border-width', () => {
@@ -369,6 +498,61 @@ describe( 'Border styles normalization', () => {
 				}
 			} );
 		} );
+
+		it( 'should parse px value', () => {
+			styles.setStyle( 'border:1px;' );
+
+			expect( styles.getNormalized( 'border-width' ) ).to.deep.equal( {
+				top: '1px',
+				right: '1px',
+				bottom: '1px',
+				left: '1px'
+			} );
+		} );
+
+		it( 'should parse em value', () => {
+			styles.setStyle( 'border:1em;' );
+
+			expect( styles.getNormalized( 'border-width' ) ).to.deep.equal( {
+				top: '1em',
+				right: '1em',
+				bottom: '1em',
+				left: '1em'
+			} );
+		} );
+
+		it( 'should parse thin value', () => {
+			styles.setStyle( 'border:thin' );
+
+			expect( styles.getNormalized( 'border-width' ) ).to.deep.equal( {
+				top: 'thin',
+				right: 'thin',
+				bottom: 'thin',
+				left: 'thin'
+			} );
+		} );
+
+		it( 'should parse medium value', () => {
+			styles.setStyle( 'border:medium' );
+
+			expect( styles.getNormalized( 'border-width' ) ).to.deep.equal( {
+				top: 'medium',
+				right: 'medium',
+				bottom: 'medium',
+				left: 'medium'
+			} );
+		} );
+
+		it( 'should parse thick value', () => {
+			styles.setStyle( 'border:thick' );
+
+			expect( styles.getNormalized( 'border-width' ) ).to.deep.equal( {
+				top: 'thick',
+				right: 'thick',
+				bottom: 'thick',
+				left: 'thick'
+			} );
+		} );
 	} );
 
 	describe( 'border-* position', () => {
@@ -392,11 +576,11 @@ describe( 'Border styles normalization', () => {
 
 	describe( 'getStyleNames() - border', () => {
 		it( 'should set all border colors (1 value defined)', () => {
-			styles.setStyle( '    border-color: deeppink deepskyblue;\n' +
-				'    border-style: solid;\n' +
-				'    border-width: 1px;\n' +
-				'    border-bottom-width: 2px;\n' +
-				'    border-right-style: dotted;' );
+			styles.setStyle( 'border-color: deeppink deepskyblue;' +
+				'border-style: solid;' +
+				'border-width: 1px;' +
+				'border-bottom-width: 2px;' +
+				'border-right-style: dotted;' );
 
 			expect( styles.getStyleNames() ).to.deep.equal( [
 				'border-top',

+ 72 - 0
packages/ckeditor5-engine/tests/view/styles/utils.js

@@ -0,0 +1,72 @@
+/**
+ * @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 { isColor, isLineStyle } from '../../../src/view/styles/utils';
+
+describe( 'Styles utils', () => {
+	describe( 'isColor()', () => {
+		it( 'returns true for #RGB color', () => {
+			testValues( [ '#f00', '#ba2' ], isColor );
+		} );
+
+		it( 'returns true for #RRGGBB color', () => {
+			testValues( [ '#ff0000', '#bbaa22' ], isColor );
+		} );
+
+		it( 'returns true for #RGBA color', () => {
+			testValues( [ '#f000', '#ba24' ], isColor );
+		} );
+
+		it( 'returns true for #RRGGBBAA color', () => {
+			testValues( [ '#ff000000', '#bbaa2244' ], isColor );
+		} );
+
+		it( 'returns true for rgb() color', () => {
+			testValues( [ 'rgb(255, 255, 255)', 'rgb(23%,0,100%)' ], isColor );
+		} );
+
+		it( 'returns true for rgba() color', () => {
+			testValues( [ 'rgba(1,2,3,0.7)', 'rgba(12%,0,0,1)' ], isColor );
+		} );
+
+		it( 'returns true for hsl() color', () => {
+			testValues( [ 'hsl(0, 100%, 50%)', 'hsl(340,80%,40%)' ], isColor );
+		} );
+
+		it( 'returns true for hsla() color', () => {
+			testValues( [ 'hsla(240, 100%, 50%, 1)', 'hsla(240, 100%, 50%, .05)' ], isColor );
+		} );
+
+		it( 'returns true for currentColor color', () => {
+			testValues( [ 'currentColor' ], isColor );
+		} );
+	} );
+
+	describe( 'isLineStyle()', () => {
+		it( 'returns true for line style', () => {
+			testValues(
+				[ 'none', 'hidden', 'dotted', 'dashed', 'solid', 'double', 'groove', 'ridge', 'inset', 'outset' ],
+				isLineStyle
+			);
+		} );
+	} );
+
+	describe( 'isLength()', () => {
+		it( 'returns true for named widths', () => {
+			testValues(
+				[ 'none', 'hidden', 'dotted', 'dashed', 'solid', 'double', 'groove', 'ridge', 'inset', 'outset' ],
+				isLineStyle
+			);
+		} );
+	} );
+
+	describe( 'isRepeat()', () => {} );
+
+	describe( 'isPosition()', () => {} );
+
+	function testValues( values, callback ) {
+		values.map( string => expect( callback( string ), string ).to.be.true );
+	}
+} );