8
0
Эх сурвалжийг харах

Remove unnecessary code, add unit tests for color table view.

Mateusz Samsel 6 жил өмнө
parent
commit
e73afa753c

+ 1 - 9
packages/ckeditor5-font/src/ui/colortableview.js

@@ -207,7 +207,7 @@ export default class ColorTableView extends View {
 	initRecentCollection() {
 		for ( let i = 0; i < this.colorColumns; i++ ) {
 			this.recentlyUsedColors.add( {
-				color: 'hsla( 0, 0%, 0%, 0 )',
+				color: 'hsla(0, 0%, 0%, 0)',
 				isEnabled: false,
 				hasBorder: true
 			} );
@@ -225,14 +225,6 @@ export default class ColorTableView extends View {
 			this.focusTracker.add( item.element );
 		}
 
-		this.items.on( 'add', ( evt, item ) => {
-			this.focusTracker.add( item.element );
-		} );
-
-		this.items.on( 'remove', ( evt, item ) => {
-			this.focusTracker.remove( item.element );
-		} );
-
 		// Start listening for the keystrokes coming from #element.
 		this.keystrokes.listenTo( this.element );
 	}

+ 8 - 4
packages/ckeditor5-font/src/ui/colorui.js

@@ -104,11 +104,15 @@ export default class ColorUI extends Plugin {
 
 			dropdownView.bind( 'isEnabled' ).to( command );
 
-			dropdownView.on( 'execute', ( evt, val ) => {
-				if ( val.value !== null ) {
-					colorTableView.recentlyUsedColors.add( { color: val.value, hasBorder: val.hasBorder, label: val.label }, 0 );
+			dropdownView.on( 'execute', ( evt, data ) => {
+				if ( data.value !== null ) {
+					colorTableView.recentlyUsedColors.add( {
+						color: data.value,
+						hasBorder: data.hasBorder,
+						label: data.label },
+					0 );
 				}
-				editor.execute( this.commandName, val );
+				editor.execute( this.commandName, data );
 				editor.editing.view.focus();
 			} );
 

+ 277 - 0
packages/ckeditor5-font/tests/ui/colortableview.js

@@ -0,0 +1,277 @@
+/**
+ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* globals Event */
+
+import ColorTableView from './../../src/ui/colortableview';
+import Collection from '@ckeditor/ckeditor5-utils/src/collection';
+import ViewCollection from '@ckeditor/ckeditor5-ui/src/viewcollection';
+import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker';
+import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler';
+import FocusCycler from '@ckeditor/ckeditor5-ui/src/focuscycler';
+import removeButtonIcon from '@ckeditor/ckeditor5-core/theme/icons/eraser.svg';
+
+import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
+
+describe( 'ColorTableView', () => {
+	let locale, colorTableView;
+	const colorsDefinition = [
+		{
+			color: '#000',
+			label: 'Black',
+			options: {
+				hasBorder: false
+			}
+		}, {
+			color: 'rgb(255, 255, 255)',
+			label: 'White',
+			options: {
+				hasBorder: true
+			}
+		}, {
+			color: 'red',
+			label: 'Red',
+			options: {
+				hasBorder: false
+			}
+		}
+	];
+
+	beforeEach( () => {
+		locale = { t() {} };
+		colorTableView = new ColorTableView( locale, {
+			colors: colorsDefinition,
+			colorColumns: 5,
+			removeButtonTooltip: 'Remove color'
+		} );
+		colorTableView.render();
+	} );
+
+	testUtils.createSinonSandbox();
+
+	describe( 'constructor()', () => {
+		it( 'creates items collection', () => {
+			expect( colorTableView.items ).to.be.instanceOf( ViewCollection );
+		} );
+
+		it( 'creates focus tracker', () => {
+			expect( colorTableView.focusTracker ).to.be.instanceOf( FocusTracker );
+		} );
+
+		it( 'creates keystroke handler', () => {
+			expect( colorTableView.keystrokes ).to.be.instanceOf( KeystrokeHandler );
+		} );
+
+		it( 'creates observable for selected color', () => {
+			expect( colorTableView.selectedColor ).to.be.undefined;
+
+			colorTableView.set( 'selectedColor', 'white' );
+
+			expect( colorTableView.selectedColor ).to.equal( 'white' );
+		} );
+
+		it( 'keep tooltip for remove color button', () => {
+			expect( colorTableView.removeButtonTooltip ).to.equal( 'Remove color' );
+		} );
+
+		it( 'keep number of drawed columns', () => {
+			expect( colorTableView.colorColumns ).to.equal( 5 );
+		} );
+
+		it( 'creaets collection of recently used colors', () => {
+			expect( colorTableView.recentlyUsedColors ).to.be.instanceOf( Collection );
+		} );
+
+		it( 'creates focus cycler', () => {
+			expect( colorTableView._focusCycler ).to.be.instanceOf( FocusCycler );
+		} );
+
+		it( 'has proper class', () => {
+			expect( colorTableView.element.classList.contains( 'ck-color-table' ) ).to.be.true;
+		} );
+	} );
+
+	describe( 'update elements in focus tracker', () => {
+		it( 'focuses the tile in DOM', () => {
+			const spy = sinon.spy( colorTableView._focusCycler, 'focusFirst' );
+
+			colorTableView.focus();
+
+			sinon.assert.calledOnce( spy );
+		} );
+
+		it( 'focuses last the tile in DOM', () => {
+			const spy = sinon.spy( colorTableView._focusCycler, 'focusLast' );
+
+			colorTableView.focusLast();
+
+			sinon.assert.calledOnce( spy );
+		} );
+	} );
+
+	describe( 'remove color button', () => {
+		let removeButton;
+
+		beforeEach( () => {
+			removeButton = colorTableView.items.first;
+		} );
+
+		it( 'has proper class', () => {
+			expect( removeButton.element.classList.contains( 'ck-color-table__remove-color' ) ).to.be.true;
+		} );
+
+		it( 'has proper setting', () => {
+			expect( removeButton.withText ).to.be.true;
+			expect( removeButton.icon ).to.equal( removeButtonIcon );
+			expect( removeButton.tooltip ).to.be.true;
+			expect( removeButton.label ).to.equal( 'Remove color' );
+		} );
+
+		it( 'executes event with "null" value', () => {
+			const spy = sinon.spy();
+			colorTableView.on( 'execute', spy );
+
+			removeButton.element.dispatchEvent( new Event( 'click' ) );
+
+			sinon.assert.calledOnce( spy );
+			sinon.assert.calledWith( spy, sinon.match.any, { value: null } );
+		} );
+	} );
+
+	describe( 'static colors grid', () => {
+		let staticColorTable;
+		beforeEach( () => {
+			staticColorTable = colorTableView.items.get( 1 );
+		} );
+
+		it( 'has added 3 children from definition', () => {
+			expect( staticColorTable.items.length ).to.equal( 3 );
+		} );
+
+		colorsDefinition.forEach( ( item, index ) => {
+			it( `dispatch event to parent element for color: ${ item.color }`, () => {
+				const spy = sinon.spy();
+				colorTableView.on( 'execute', spy );
+
+				staticColorTable.items.get( index ).element.dispatchEvent( new Event( 'click' ) );
+
+				sinon.assert.calledOnce( spy );
+				sinon.assert.calledWith( spy, sinon.match.any, {
+					value: item.color,
+					label: item.label,
+					hasBorder: item.options.hasBorder
+				} );
+			} );
+		} );
+	} );
+
+	describe( 'recent colors grid', () => {
+		const colorBlack = {
+			color: '#000000',
+			label: 'Black',
+			hasBorder: false
+		};
+		const colorWhite = {
+			color: '#FFFFFF',
+			label: 'Black',
+			hasBorder: true
+		};
+		const colorRed = {
+			color: 'rgb(255,0,0)',
+			hasBorder: false
+		};
+		const emptyColor = {
+			color: 'hsla(0, 0%, 0%, 0)',
+			isEnabled: false,
+			hasBorder: true
+		};
+		let recentColorsGridView, recentColorModel;
+
+		beforeEach( () => {
+			recentColorModel = colorTableView.recentlyUsedColors;
+			recentColorsGridView = colorTableView.items.last;
+		} );
+
+		describe( 'initialization', () => {
+			it( 'has proper length of populated items', () => {
+				expect( recentColorModel.length ).to.equal( 5 );
+			} );
+			for ( let i = 0; i < 5; i++ ) {
+				it( `initialized item with index: "${ i }" has proper attributes`, () => {
+					const modelItem = recentColorModel.get( i );
+					const viewItem = recentColorsGridView.items.get( i );
+					expect( modelItem.color ).to.equal( 'hsla(0, 0%, 0%, 0)' );
+					expect( modelItem.isEnabled ).to.be.false;
+					expect( modelItem.hasBorder ).to.be.true;
+					expect( viewItem.isEnabled ).to.be.false;
+					expect( viewItem.color ).to.equal( 'hsla(0, 0%, 0%, 0)' );
+					expect( viewItem.hasBorder ).to.be.true;
+					expect( viewItem.label ).to.be.undefined;
+				} );
+			}
+		} );
+
+		describe( 'model manipulation', () => {
+			it( 'adding item will preserve length of collection', () => {
+				expect( recentColorModel.length ).to.equal( 5 );
+
+				recentColorModel.add( Object.assign( {}, colorBlack ), 0 );
+
+				expect( recentColorModel.length ).to.equal( 5 );
+				expect( recentColorModel.first.color ).to.equal( '#000000' );
+				expect( recentColorModel.first.label ).to.equal( 'Black' );
+				expect( recentColorModel.first.hasBorder ).to.be.false;
+			} );
+
+			it( 'adding multiple times same color should not work', () => {
+				recentColorModel.add( Object.assign( {}, colorBlack ), 0 );
+
+				expect( recentColorModel.first ).to.own.include( colorBlack );
+				expect( recentColorModel.get( 1 ) ).to.own.include( emptyColor );
+
+				recentColorModel.add( Object.assign( {}, colorBlack ), 0 );
+
+				expect( recentColorModel.first ).to.own.include( colorBlack );
+				expect( recentColorModel.get( 1 ) ).to.own.include( emptyColor );
+			} );
+
+			it( 'adding duplicates move color to the front', () => {
+				recentColorModel.add( Object.assign( {}, colorBlack ), 0 );
+				recentColorModel.add( Object.assign( {}, colorWhite ), 0 );
+				recentColorModel.add( Object.assign( {}, colorRed ), 0 );
+
+				expect( recentColorModel.get( 0 ) ).to.own.include( colorRed );
+				expect( recentColorModel.get( 1 ) ).to.own.include( colorWhite );
+				expect( recentColorModel.get( 2 ) ).to.own.include( colorBlack );
+				expect( recentColorModel.get( 3 ) ).to.own.include( emptyColor );
+
+				recentColorModel.add( Object.assign( {}, colorBlack ), 0 );
+
+				expect( recentColorModel.get( 0 ) ).to.own.include( colorBlack );
+				expect( recentColorModel.get( 1 ) ).to.own.include( colorRed );
+				expect( recentColorModel.get( 2 ) ).to.own.include( colorWhite );
+				expect( recentColorModel.get( 3 ) ).to.own.include( emptyColor );
+			} );
+		} );
+
+		describe( 'events', () => {
+			it( 'added colors delegates eexecute to parent', () => {
+				const spy = sinon.spy();
+				colorTableView.on( 'execute', spy );
+
+				recentColorModel.add( Object.assign( {}, colorBlack ), 0 );
+
+				recentColorsGridView.items.first.element.dispatchEvent( new Event( 'click' ) );
+
+				sinon.assert.calledOnce( spy );
+				sinon.assert.calledWith( spy, sinon.match.any, {
+					value: '#000000',
+					label: 'Black',
+					hasBorder: false
+				} );
+			} );
+		} );
+	} );
+} );