Merge pull request #242 from ckeditor/i/6227

Internal: Setting table/cell border style to "none" should reset other border fields. Closes ckeditor/ckeditor5#6227.
ckeditor · Feb 12, 2020 · 5726090 · 5726090
2 parents 717608c + 08f05f6
commit 5726090
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 59 deletions.
diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js
@@ -62,34 +62,6 @@ export default class TableCellPropertiesView extends View {
 	constructor( locale ) {
 		super( locale );
 
-		const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields();
-		const { widthInput, operatorLabel, heightInput, dimensionsLabel } = this._createDimensionFields();
-		const { horizontalAlignmentToolbar, verticalAlignmentToolbar, alignmentLabel } = this._createAlignmentFields();
-
-		/**
-		 * Tracks information about the DOM focus in the form.
-		 *
-		 * @readonly
-		 * @member {module:utils/focustracker~FocusTracker}
-		 */
-		this.focusTracker = new FocusTracker();
-
-		/**
-		 * An instance of the {@link module:utils/keystrokehandler~KeystrokeHandler}.
-		 *
-		 * @readonly
-		 * @member {module:utils/keystrokehandler~KeystrokeHandler}
-		 */
-		this.keystrokes = new KeystrokeHandler();
-
-		/**
-		 * A collection of child views in the form.
-		 *
-		 * @readonly
-		 * @type {module:ui/viewcollection~ViewCollection}
-		 */
-		this.children = this.createCollection();
-
 		this.set( {
 			/**
 			 * The value of the cell border style.
@@ -173,6 +145,34 @@ export default class TableCellPropertiesView extends View {
 			verticalAlignment: 'middle'
 		} );
 
+		const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields();
+		const { widthInput, operatorLabel, heightInput, dimensionsLabel } = this._createDimensionFields();
+		const { horizontalAlignmentToolbar, verticalAlignmentToolbar, alignmentLabel } = this._createAlignmentFields();
+
+		/**
+		 * Tracks information about the DOM focus in the form.
+		 *
+		 * @readonly
+		 * @member {module:utils/focustracker~FocusTracker}
+		 */
+		this.focusTracker = new FocusTracker();
+
+		/**
+		 * An instance of the {@link module:utils/keystrokehandler~KeystrokeHandler}.
+		 *
+		 * @readonly
+		 * @member {module:utils/keystrokehandler~KeystrokeHandler}
+		 */
+		this.keystrokes = new KeystrokeHandler();
+
+		/**
+		 * A collection of child views in the form.
+		 *
+		 * @readonly
+		 * @type {module:ui/viewcollection~ViewCollection}
+		 */
+		this.children = this.createCollection();
+
 		/**
 		 * A dropdown that allows selecting the style of the table cell border.
 		 *
@@ -494,6 +494,15 @@ export default class TableCellPropertiesView extends View {
 			this.borderColor = borderColorInput.view.element.value;
 		} );
 
+		// Reset the border color and width fields when style is "none".
+		// https://github.com/ckeditor/ckeditor5/issues/6227
+		this.on( 'change:borderStyle', ( evt, name, value ) => {
+			if ( value === 'none' ) {
+				this.borderColor = '';
+				this.borderWidth = '';
+			}
+		} );
+
 		return {
 			borderRowLabel,
 			borderStyleDropdown,

diff --git a/src/tableproperties/ui/tablepropertiesview.js b/src/tableproperties/ui/tablepropertiesview.js
@@ -54,34 +54,6 @@ export default class TablePropertiesView extends View {
 	constructor( locale ) {
 		super( locale );
 
-		const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields();
-		const { widthInput, operatorLabel, heightInput, dimensionsLabel } = this._createDimensionFields();
-		const { alignmentToolbar, alignmentLabel } = this._createAlignmentFields();
-
-		/**
-		 * Tracks information about the DOM focus in the form.
-		 *
-		 * @readonly
-		 * @member {module:utils/focustracker~FocusTracker}
-		 */
-		this.focusTracker = new FocusTracker();
-
-		/**
-		 * An instance of the {@link module:utils/keystrokehandler~KeystrokeHandler}.
-		 *
-		 * @readonly
-		 * @member {module:utils/keystrokehandler~KeystrokeHandler}
-		 */
-		this.keystrokes = new KeystrokeHandler();
-
-		/**
-		 * A collection of child views in the form.
-		 *
-		 * @readonly
-		 * @type {module:ui/viewcollection~ViewCollection}
-		 */
-		this.children = this.createCollection();
-
 		this.set( {
 			/**
 			 * The value of the border style.
@@ -147,6 +119,34 @@ export default class TablePropertiesView extends View {
 			alignment: ''
 		} );
 
+		const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields();
+		const { widthInput, operatorLabel, heightInput, dimensionsLabel } = this._createDimensionFields();
+		const { alignmentToolbar, alignmentLabel } = this._createAlignmentFields();
+
+		/**
+		 * Tracks information about the DOM focus in the form.
+		 *
+		 * @readonly
+		 * @member {module:utils/focustracker~FocusTracker}
+		 */
+		this.focusTracker = new FocusTracker();
+
+		/**
+		 * An instance of the {@link module:utils/keystrokehandler~KeystrokeHandler}.
+		 *
+		 * @readonly
+		 * @member {module:utils/keystrokehandler~KeystrokeHandler}
+		 */
+		this.keystrokes = new KeystrokeHandler();
+
+		/**
+		 * A collection of child views in the form.
+		 *
+		 * @readonly
+		 * @type {module:ui/viewcollection~ViewCollection}
+		 */
+		this.children = this.createCollection();
+
 		/**
 		 * A dropdown that allows selecting the style of the table border.
 		 *
@@ -440,11 +440,20 @@ export default class TablePropertiesView extends View {
 			this.borderColor = borderColorInput.view.element.value;
 		} );
 
+		// Reset the border color and width fields when style is "none".
+		// https://github.com/ckeditor/ckeditor5/issues/6227
+		this.on( 'change:borderStyle', ( evt, name, value ) => {
+			if ( value === 'none' ) {
+				this.borderColor = '';
+				this.borderWidth = '';
+			}
+		} );
+
 		return {
 			borderRowLabel,
 			borderStyleDropdown,
 			borderColorInput,
-			borderWidthInput,
+			borderWidthInput
 		};
 	}
 

diff --git a/tests/tablecellproperties/ui/tablecellpropertiesview.js b/tests/tablecellproperties/ui/tablecellpropertiesview.js
@@ -134,9 +134,20 @@ describe( 'table cell properties', () => {
 							expect( labeledDropdown.view.listView.items.map( item => {
 								return item.children.first.label;
 							} ) ).to.have.ordered.members( [
-								'None', 'Solid', 'Dotted', 'Dashed', 'Double', 'Groove', 'Ridge', 'Inset', 'Outset',
+								'None', 'Solid', 'Dotted', 'Dashed', 'Double', 'Groove', 'Ridge', 'Inset', 'Outset'
 							] );
 						} );
+
+						it( 'should reset border width and color inputs when setting style to none', () => {
+							view.borderStyle = 'dotted';
+							view.borderWidth = '1px';
+							view.borderColor = 'red';
+
+							view.borderStyle = 'none';
+
+							expect( view.borderColor ).to.equal( '' );
+							expect( view.borderWidth ).to.equal( '' );
+						} );
 					} );
 
 					describe( 'border width input', () => {

diff --git a/tests/tableproperties/ui/tablepropertiesview.js b/tests/tableproperties/ui/tablepropertiesview.js
@@ -134,9 +134,20 @@ describe( 'table properties', () => {
 							expect( labeledDropdown.view.listView.items.map( item => {
 								return item.children.first.label;
 							} ) ).to.have.ordered.members( [
-								'None', 'Solid', 'Dotted', 'Dashed', 'Double', 'Groove', 'Ridge', 'Inset', 'Outset',
+								'None', 'Solid', 'Dotted', 'Dashed', 'Double', 'Groove', 'Ridge', 'Inset', 'Outset'
 							] );
 						} );
+
+						it( 'should reset border width and color inputs when setting style to none', () => {
+							view.borderStyle = 'dotted';
+							view.borderWidth = '1px';
+							view.borderColor = 'red';
+
+							view.borderStyle = 'none';
+
+							expect( view.borderColor ).to.equal( '' );
+							expect( view.borderWidth ).to.equal( '' );
+						} );
 					} );
 
 					describe( 'border width input', () => {