Browse Source

Merge pull request #44 from ckeditor/t/35

Other: Updated view attribute elements priorities to ensure proper order in which attribute elements are applied. Closes #35.
Piotrek Koszuliński 6 years ago
parent
commit
66cfdf8550

+ 1 - 1
packages/ckeditor5-font/src/fontfamily/utils.js

@@ -72,7 +72,7 @@ function generateFontPreset( fontDefinition ) {
 			styles: {
 				'font-family': cssFontNames
 			},
-			priority: 5
+			priority: 7
 		}
 	};
 }

+ 5 - 5
packages/ckeditor5-font/src/fontsize/utils.js

@@ -30,7 +30,7 @@ const namedPresets = {
 		view: {
 			name: 'span',
 			classes: 'text-tiny',
-			priority: 5
+			priority: 7
 		}
 	},
 	small: {
@@ -39,7 +39,7 @@ const namedPresets = {
 		view: {
 			name: 'span',
 			classes: 'text-small',
-			priority: 5
+			priority: 7
 		}
 	},
 	big: {
@@ -48,7 +48,7 @@ const namedPresets = {
 		view: {
 			name: 'span',
 			classes: 'text-big',
-			priority: 5
+			priority: 7
 		}
 	},
 	huge: {
@@ -57,7 +57,7 @@ const namedPresets = {
 		view: {
 			name: 'span',
 			classes: 'text-huge',
-			priority: 5
+			priority: 7
 		}
 	}
 };
@@ -113,7 +113,7 @@ function generatePixelPreset( size ) {
 			styles: {
 				'font-size': `${ size }px`
 			},
-			priority: 5
+			priority: 7
 		}
 	};
 }

+ 3 - 5
packages/ckeditor5-font/src/utils.js

@@ -84,7 +84,7 @@ export function renderUpcastAttribute( styleAttr ) {
 export function renderDowncastElement( styleAttr ) {
 	return ( modelAttributeValue, viewWriter ) => viewWriter.createAttributeElement( 'span', {
 		style: `${ styleAttr }:${ modelAttributeValue }`
-	} );
+	}, { priority: 7 } );
 }
 
 /**
@@ -190,8 +190,7 @@ function normalizeSingleColorDefinition( color ) {
 				name: 'span',
 				styles: {
 					color
-				},
-				priority: 5
+				}
 			}
 		};
 	} else {
@@ -203,8 +202,7 @@ function normalizeSingleColorDefinition( color ) {
 				name: 'span',
 				styles: {
 					color: `${ color.color }`
-				},
-				priority: 5
+				}
 			}
 		};
 	}

+ 3 - 3
packages/ckeditor5-font/tests/fontfamily/utils.js

@@ -57,7 +57,7 @@ describe( 'FontFamily utils', () => {
 							styles: {
 								'font-family': 'Arial'
 							},
-							priority: 5
+							priority: 7
 						}
 					},
 					{
@@ -68,7 +68,7 @@ describe( 'FontFamily utils', () => {
 							styles: {
 								'font-family': '\'Comic Sans MS\', sans-serif'
 							},
-							priority: 5
+							priority: 7
 						}
 					},
 					{
@@ -79,7 +79,7 @@ describe( 'FontFamily utils', () => {
 							styles: {
 								'font-family': '\'Lucida Console\', \'Courier New\', Courier, monospace'
 							},
-							priority: 5
+							priority: 7
 						}
 					}
 				] );

+ 4 - 4
packages/ckeditor5-font/tests/fontsize/fontsizeui.js

@@ -203,11 +203,11 @@ describe( 'FontSizeUI', () => {
 			it( 'does not alter normalizeOptions() internals', () => {
 				const options = normalizeOptions( [ 'tiny', 'small', 'default', 'big', 'huge' ] );
 				expect( options ).to.deep.equal( [
-					{ title: 'Tiny', model: 'tiny', view: { name: 'span', classes: 'text-tiny', priority: 5 } },
-					{ title: 'Small', model: 'small', view: { name: 'span', classes: 'text-small', priority: 5 } },
+					{ title: 'Tiny', model: 'tiny', view: { name: 'span', classes: 'text-tiny', priority: 7 } },
+					{ title: 'Small', model: 'small', view: { name: 'span', classes: 'text-small', priority: 7 } },
 					{ title: 'Default', model: undefined },
-					{ title: 'Big', model: 'big', view: { name: 'span', classes: 'text-big', priority: 5 } },
-					{ title: 'Huge', model: 'huge', view: { name: 'span', classes: 'text-huge', priority: 5 } }
+					{ title: 'Big', model: 'big', view: { name: 'span', classes: 'text-big', priority: 7 } },
+					{ title: 'Huge', model: 'huge', view: { name: 'span', classes: 'text-huge', priority: 7 } }
 				] );
 			} );
 

+ 8 - 8
packages/ckeditor5-font/tests/fontsize/utils.js

@@ -28,11 +28,11 @@ describe( 'FontSizeEditing Utils', () => {
 		describe( 'named presets', () => {
 			it( 'should return defined presets', () => {
 				expect( normalizeOptions( [ 'tiny', 'small', 'default', 'big', 'huge' ] ) ).to.deep.equal( [
-					{ title: 'Tiny', model: 'tiny', view: { name: 'span', classes: 'text-tiny', priority: 5 } },
-					{ title: 'Small', model: 'small', view: { name: 'span', classes: 'text-small', priority: 5 } },
+					{ title: 'Tiny', model: 'tiny', view: { name: 'span', classes: 'text-tiny', priority: 7 } },
+					{ title: 'Small', model: 'small', view: { name: 'span', classes: 'text-small', priority: 7 } },
 					{ title: 'Default', model: undefined },
-					{ title: 'Big', model: 'big', view: { name: 'span', classes: 'text-big', priority: 5 } },
-					{ title: 'Huge', model: 'huge', view: { name: 'span', classes: 'text-huge', priority: 5 } }
+					{ title: 'Big', model: 'big', view: { name: 'span', classes: 'text-big', priority: 7 } },
+					{ title: 'Huge', model: 'huge', view: { name: 'span', classes: 'text-huge', priority: 7 } }
 				] );
 			} );
 		} );
@@ -40,11 +40,11 @@ describe( 'FontSizeEditing Utils', () => {
 		describe( 'numerical presets', () => {
 			it( 'should return generated presets', () => {
 				expect( normalizeOptions( [ '10', 12, 'default', '14.1', 18.3 ] ) ).to.deep.equal( [
-					{ title: '10', model: 10, view: { name: 'span', styles: { 'font-size': '10px' }, priority: 5 } },
-					{ title: '12', model: 12, view: { name: 'span', styles: { 'font-size': '12px' }, priority: 5 } },
+					{ title: '10', model: 10, view: { name: 'span', styles: { 'font-size': '10px' }, priority: 7 } },
+					{ title: '12', model: 12, view: { name: 'span', styles: { 'font-size': '12px' }, priority: 7 } },
 					{ title: 'Default', model: undefined },
-					{ title: '14.1', model: 14.1, view: { name: 'span', styles: { 'font-size': '14.1px' }, priority: 5 } },
-					{ title: '18.3', model: 18.3, view: { name: 'span', styles: { 'font-size': '18.3px' }, priority: 5 } }
+					{ title: '14.1', model: 14.1, view: { name: 'span', styles: { 'font-size': '14.1px' }, priority: 7 } },
+					{ title: '18.3', model: 18.3, view: { name: 'span', styles: { 'font-size': '18.3px' }, priority: 7 } }
 				] );
 			} );
 		} );

+ 89 - 0
packages/ckeditor5-font/tests/integration.js

@@ -0,0 +1,89 @@
+/**
+ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+/* global document */
+
+import Font from '../src/font';
+import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
+import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
+import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+import env from '@ckeditor/ckeditor5-utils/src/env';
+
+describe( 'Integration test Font', () => {
+	let element, editor, model;
+
+	beforeEach( () => {
+		element = document.createElement( 'div' );
+		document.body.appendChild( element );
+
+		return ClassicTestEditor
+			.create( element, {
+				plugins: [ Font, ArticlePluginSet ]
+			} )
+			.then( newEditor => {
+				editor = newEditor;
+				model = editor.model;
+			} );
+	} );
+
+	afterEach( () => {
+		element.remove();
+
+		return editor.destroy();
+	} );
+
+	describe( 'in-between font plugin features', () => {
+		it( 'should render one span element for all types of font features', () => {
+			setModelData( model,
+				'<paragraph>' +
+					'<$text fontColor="#123456" fontBackgroundColor="rgb(10,20,30)" fontSize="big" fontFamily="Arial">foo</$text>' +
+				'</paragraph>'
+			);
+
+			if ( !env.isEdge ) {
+				expect( editor.getData() ).to.equal(
+					'<p>' +
+						'<span ' +
+							'class="text-big" ' +
+							'style="font-family:Arial, Helvetica, sans-serif;background-color:rgb(10,20,30);color:#123456;"' +
+						'>foo' +
+						'</span>' +
+					'</p>'
+				);
+			} else {
+				// Edge sorts attributes of an element.
+				expect( editor.getData() ).to.equal(
+					'<p>' +
+						'<span ' +
+							'class="text-big" ' +
+							'style="font-family:Arial, Helvetica, sans-serif;color:#123456;background-color:rgb(10,20,30);"' +
+						'>foo' +
+						'</span>' +
+					'</p>'
+				);
+			}
+		} );
+	} );
+
+	describe( 'between font plugin and other', () => {
+		it( 'should render elements wrapped in proper order', () => {
+			setModelData( model,
+				'<paragraph>' +
+					'<$text bold="true" linkHref="foo" fontColor="red" fontSize="big">foo</$text>' +
+				'</paragraph>'
+			);
+
+			expect( editor.getData() ).to.equal(
+				'<p>' +
+					'<a href="foo">' +
+						'<span class="text-big" style="color:red;">' +
+							'<strong>foo</strong>' +
+						'</span>' +
+					'</a>' +
+				'</p>'
+			);
+		} );
+	} );
+} );

+ 83 - 43
packages/ckeditor5-font/tests/utils.js

@@ -7,40 +7,26 @@ import {
 	FONT_COLOR,
 	FONT_BACKGROUND_COLOR,
 	normalizeColorOptions,
-	addColorTableToDropdown
+	addColorTableToDropdown,
+	renderDowncastElement
 } from './../src/utils';
 import { createDropdown } from '@ckeditor/ckeditor5-ui/src/dropdown/utils';
 import ColorTableView from './../src/ui/colortableview';
+import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 
 describe( 'utils', () => {
-	describe( 'color and background color related', () => {
-		it( 'plugin names has proper values', () => {
-			expect( FONT_COLOR ).to.equal( 'fontColor' );
-			expect( FONT_BACKGROUND_COLOR ).to.equal( 'fontBackgroundColor' );
-		} );
+	testUtils.createSinonSandbox();
 
-		it( 'normalizeColorOptions can produce the same output object', () => {
-			const normalizedArray = normalizeColorOptions( [
-				'black',
-				{
-					color: 'black'
-				},
-				{
-					color: 'black',
-					label: 'Black'
-				},
-				{
-					color: 'black',
-					label: 'Black',
-					hasBorder: true
-				},
-				{
-					color: 'black',
-					hasBorder: true
-				}
-			] );
+	it( 'plugin names has proper values', () => {
+		expect( FONT_COLOR ).to.equal( 'fontColor' );
+		expect( FONT_BACKGROUND_COLOR ).to.equal( 'fontBackgroundColor' );
+	} );
+
+	describe( 'normalizeColorOptions()', () => {
+		it( 'should return normalized config object from string', () => {
+			const normalizedOption = normalizeColorOptions( [ 'black' ] );
 
-			expect( normalizedArray ).to.deep.equal( [
+			expect( normalizedOption ).to.deep.equal( [
 				{
 					model: 'black',
 					label: 'black',
@@ -49,10 +35,16 @@ describe( 'utils', () => {
 						name: 'span',
 						styles: {
 							color: 'black'
-						},
-						priority: 5
+						}
 					}
-				},
+				}
+			] );
+		} );
+
+		it( 'should return normalized config object from object( color )', () => {
+			const normalizedOption = normalizeColorOptions( [ { color: 'black' } ] );
+
+			expect( normalizedOption ).to.deep.equal( [
 				{
 					model: 'black',
 					label: 'black',
@@ -61,10 +53,21 @@ describe( 'utils', () => {
 						name: 'span',
 						styles: {
 							color: 'black'
-						},
-						priority: 5
+						}
 					}
-				},
+				}
+			] );
+		} );
+
+		it( 'should return normalized config object from object( color, label )', () => {
+			const normalizedOption = normalizeColorOptions( [
+				{
+					color: 'black',
+					label: 'Black'
+				}
+			] );
+
+			expect( normalizedOption ).to.deep.equal( [
 				{
 					model: 'black',
 					label: 'Black',
@@ -73,10 +76,22 @@ describe( 'utils', () => {
 						name: 'span',
 						styles: {
 							color: 'black'
-						},
-						priority: 5
+						}
 					}
-				},
+				}
+			] );
+		} );
+
+		it( 'should return normalized config object from object( color, label, hasBorder )', () => {
+			const normalizedOption = normalizeColorOptions( [
+				{
+					color: 'black',
+					label: 'Black',
+					hasBorder: true
+				}
+			] );
+
+			expect( normalizedOption ).to.deep.equal( [
 				{
 					model: 'black',
 					label: 'Black',
@@ -85,10 +100,21 @@ describe( 'utils', () => {
 						name: 'span',
 						styles: {
 							color: 'black'
-						},
-						priority: 5
+						}
 					}
-				},
+				}
+			] );
+		} );
+
+		it( 'should return normalized config object from object( color, hasBorder )', () => {
+			const normalizedOption = normalizeColorOptions( [
+				{
+					color: 'black',
+					hasBorder: true
+				}
+			] );
+
+			expect( normalizedOption ).to.deep.equal( [
 				{
 					model: 'black',
 					label: 'black',
@@ -97,14 +123,15 @@ describe( 'utils', () => {
 						name: 'span',
 						styles: {
 							color: 'black'
-						},
-						priority: 5
+						}
 					}
-				},
+				}
 			] );
 		} );
+	} );
 
-		it( 'adding colors table to dropdown works', () => {
+	describe( 'addColorTableToDropdown()', () => {
+		it( 'should create dropdown with color table', () => {
 			const dropdown = createDropdown();
 			dropdown.render();
 
@@ -132,6 +159,19 @@ describe( 'utils', () => {
 
 			expect( dropdown.colorTableView ).to.be.instanceOf( ColorTableView );
 			expect( dropdown.panelView.children.length ).to.equal( 1 );
+			expect( dropdown.colorTableView.element ).to.equal( dropdown.panelView.children.first.element );
+		} );
+	} );
+
+	describe( 'renderDowncastElement()', () => {
+		it( 'should create function executes viewWriter with proper arguments', () => {
+			const downcastViewConverterFn = renderDowncastElement( 'color' );
+			const fake = testUtils.sinon.fake();
+			const fakeViewWriter = { createAttributeElement: fake };
+
+			downcastViewConverterFn( 'blue', fakeViewWriter );
+
+			sinon.assert.calledWithExactly( fake, 'span', { style: 'color:blue' }, { priority: 7 } );
 		} );
 	} );
 } );