瀏覽代碼

Merge pull request #238 from ckeditor/i/6131

Internal: Table and table cell property form fields should be validated. Closes ckeditor/ckeditor5#6131.
Maciej 5 年之前
父節點
當前提交
01714c4787

+ 93 - 8
packages/ckeditor5-table/src/tablecellproperties/tablecellpropertiesui.js

@@ -14,11 +14,20 @@ import clickOutsideHandler from '@ckeditor/ckeditor5-ui/src/bindings/clickoutsid
 import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon';
 import TableCellPropertiesView from './ui/tablecellpropertiesview';
 import tableCellProperties from './../../theme/icons/table-cell-properties.svg';
-import { repositionContextualBalloon, getBalloonCellPositionData } from '../ui/utils';
+import {
+	repositionContextualBalloon,
+	getBalloonCellPositionData,
+	getLocalizedColorErrorText,
+	getLocalizedLengthErrorText,
+	colorFieldValidator,
+	lengthFieldValidator
+} from '../ui/utils';
+import { debounce } from 'lodash-es';
 
 const DEFAULT_BORDER_STYLE = 'none';
 const DEFAULT_HORIZONTAL_ALIGNMENT = 'left';
 const DEFAULT_VERTICAL_ALIGNMENT = 'middle';
+const ERROR_TEXT_TIMEOUT = 500;
 
 /**
  * The table cell properties UI plugin. It introduces the `'tableCellProperties'` button
@@ -112,6 +121,7 @@ export default class TableCellPropertiesUI extends Plugin {
 		const editor = this.editor;
 		const viewDocument = editor.editing.view.document;
 		const view = new TableCellPropertiesView( editor.locale );
+		const t = editor.t;
 
 		// Render the view so its #element is available for the clickOutsideHandler.
 		view.render();
@@ -148,17 +158,58 @@ export default class TableCellPropertiesUI extends Plugin {
 			callback: () => this._hideView()
 		} );
 
+		const colorErrorText = getLocalizedColorErrorText( t );
+		const lengthErrorText = getLocalizedLengthErrorText( t );
+
 		// Create the "UI -> editor data" binding.
 		// These listeners update the editor data (via table commands) when any observable
-		// property of the view has changed. This makes the view live, which means the changes are
+		// property of the view has changed. They also validate the value and display errors in the UI
+		// when necessary. This makes the view live, which means the changes are
 		// visible in the editing as soon as the user types or changes fields' values.
 		view.on( 'change:borderStyle', this._getPropertyChangeCallback( 'tableCellBorderStyle' ) );
-		view.on( 'change:borderColor', this._getPropertyChangeCallback( 'tableCellBorderColor' ) );
-		view.on( 'change:borderWidth', this._getPropertyChangeCallback( 'tableCellBorderWidth' ) );
-		view.on( 'change:padding', this._getPropertyChangeCallback( 'tableCellPadding' ) );
-		view.on( 'change:backgroundColor', this._getPropertyChangeCallback( 'tableCellBackgroundColor' ) );
-		view.on( 'change:width', this._getPropertyChangeCallback( 'tableCellWidth' ) );
-		view.on( 'change:height', this._getPropertyChangeCallback( 'tableCellHeight' ) );
+
+		view.on( 'change:borderColor', this._getValidatedPropertyChangeCallback( {
+			viewField: view.borderColorInput,
+			commandName: 'tableCellBorderColor',
+			errorText: colorErrorText,
+			validator: colorFieldValidator
+		} ) );
+
+		view.on( 'change:borderWidth', this._getValidatedPropertyChangeCallback( {
+			viewField: view.borderWidthInput,
+			commandName: 'tableCellBorderWidth',
+			errorText: lengthErrorText,
+			validator: lengthFieldValidator
+		} ) );
+
+		view.on( 'change:padding', this._getValidatedPropertyChangeCallback( {
+			viewField: view.paddingInput,
+			commandName: 'tableCellPadding',
+			errorText: lengthErrorText,
+			validator: lengthFieldValidator
+		} ) );
+
+		view.on( 'change:width', this._getValidatedPropertyChangeCallback( {
+			viewField: view.widthInput,
+			commandName: 'tableCellWidth',
+			errorText: lengthErrorText,
+			validator: lengthFieldValidator
+		} ) );
+
+		view.on( 'change:height', this._getValidatedPropertyChangeCallback( {
+			viewField: view.heightInput,
+			commandName: 'tableCellHeight',
+			errorText: lengthErrorText,
+			validator: lengthFieldValidator
+		} ) );
+
+		view.on( 'change:backgroundColor', this._getValidatedPropertyChangeCallback( {
+			viewField: view.backgroundInput,
+			commandName: 'tableCellBackgroundColor',
+			errorText: colorErrorText,
+			validator: colorFieldValidator
+		} ) );
+
 		view.on( 'change:horizontalAlignment', this._getPropertyChangeCallback( 'tableCellHorizontalAlignment' ) );
 		view.on( 'change:verticalAlignment', this._getPropertyChangeCallback( 'tableCellVerticalAlignment' ) );
 
@@ -279,4 +330,38 @@ export default class TableCellPropertiesUI extends Plugin {
 			} );
 		};
 	}
+
+	/**
+	 * Creates a callback that when executed upon {@link #view view's} property change:
+	 * * executes a related editor command with the new property value if the value is valid,
+	 * * or sets the error text next to the invalid field, if the value did not pass the validation.
+	 *
+	 * @private
+	 * @param {Object} options
+	 * @param {String} options.commandName
+	 * @param {module:ui/view~View} options.viewField
+	 * @param {Function} options.validator
+	 * @param {String} options.errorText
+	 * @returns {Function}
+	 */
+	_getValidatedPropertyChangeCallback( { commandName, viewField, validator, errorText } ) {
+		const setErrorTextDebounced = debounce( () => {
+			viewField.errorText = errorText;
+		}, ERROR_TEXT_TIMEOUT );
+
+		return ( evt, propertyName, newValue ) => {
+			setErrorTextDebounced.cancel();
+
+			if ( validator( newValue ) ) {
+				this.editor.execute( commandName, {
+					value: newValue,
+					batch: this._undoStepBatch
+				} );
+
+				viewField.errorText = null;
+			} else {
+				setErrorTextDebounced();
+			}
+		};
+	}
 }

+ 15 - 2
packages/ckeditor5-table/src/tablecellproperties/ui/tablecellpropertiesview.js

@@ -65,7 +65,6 @@ export default class TableCellPropertiesView extends View {
 		const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields();
 		const { widthInput, operatorLabel, heightInput, dimensionsLabel } = this._createDimensionFields();
 		const { horizontalAlignmentToolbar, verticalAlignmentToolbar, alignmentLabel } = this._createAlignmentFields();
-		const { saveButtonView, cancelButtonView } = this._createActionButtons();
 
 		/**
 		 * Tracks information about the DOM focus in the form.
@@ -246,6 +245,11 @@ export default class TableCellPropertiesView extends View {
 		 */
 		this.verticalAlignmentToolbar = verticalAlignmentToolbar;
 
+		// Defer creating to make sure other fields are present and the Save button can
+		// bind its #isEnabled to their error messages so there's no way to save unless all
+		// fields are valid.
+		const { saveButtonView, cancelButtonView } = this._createActionButtons();
+
 		/**
 		 * The "Save" button view.
 		 *
@@ -691,9 +695,14 @@ export default class TableCellPropertiesView extends View {
 	_createActionButtons() {
 		const locale = this.locale;
 		const t = this.t;
-
 		const saveButtonView = new ButtonView( locale );
 		const cancelButtonView = new ButtonView( locale );
+		const fieldsThatShouldValidateToSave = [
+			this.borderWidthInput,
+			this.borderColorInput,
+			this.backgroundInput,
+			this.paddingInput
+		];
 
 		saveButtonView.set( {
 			label: t( 'Save' ),
@@ -703,6 +712,10 @@ export default class TableCellPropertiesView extends View {
 			withText: true,
 		} );
 
+		saveButtonView.bind( 'isEnabled' ).toMany( fieldsThatShouldValidateToSave, 'errorText', ( ...errorTexts ) => {
+			return errorTexts.every( errorText => !errorText );
+		} );
+
 		cancelButtonView.set( {
 			label: t( 'Cancel' ),
 			icon: cancelIcon,

+ 83 - 7
packages/ckeditor5-table/src/tableproperties/tablepropertiesui.js

@@ -16,11 +16,17 @@ import TablePropertiesView from './ui/tablepropertiesview';
 import tableProperties from './../../theme/icons/table-properties.svg';
 import {
 	repositionContextualBalloon,
-	getBalloonTablePositionData
+	getBalloonTablePositionData,
+	getLocalizedColorErrorText,
+	getLocalizedLengthErrorText,
+	colorFieldValidator,
+	lengthFieldValidator
 } from '../ui/utils';
+import { debounce } from 'lodash-es';
 
 const DEFAULT_BORDER_STYLE = 'none';
 const DEFAULT_ALIGNMENT = 'center';
+const ERROR_TEXT_TIMEOUT = 500;
 
 /**
  * The table properties UI plugin. It introduces the `'tableProperties'` button
@@ -114,6 +120,7 @@ export default class TablePropertiesUI extends Plugin {
 		const editor = this.editor;
 		const viewDocument = editor.editing.view.document;
 		const view = new TablePropertiesView( editor.locale );
+		const t = editor.t;
 
 		// Render the view so its #element is available for the clickOutsideHandler.
 		view.render();
@@ -150,16 +157,51 @@ export default class TablePropertiesUI extends Plugin {
 			callback: () => this._hideView()
 		} );
 
+		const colorErrorText = getLocalizedColorErrorText( t );
+		const lengthErrorText = getLocalizedLengthErrorText( t );
+
 		// Create the "UI -> editor data" binding.
 		// These listeners update the editor data (via table commands) when any observable
-		// property of the view has changed. This makes the view live, which means the changes are
+		// property of the view has changed. They also validate the value and display errors in the UI
+		// when necessary. This makes the view live, which means the changes are
 		// visible in the editing as soon as the user types or changes fields' values.
 		view.on( 'change:borderStyle', this._getPropertyChangeCallback( 'tableBorderStyle' ) );
-		view.on( 'change:borderColor', this._getPropertyChangeCallback( 'tableBorderColor' ) );
-		view.on( 'change:borderWidth', this._getPropertyChangeCallback( 'tableBorderWidth' ) );
-		view.on( 'change:backgroundColor', this._getPropertyChangeCallback( 'tableBackgroundColor' ) );
-		view.on( 'change:width', this._getPropertyChangeCallback( 'tableWidth' ) );
-		view.on( 'change:height', this._getPropertyChangeCallback( 'tableHeight' ) );
+
+		view.on( 'change:borderColor', this._getValidatedPropertyChangeCallback( {
+			viewField: view.borderColorInput,
+			commandName: 'tableBorderColor',
+			errorText: colorErrorText,
+			validator: colorFieldValidator
+		} ) );
+
+		view.on( 'change:borderWidth', this._getValidatedPropertyChangeCallback( {
+			viewField: view.borderWidthInput,
+			commandName: 'tableBorderWidth',
+			errorText: lengthErrorText,
+			validator: lengthFieldValidator
+		} ) );
+
+		view.on( 'change:backgroundColor', this._getValidatedPropertyChangeCallback( {
+			viewField: view.backgroundInput,
+			commandName: 'tableBackgroundColor',
+			errorText: colorErrorText,
+			validator: colorFieldValidator
+		} ) );
+
+		view.on( 'change:width', this._getValidatedPropertyChangeCallback( {
+			viewField: view.widthInput,
+			commandName: 'tableWidth',
+			errorText: lengthErrorText,
+			validator: lengthFieldValidator
+		} ) );
+
+		view.on( 'change:height', this._getValidatedPropertyChangeCallback( {
+			viewField: view.heightInput,
+			commandName: 'tableHeight',
+			errorText: lengthErrorText,
+			validator: lengthFieldValidator
+		} ) );
+
 		view.on( 'change:alignment', this._getPropertyChangeCallback( 'tableAlignment' ) );
 
 		return view;
@@ -277,4 +319,38 @@ export default class TablePropertiesUI extends Plugin {
 			} );
 		};
 	}
+
+	/**
+	 * Creates a callback that when executed upon {@link #view view's} property change:
+	 * * executes a related editor command with the new property value if the value is valid,
+	 * * or sets the error text next to the invalid field, if the value did not pass the validation.
+	 *
+	 * @private
+	 * @param {Object} options
+	 * @param {String} options.commandName
+	 * @param {module:ui/view~View} options.viewField
+	 * @param {Function} options.validator
+	 * @param {String} options.errorText
+	 * @returns {Function}
+	 */
+	_getValidatedPropertyChangeCallback( { commandName, viewField, validator, errorText } ) {
+		const setErrorTextDebounced = debounce( () => {
+			viewField.errorText = errorText;
+		}, ERROR_TEXT_TIMEOUT );
+
+		return ( evt, propertyName, newValue ) => {
+			setErrorTextDebounced.cancel();
+
+			if ( validator( newValue ) ) {
+				this.editor.execute( commandName, {
+					value: newValue,
+					batch: this._undoStepBatch
+				} );
+
+				viewField.errorText = null;
+			} else {
+				setErrorTextDebounced();
+			}
+		};
+	}
 }

+ 16 - 1
packages/ckeditor5-table/src/tableproperties/ui/tablepropertiesview.js

@@ -57,7 +57,6 @@ export default class TablePropertiesView extends View {
 		const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields();
 		const { widthInput, operatorLabel, heightInput, dimensionsLabel } = this._createDimensionFields();
 		const { alignmentToolbar, alignmentLabel } = this._createAlignmentFields();
-		const { saveButtonView, cancelButtonView } = this._createActionButtons();
 
 		/**
 		 * Tracks information about the DOM focus in the form.
@@ -204,6 +203,11 @@ export default class TablePropertiesView extends View {
 		 */
 		this.alignmentToolbar = alignmentToolbar;
 
+		// Defer creating to make sure other fields are present and the Save button can
+		// bind its #isEnabled to their error messages so there's no way to save unless all
+		// fields are valid.
+		const { saveButtonView, cancelButtonView } = this._createActionButtons();
+
 		/**
 		 * The "Save" button view.
 		 *
@@ -594,6 +598,13 @@ export default class TablePropertiesView extends View {
 
 		const saveButtonView = new ButtonView( locale );
 		const cancelButtonView = new ButtonView( locale );
+		const fieldsThatShouldValidateToSave = [
+			this.borderWidthInput,
+			this.borderColorInput,
+			this.backgroundInput,
+			this.widthInput,
+			this.heightInput
+		];
 
 		saveButtonView.set( {
 			label: t( 'Save' ),
@@ -603,6 +614,10 @@ export default class TablePropertiesView extends View {
 			withText: true,
 		} );
 
+		saveButtonView.bind( 'isEnabled' ).toMany( fieldsThatShouldValidateToSave, 'errorText', ( ...errorTexts ) => {
+			return errorTexts.every( errorText => !errorText );
+		} );
+
 		cancelButtonView.set( {
 			label: t( 'Cancel' ),
 			icon: cancelIcon,

+ 57 - 0
packages/ckeditor5-table/src/ui/utils.js

@@ -11,6 +11,7 @@ import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpa
 import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview';
 import Collection from '@ckeditor/ckeditor5-utils/src/collection';
 import Model from '@ckeditor/ckeditor5-ui/src/model';
+import { isColor, isLength } from '@ckeditor/ckeditor5-engine/src/view/styles/utils';
 import { getTableWidgetAncestor } from '../utils';
 import { findAncestor } from '../commands/utils';
 
@@ -24,6 +25,8 @@ const BALLOON_POSITIONS = [
 	DEFAULT_BALLOON_POSITIONS.southArrowNorthEast
 ];
 
+const isEmpty = val => val === '';
+
 /**
  * A helper utility that positions the
  * {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon contextual balloon} instance
@@ -111,6 +114,60 @@ export function getBorderStyleLabels( t ) {
 }
 
 /**
+ * Returns a localized error string that can be displayed next to color (background, border)
+ * fields that have an invalid value.
+ *
+ * @param {module:utils/locale~Locale#t} t The "t" function provided by the editor
+ * that is used to localize strings.
+ * @returns {String}
+ */
+export function getLocalizedColorErrorText( t ) {
+	return t( 'The color is invalid. Try "#FF0000" or "rgb(255,0,0)" or "red".' );
+}
+
+/**
+ * Returns a localized error string that can be displayed next to length (padding, border width)
+ * fields that have an invalid value.
+ *
+ * @param {module:utils/locale~Locale#t} t The "t" function provided by the editor
+ * that is used to localize strings.
+ * @returns {String}
+ */
+export function getLocalizedLengthErrorText( t ) {
+	return t( 'The value is invalid. Try "10px" or "2em" or simply "2".' );
+}
+
+/**
+ * Returns `true` when the passed value is an empty string or a valid CSS color expression.
+ * Otherwise, `false` is returned.
+ *
+ * See {@link module:engine/view/styles/utils~isColor}.
+ *
+ * @param {String} value
+ * @returns {Boolean}
+ */
+export function colorFieldValidator( value ) {
+	value = value.trim();
+
+	return isEmpty( value ) || isColor( value );
+}
+
+/**
+ * Returns `true` when the passed value is an empty string or a valid CSS length expression.
+ * Otherwise, `false` is returned.
+ *
+ * See {@link module:engine/view/styles/utils~isLength}.
+ *
+ * @param {String} value
+ * @returns {Boolean}
+ */
+export function lengthFieldValidator( value ) {
+	value = value.trim();
+
+	return isEmpty( value ) || isLength( value );
+}
+
+/**
  * Generates item definitions for a UI dropdown that allows changing the border style of a table or a table cell.
  *
  * @param {module:table/tablecellproperties/ui/tablecellpropertiesview~TableCellPropertiesView|

+ 238 - 8
packages/ckeditor5-table/tests/tablecellproperties/tablecellpropertiesui.js

@@ -24,11 +24,13 @@ import TableCellPropertiesUIView from '../../src/tablecellproperties/ui/tablecel
 describe( 'table cell properties', () => {
 	describe( 'TableCellPropertiesUI', () => {
 		let editor, editorElement, contextualBalloon,
-			tableCellPropertiesUI, tableCellPropertiesView, tableCellPropertiesButton;
+			tableCellPropertiesUI, tableCellPropertiesView, tableCellPropertiesButton,
+			clock;
 
 		testUtils.createSinonSandbox();
 
 		beforeEach( () => {
+			clock = sinon.useFakeTimers();
 			editorElement = document.createElement( 'div' );
 			document.body.appendChild( editorElement );
 
@@ -52,6 +54,7 @@ describe( 'table cell properties', () => {
 		} );
 
 		afterEach( () => {
+			clock.restore();
 			editorElement.remove();
 
 			return editor.destroy();
@@ -215,14 +218,242 @@ describe( 'table cell properties', () => {
 			} );
 
 			describe( 'property changes', () => {
-				it( 'should affect the editor state', () => {
-					const spy = testUtils.sinon.stub( editor, 'execute' );
-
+				beforeEach( () => {
 					tableCellPropertiesUI._undoStepBatch = 'foo';
-					tableCellPropertiesView.borderStyle = 'dotted';
+				} );
 
-					sinon.assert.calledOnce( spy );
-					sinon.assert.calledWithExactly( spy, 'tableCellBorderStyle', { value: 'dotted', batch: 'foo' } );
+				describe( '#borderStyle', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tableCellPropertiesView.borderStyle = 'dotted';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableCellBorderStyle', { value: 'dotted', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#borderColor', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tableCellPropertiesView.borderColor = '#FFAAFF';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableCellBorderColor', { value: '#FFAAFF', batch: 'foo' } );
+					} );
+
+					it( 'should display an error message if value is invalid', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						// First, let's pass an invalid value and check what happens.
+						tableCellPropertiesView.borderColor = '42';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.borderColorInput.errorText ).to.match( /^The color is invalid/ );
+						sinon.assert.notCalled( spy );
+
+						// And now let's pass a valid value and check if the error text will be gone.
+						tableCellPropertiesView.borderColor = '#AAA';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.borderColorInput.errorText ).to.be.null;
+						sinon.assert.calledWithExactly( spy, 'tableCellBorderColor', { value: '#AAA', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#borderWidth', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tableCellPropertiesView.borderWidth = '12px';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableCellBorderWidth', { value: '12px', batch: 'foo' } );
+					} );
+
+					it( 'should display an error message if value is invalid', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						// First, let's pass an invalid value and check what happens.
+						tableCellPropertiesView.borderWidth = 'wrong';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.borderWidthInput.errorText ).to.match( /^The value is invalid/ );
+						sinon.assert.notCalled( spy );
+
+						// And now let's pass a valid value and check if the error text will be gone.
+						tableCellPropertiesView.borderWidth = '3em';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.backgroundInput.errorText ).to.be.null;
+						sinon.assert.calledWithExactly( spy, 'tableCellBorderWidth', { value: '3em', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#width', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tableCellPropertiesView.width = '12px';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableCellWidth', { value: '12px', batch: 'foo' } );
+					} );
+
+					it( 'should display an error message if value is invalid', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						// First, let's pass an invalid value and check what happens.
+						tableCellPropertiesView.width = 'wrong';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.widthInput.errorText ).to.match( /^The value is invalid/ );
+						sinon.assert.notCalled( spy );
+
+						// And now let's pass a valid value and check if the error text will be gone.
+						tableCellPropertiesView.width = '3em';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.backgroundInput.errorText ).to.be.null;
+						sinon.assert.calledWithExactly( spy, 'tableCellWidth', { value: '3em', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#height', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tableCellPropertiesView.height = '12px';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableCellHeight', { value: '12px', batch: 'foo' } );
+					} );
+
+					it( 'should display an error message if value is invalid', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						// First, let's pass an invalid value and check what happens.
+						tableCellPropertiesView.height = 'wrong';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.heightInput.errorText ).to.match( /^The value is invalid/ );
+						sinon.assert.notCalled( spy );
+
+						// And now let's pass a valid value and check if the error text will be gone.
+						tableCellPropertiesView.height = '3em';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.backgroundInput.errorText ).to.be.null;
+						sinon.assert.calledWithExactly( spy, 'tableCellHeight', { value: '3em', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#padding', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tableCellPropertiesView.padding = '12px';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableCellPadding', { value: '12px', batch: 'foo' } );
+					} );
+
+					it( 'should display an error message if value is invalid', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						// First, let's pass an invalid value and check what happens.
+						tableCellPropertiesView.padding = 'wrong';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.paddingInput.errorText ).to.match( /^The value is invalid/ );
+						sinon.assert.notCalled( spy );
+
+						// And now let's pass a valid value and check if the error text will be gone.
+						tableCellPropertiesView.padding = '3em';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.backgroundInput.errorText ).to.be.null;
+						sinon.assert.calledWithExactly( spy, 'tableCellPadding', { value: '3em', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#backgroundColor', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tableCellPropertiesView.backgroundColor = '#FFAAFF';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableCellBackgroundColor', { value: '#FFAAFF', batch: 'foo' } );
+					} );
+
+					it( 'should display an error message if value is invalid', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						// First, let's pass an invalid value and check what happens.
+						tableCellPropertiesView.backgroundColor = '42';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.backgroundInput.errorText ).to.match( /^The color is invalid/ );
+						sinon.assert.notCalled( spy );
+
+						// And now let's pass a valid value and check if the error text will be gone.
+						tableCellPropertiesView.backgroundColor = '#AAA';
+
+						clock.tick( 500 );
+
+						expect( tableCellPropertiesView.backgroundInput.errorText ).to.be.null;
+						sinon.assert.calledWithExactly( spy, 'tableCellBackgroundColor', { value: '#AAA', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#horizontalAlignment', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tableCellPropertiesView.horizontalAlignment = 'right';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableCellHorizontalAlignment', { value: 'right', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#verticalAlignment', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tableCellPropertiesView.verticalAlignment = 'right';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableCellVerticalAlignment', { value: 'right', batch: 'foo' } );
+					} );
+				} );
+
+				it( 'should not display an error text if user managed to fix the value before the UI timeout', () => {
+					// First, let's pass an invalid value.
+					tableCellPropertiesView.borderColor = '#';
+
+					clock.tick( 100 );
+
+					// Then the user managed to quickly type the correct value.
+					tableCellPropertiesView.borderColor = '#aaa';
+
+					clock.tick( 400 );
+
+					// Because they were quick, they should see no error
+					expect( tableCellPropertiesView.borderColorInput.errorText ).to.be.null;
 				} );
 
 				it( 'should not affect the editor state if internal property has changed', () => {
@@ -230,7 +461,6 @@ describe( 'table cell properties', () => {
 
 					tableCellPropertiesView.set( 'internalProp', 'foo' );
 
-					tableCellPropertiesUI._undoStepBatch = 'foo';
 					tableCellPropertiesView.internalProp = 'bar';
 
 					sinon.assert.notCalled( spy );

+ 37 - 0
packages/ckeditor5-table/tests/tablecellproperties/ui/tablecellpropertiesview.js

@@ -500,6 +500,43 @@ describe( 'table cell properties', () => {
 
 						expect( spy.calledOnce ).to.be.true;
 					} );
+
+					it( 'should make sure the #saveButtonView is disabled until text fields are without errors', () => {
+						view.borderWidthInput.errorText = 'foo';
+						view.borderColorInput.errorText = 'foo';
+						view.backgroundInput.errorText = 'foo';
+						view.paddingInput.errorText = 'foo';
+
+						expect( view.saveButtonView.isEnabled ).to.be.false;
+
+						view.borderWidthInput.errorText = 'foo';
+						view.borderColorInput.errorText = 'foo';
+						view.backgroundInput.errorText = 'foo';
+						view.paddingInput.errorText = null;
+
+						expect( view.saveButtonView.isEnabled ).to.be.false;
+
+						view.borderWidthInput.errorText = 'foo';
+						view.borderColorInput.errorText = 'foo';
+						view.backgroundInput.errorText = null;
+						view.paddingInput.errorText = null;
+
+						expect( view.saveButtonView.isEnabled ).to.be.false;
+
+						view.borderWidthInput.errorText = 'foo';
+						view.borderColorInput.errorText = null;
+						view.backgroundInput.errorText = null;
+						view.paddingInput.errorText = null;
+
+						expect( view.saveButtonView.isEnabled ).to.be.false;
+
+						view.borderWidthInput.errorText = null;
+						view.borderColorInput.errorText = null;
+						view.backgroundInput.errorText = null;
+						view.paddingInput.errorText = null;
+
+						expect( view.saveButtonView.isEnabled ).to.be.true;
+					} );
 				} );
 			} );
 

+ 196 - 8
packages/ckeditor5-table/tests/tableproperties/tablepropertiesui.js

@@ -24,11 +24,13 @@ import TablePropertiesUIView from '../../src/tableproperties/ui/tablepropertiesv
 describe( 'table properties', () => {
 	describe( 'TablePropertiesUI', () => {
 		let editor, editorElement, contextualBalloon,
-			tablePropertiesUI, tablePropertiesView, tablePropertiesButton;
+			tablePropertiesUI, tablePropertiesView, tablePropertiesButton,
+			clock;
 
 		testUtils.createSinonSandbox();
 
 		beforeEach( () => {
+			clock = sinon.useFakeTimers();
 			editorElement = document.createElement( 'div' );
 			document.body.appendChild( editorElement );
 
@@ -52,6 +54,7 @@ describe( 'table properties', () => {
 		} );
 
 		afterEach( () => {
+			clock.restore();
 			editorElement.remove();
 
 			return editor.destroy();
@@ -215,14 +218,200 @@ describe( 'table properties', () => {
 			} );
 
 			describe( 'property changes', () => {
-				it( 'should affect the editor state', () => {
-					const spy = testUtils.sinon.stub( editor, 'execute' );
-
+				beforeEach( () => {
 					tablePropertiesUI._undoStepBatch = 'foo';
-					tablePropertiesView.borderStyle = 'dotted';
+				} );
 
-					sinon.assert.calledOnce( spy );
-					sinon.assert.calledWithExactly( spy, 'tableBorderStyle', { value: 'dotted', batch: 'foo' } );
+				describe( '#borderStyle', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tablePropertiesView.borderStyle = 'dotted';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableBorderStyle', { value: 'dotted', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#borderColor', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tablePropertiesView.borderColor = '#FFAAFF';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableBorderColor', { value: '#FFAAFF', batch: 'foo' } );
+					} );
+
+					it( 'should display an error message if value is invalid', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						// First, let's pass an invalid value and check what happens.
+						tablePropertiesView.borderColor = '42';
+
+						clock.tick( 500 );
+
+						expect( tablePropertiesView.borderColorInput.errorText ).to.match( /^The color is invalid/ );
+						sinon.assert.notCalled( spy );
+
+						// And now let's pass a valid value and check if the error text will be gone.
+						tablePropertiesView.borderColor = '#AAA';
+
+						clock.tick( 500 );
+
+						expect( tablePropertiesView.borderColorInput.errorText ).to.be.null;
+						sinon.assert.calledWithExactly( spy, 'tableBorderColor', { value: '#AAA', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#borderWidth', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tablePropertiesView.borderWidth = '12px';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableBorderWidth', { value: '12px', batch: 'foo' } );
+					} );
+
+					it( 'should display an error message if value is invalid', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						// First, let's pass an invalid value and check what happens.
+						tablePropertiesView.borderWidth = 'wrong';
+
+						clock.tick( 500 );
+
+						expect( tablePropertiesView.borderWidthInput.errorText ).to.match( /^The value is invalid/ );
+						sinon.assert.notCalled( spy );
+
+						// And now let's pass a valid value and check if the error text will be gone.
+						tablePropertiesView.borderWidth = '3em';
+
+						clock.tick( 500 );
+
+						expect( tablePropertiesView.backgroundInput.errorText ).to.be.null;
+						sinon.assert.calledWithExactly( spy, 'tableBorderWidth', { value: '3em', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#backgroundColor', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tablePropertiesView.backgroundColor = '#FFAAFF';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableBackgroundColor', { value: '#FFAAFF', batch: 'foo' } );
+					} );
+
+					it( 'should display an error message if value is invalid', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						// First, let's pass an invalid value and check what happens.
+						tablePropertiesView.backgroundColor = '42';
+
+						clock.tick( 500 );
+
+						expect( tablePropertiesView.backgroundInput.errorText ).to.match( /^The color is invalid/ );
+						sinon.assert.notCalled( spy );
+
+						// And now let's pass a valid value and check if the error text will be gone.
+						tablePropertiesView.backgroundColor = '#AAA';
+
+						clock.tick( 500 );
+
+						expect( tablePropertiesView.backgroundInput.errorText ).to.be.null;
+						sinon.assert.calledWithExactly( spy, 'tableBackgroundColor', { value: '#AAA', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#width', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tablePropertiesView.width = '12px';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableWidth', { value: '12px', batch: 'foo' } );
+					} );
+
+					it( 'should display an error message if value is invalid', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						// First, let's pass an invalid value and check what happens.
+						tablePropertiesView.width = 'wrong';
+
+						clock.tick( 500 );
+
+						expect( tablePropertiesView.widthInput.errorText ).to.match( /^The value is invalid/ );
+						sinon.assert.notCalled( spy );
+
+						// And now let's pass a valid value and check if the error text will be gone.
+						tablePropertiesView.width = '3em';
+
+						clock.tick( 500 );
+
+						expect( tablePropertiesView.backgroundInput.errorText ).to.be.null;
+						sinon.assert.calledWithExactly( spy, 'tableWidth', { value: '3em', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#height', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tablePropertiesView.height = '12px';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableHeight', { value: '12px', batch: 'foo' } );
+					} );
+
+					it( 'should display an error message if value is invalid', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						// First, let's pass an invalid value and check what happens.
+						tablePropertiesView.height = 'wrong';
+
+						clock.tick( 500 );
+
+						expect( tablePropertiesView.heightInput.errorText ).to.match( /^The value is invalid/ );
+						sinon.assert.notCalled( spy );
+
+						// And now let's pass a valid value and check if the error text will be gone.
+						tablePropertiesView.height = '3em';
+
+						clock.tick( 500 );
+
+						expect( tablePropertiesView.backgroundInput.errorText ).to.be.null;
+						sinon.assert.calledWithExactly( spy, 'tableHeight', { value: '3em', batch: 'foo' } );
+					} );
+				} );
+
+				describe( '#alignment', () => {
+					it( 'should affect the editor state', () => {
+						const spy = testUtils.sinon.stub( editor, 'execute' );
+
+						tablePropertiesView.alignment = 'right';
+
+						sinon.assert.calledOnce( spy );
+						sinon.assert.calledWithExactly( spy, 'tableAlignment', { value: 'right', batch: 'foo' } );
+					} );
+				} );
+
+				it( 'should not display an error text if user managed to fix the value before the UI timeout', () => {
+					// First, let's pass an invalid value.
+					tablePropertiesView.borderColor = '#';
+
+					clock.tick( 100 );
+
+					// Then the user managed to quickly type the correct value.
+					tablePropertiesView.borderColor = '#aaa';
+
+					clock.tick( 400 );
+
+					// Because they were quick, they should see no error
+					expect( tablePropertiesView.borderColorInput.errorText ).to.be.null;
 				} );
 
 				it( 'should not affect the editor state if internal property has changed', () => {
@@ -230,7 +419,6 @@ describe( 'table properties', () => {
 
 					tablePropertiesView.set( 'internalProp', 'foo' );
 
-					tablePropertiesUI._undoStepBatch = 'foo';
 					tablePropertiesView.internalProp = 'bar';
 
 					sinon.assert.notCalled( spy );

+ 50 - 0
packages/ckeditor5-table/tests/tableproperties/ui/tablepropertiesview.js

@@ -417,6 +417,56 @@ describe( 'table properties', () => {
 
 						expect( spy.calledOnce ).to.be.true;
 					} );
+
+					it( 'should make sure the #saveButtonView is disabled until text fields are without errors', () => {
+						view.borderWidthInput.errorText = 'foo';
+						view.borderColorInput.errorText = 'foo';
+						view.backgroundInput.errorText = 'foo';
+						view.widthInput.errorText = 'foo';
+						view.heightInput.errorText = 'foo';
+
+						expect( view.saveButtonView.isEnabled ).to.be.false;
+
+						view.borderWidthInput.errorText = 'foo';
+						view.borderColorInput.errorText = 'foo';
+						view.backgroundInput.errorText = 'foo';
+						view.widthInput.errorText = 'foo';
+						view.heightInput.errorText = null;
+
+						expect( view.saveButtonView.isEnabled ).to.be.false;
+
+						view.borderWidthInput.errorText = 'foo';
+						view.borderColorInput.errorText = 'foo';
+						view.backgroundInput.errorText = 'foo';
+						view.widthInput.errorText = null;
+						view.heightInput.errorText = null;
+
+						expect( view.saveButtonView.isEnabled ).to.be.false;
+
+						view.borderWidthInput.errorText = 'foo';
+						view.borderColorInput.errorText = 'foo';
+						view.backgroundInput.errorText = null;
+						view.widthInput.errorText = null;
+						view.heightInput.errorText = null;
+
+						expect( view.saveButtonView.isEnabled ).to.be.false;
+
+						view.borderWidthInput.errorText = 'foo';
+						view.borderColorInput.errorText = null;
+						view.backgroundInput.errorText = null;
+						view.widthInput.errorText = null;
+						view.heightInput.errorText = null;
+
+						expect( view.saveButtonView.isEnabled ).to.be.false;
+
+						view.borderWidthInput.errorText = null;
+						view.borderColorInput.errorText = null;
+						view.backgroundInput.errorText = null;
+						view.widthInput.errorText = null;
+						view.heightInput.errorText = null;
+
+						expect( view.saveButtonView.isEnabled ).to.be.true;
+					} );
 				} );
 			} );
 

+ 64 - 1
packages/ckeditor5-table/tests/ui/utils.js

@@ -19,6 +19,10 @@ import {
 	getBalloonCellPositionData,
 	getBorderStyleDefinitions,
 	getBorderStyleLabels,
+	getLocalizedColorErrorText,
+	getLocalizedLengthErrorText,
+	lengthFieldValidator,
+	colorFieldValidator,
 	fillToolbar
 } from '../../src/ui/utils';
 import Collection from '@ckeditor/ckeditor5-utils/src/collection';
@@ -145,7 +149,7 @@ describe( 'UI Utils', () => {
 		} );
 	} );
 
-	describe( 'getBalloonCellPositionData', () => {
+	describe( 'getBalloonCellPositionData()', () => {
 		it( 'returns the position data', () => {
 			const defaultPositions = BalloonPanelView.defaultPositions;
 
@@ -190,6 +194,65 @@ describe( 'UI Utils', () => {
 		} );
 	} );
 
+	describe( 'getLocalizedColorErrorText()', () => {
+		it( 'should return the error text', () => {
+			const t = string => string;
+
+			expect( getLocalizedColorErrorText( t ) ).to.match( /^The color is invalid/ );
+		} );
+	} );
+
+	describe( 'getLocalizedLengthErrorText()', () => {
+		it( 'should return the error text', () => {
+			const t = string => string;
+
+			expect( getLocalizedLengthErrorText( t ) ).to.match( /^The value is invalid/ );
+		} );
+	} );
+
+	describe( 'colorFieldValidator()', () => {
+		it( 'should passe for an empty value', () => {
+			expect( colorFieldValidator( '' ) ).to.be.true;
+		} );
+
+		it( 'should passe for white spaces', () => {
+			expect( colorFieldValidator( '  ' ) ).to.be.true;
+		} );
+
+		it( 'should pass for colors', () => {
+			expect( colorFieldValidator( '#FFF' ) ).to.be.true;
+			expect( colorFieldValidator( '#FFAA11' ) ).to.be.true;
+			expect( colorFieldValidator( 'rgb(255,123,100)' ) ).to.be.true;
+			expect( colorFieldValidator( 'red' ) ).to.be.true;
+		} );
+
+		it( 'should pass for colors surrounded by white spaces', () => {
+			expect( colorFieldValidator( ' #AAA ' ) ).to.be.true;
+			expect( colorFieldValidator( ' rgb(255,123,100) ' ) ).to.be.true;
+		} );
+	} );
+
+	describe( 'lengthFieldValidator()', () => {
+		it( 'should passe for an empty value', () => {
+			expect( lengthFieldValidator( '' ) ).to.be.true;
+		} );
+
+		it( 'should passe for white spaces', () => {
+			expect( lengthFieldValidator( '  ' ) ).to.be.true;
+		} );
+
+		it( 'should pass for lengths', () => {
+			expect( lengthFieldValidator( '1px' ) ).to.be.true;
+			expect( lengthFieldValidator( '12em' ) ).to.be.true;
+			expect( lengthFieldValidator( ' 12em ' ) ).to.be.true;
+		} );
+
+		it( 'should pass for lengths surrounded by white spaces', () => {
+			expect( lengthFieldValidator( '3px ' ) ).to.be.true;
+			expect( lengthFieldValidator( ' 12em ' ) ).to.be.true;
+		} );
+	} );
+
 	describe( 'getBorderStyleDefinitions()', () => {
 		let view, locale, definitions;
 

+ 24 - 8
packages/ckeditor5-table/theme/tableform.css

@@ -12,24 +12,16 @@
 			flex-direction: column-reverse;
 			align-items: center;
 
-			& > .ck-label {
-				font-size: 10px;
-			}
-
 			& .ck.ck-dropdown {
 				flex-grow: 0;
 			}
 		}
 
 		& .ck-table-form__border-style {
-			width: 80px;
-			min-width: 80px;
 			flex-grow: 0;
 		}
 
 		& .ck-table-form__border-width {
-			width: 80px;
-			min-width: 80px;
 			flex-grow: 0;
 		}
 	}
@@ -37,4 +29,28 @@
 	& .ck-table-form__dimension-operator {
 		flex-grow: 0;
 	}
+
+	& .ck.ck-labeled-view {
+		/* Allow absolute positioning of the status (error) balloons. */
+		position: relative;
+
+		& .ck.ck-labeled-view__status {
+			position: absolute;
+			left: 50%;
+			bottom: calc( -1 * var(--ck-table-properties-error-arrow-size) );
+			transform: translate(-50%,100%);
+
+			/* Make sure the balloon status stays on top of other form elements. */
+			z-index: 1;
+
+			/* The arrow pointing towards the field. */
+			&::after {
+				content: "";
+				position: absolute;
+				top: calc( -1 * var(--ck-table-properties-error-arrow-size) );
+				left: 50%;
+				transform: translateX( -50% );
+			}
+		}
+	}
 }