Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.

Commit 5726090

Browse files
oleqoleq
authored
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.
2 parents 717608c + 08f05f6 commit 5726090

File tree

4 files changed

+99
-59
lines changed

4 files changed

+99
-59
lines changed

‎src/tablecellproperties/ui/tablecellpropertiesview.js

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -62,34 +62,6 @@ export default class TableCellPropertiesView extends View {
6262
constructor( locale ) {
6363
super( locale );
6464

65-
const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields();
66-
const { widthInput, operatorLabel, heightInput, dimensionsLabel } = this._createDimensionFields();
67-
const { horizontalAlignmentToolbar, verticalAlignmentToolbar, alignmentLabel } = this._createAlignmentFields();
68-
69-
/**
70-
* Tracks information about the DOM focus in the form.
71-
*
72-
* @readonly
73-
* @member {module:utils/focustracker~FocusTracker}
74-
*/
75-
this.focusTracker = new FocusTracker();
76-
77-
/**
78-
* An instance of the {@link module:utils/keystrokehandler~KeystrokeHandler}.
79-
*
80-
* @readonly
81-
* @member {module:utils/keystrokehandler~KeystrokeHandler}
82-
*/
83-
this.keystrokes = new KeystrokeHandler();
84-
85-
/**
86-
* A collection of child views in the form.
87-
*
88-
* @readonly
89-
* @type {module:ui/viewcollection~ViewCollection}
90-
*/
91-
this.children = this.createCollection();
92-
9365
this.set( {
9466
/**
9567
* The value of the cell border style.
@@ -173,6 +145,34 @@ export default class TableCellPropertiesView extends View {
173145
verticalAlignment: 'middle'
174146
} );
175147

148+
const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields();
149+
const { widthInput, operatorLabel, heightInput, dimensionsLabel } = this._createDimensionFields();
150+
const { horizontalAlignmentToolbar, verticalAlignmentToolbar, alignmentLabel } = this._createAlignmentFields();
151+
152+
/**
153+
* Tracks information about the DOM focus in the form.
154+
*
155+
* @readonly
156+
* @member {module:utils/focustracker~FocusTracker}
157+
*/
158+
this.focusTracker = new FocusTracker();
159+
160+
/**
161+
* An instance of the {@link module:utils/keystrokehandler~KeystrokeHandler}.
162+
*
163+
* @readonly
164+
* @member {module:utils/keystrokehandler~KeystrokeHandler}
165+
*/
166+
this.keystrokes = new KeystrokeHandler();
167+
168+
/**
169+
* A collection of child views in the form.
170+
*
171+
* @readonly
172+
* @type {module:ui/viewcollection~ViewCollection}
173+
*/
174+
this.children = this.createCollection();
175+
176176
/**
177177
* A dropdown that allows selecting the style of the table cell border.
178178
*
@@ -494,6 +494,15 @@ export default class TableCellPropertiesView extends View {
494494
this.borderColor = borderColorInput.view.element.value;
495495
} );
496496

497+
// Reset the border color and width fields when style is "none".
498+
// https://github.com/ckeditor/ckeditor5/issues/6227
499+
this.on( 'change:borderStyle', ( evt, name, value ) => {
500+
if ( value === 'none' ) {
501+
this.borderColor = '';
502+
this.borderWidth = '';
503+
}
504+
} );
505+
497506
return {
498507
borderRowLabel,
499508
borderStyleDropdown,

‎src/tableproperties/ui/tablepropertiesview.js

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -54,34 +54,6 @@ export default class TablePropertiesView extends View {
5454
constructor( locale ) {
5555
super( locale );
5656

57-
const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields();
58-
const { widthInput, operatorLabel, heightInput, dimensionsLabel } = this._createDimensionFields();
59-
const { alignmentToolbar, alignmentLabel } = this._createAlignmentFields();
60-
61-
/**
62-
* Tracks information about the DOM focus in the form.
63-
*
64-
* @readonly
65-
* @member {module:utils/focustracker~FocusTracker}
66-
*/
67-
this.focusTracker = new FocusTracker();
68-
69-
/**
70-
* An instance of the {@link module:utils/keystrokehandler~KeystrokeHandler}.
71-
*
72-
* @readonly
73-
* @member {module:utils/keystrokehandler~KeystrokeHandler}
74-
*/
75-
this.keystrokes = new KeystrokeHandler();
76-
77-
/**
78-
* A collection of child views in the form.
79-
*
80-
* @readonly
81-
* @type {module:ui/viewcollection~ViewCollection}
82-
*/
83-
this.children = this.createCollection();
84-
8557
this.set( {
8658
/**
8759
* The value of the border style.
@@ -147,6 +119,34 @@ export default class TablePropertiesView extends View {
147119
alignment: ''
148120
} );
149121

122+
const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields();
123+
const { widthInput, operatorLabel, heightInput, dimensionsLabel } = this._createDimensionFields();
124+
const { alignmentToolbar, alignmentLabel } = this._createAlignmentFields();
125+
126+
/**
127+
* Tracks information about the DOM focus in the form.
128+
*
129+
* @readonly
130+
* @member {module:utils/focustracker~FocusTracker}
131+
*/
132+
this.focusTracker = new FocusTracker();
133+
134+
/**
135+
* An instance of the {@link module:utils/keystrokehandler~KeystrokeHandler}.
136+
*
137+
* @readonly
138+
* @member {module:utils/keystrokehandler~KeystrokeHandler}
139+
*/
140+
this.keystrokes = new KeystrokeHandler();
141+
142+
/**
143+
* A collection of child views in the form.
144+
*
145+
* @readonly
146+
* @type {module:ui/viewcollection~ViewCollection}
147+
*/
148+
this.children = this.createCollection();
149+
150150
/**
151151
* A dropdown that allows selecting the style of the table border.
152152
*
@@ -440,11 +440,20 @@ export default class TablePropertiesView extends View {
440440
this.borderColor = borderColorInput.view.element.value;
441441
} );
442442

443+
// Reset the border color and width fields when style is "none".
444+
// https://github.com/ckeditor/ckeditor5/issues/6227
445+
this.on( 'change:borderStyle', ( evt, name, value ) => {
446+
if ( value === 'none' ) {
447+
this.borderColor = '';
448+
this.borderWidth = '';
449+
}
450+
} );
451+
443452
return {
444453
borderRowLabel,
445454
borderStyleDropdown,
446455
borderColorInput,
447-
borderWidthInput,
456+
borderWidthInput
448457
};
449458
}
450459

‎tests/tablecellproperties/ui/tablecellpropertiesview.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,20 @@ describe( 'table cell properties', () => {
134134
expect( labeledDropdown.view.listView.items.map( item => {
135135
return item.children.first.label;
136136
} ) ).to.have.ordered.members( [
137-
'None', 'Solid', 'Dotted', 'Dashed', 'Double', 'Groove', 'Ridge', 'Inset', 'Outset',
137+
'None', 'Solid', 'Dotted', 'Dashed', 'Double', 'Groove', 'Ridge', 'Inset', 'Outset'
138138
] );
139139
} );
140+
141+
it( 'should reset border width and color inputs when setting style to none', () => {
142+
view.borderStyle = 'dotted';
143+
view.borderWidth = '1px';
144+
view.borderColor = 'red';
145+
146+
view.borderStyle = 'none';
147+
148+
expect( view.borderColor ).to.equal( '' );
149+
expect( view.borderWidth ).to.equal( '' );
150+
} );
140151
} );
141152

142153
describe( 'border width input', () => {

‎tests/tableproperties/ui/tablepropertiesview.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,20 @@ describe( 'table properties', () => {
134134
expect( labeledDropdown.view.listView.items.map( item => {
135135
return item.children.first.label;
136136
} ) ).to.have.ordered.members( [
137-
'None', 'Solid', 'Dotted', 'Dashed', 'Double', 'Groove', 'Ridge', 'Inset', 'Outset',
137+
'None', 'Solid', 'Dotted', 'Dashed', 'Double', 'Groove', 'Ridge', 'Inset', 'Outset'
138138
] );
139139
} );
140+
141+
it( 'should reset border width and color inputs when setting style to none', () => {
142+
view.borderStyle = 'dotted';
143+
view.borderWidth = '1px';
144+
view.borderColor = 'red';
145+
146+
view.borderStyle = 'none';
147+
148+
expect( view.borderColor ).to.equal( '' );
149+
expect( view.borderWidth ).to.equal( '' );
150+
} );
140151
} );
141152

142153
describe( 'border width input', () => {

0 commit comments

Comments
 (0)