diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js index dab56504..210c819a 100644 --- 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 index e1035b44..a3579a26 100644 --- 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/src/tableui.js b/src/tableui.js index 34646f1a..7acc4cd5 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -54,21 +54,29 @@ export default class TableUI extends Plugin { tooltip: true } ); - // Prepare custom view for dropdown's panel. - const insertTableView = new InsertTableView( locale ); - dropdownView.panelView.children.add( insertTableView ); + let insertTableView; - insertTableView.delegate( 'execute' ).to( dropdownView ); + dropdownView.on( 'change:isOpen', () => { + if ( insertTableView ) { + return; + } - dropdownView.buttonView.on( 'open', () => { - // Reset the chooser before showing it to the user. - insertTableView.rows = 0; - insertTableView.columns = 0; - } ); + // Prepare custom view for dropdown's panel. + insertTableView = new InsertTableView( locale ); + dropdownView.panelView.children.add( insertTableView ); + + insertTableView.delegate( 'execute' ).to( dropdownView ); + + dropdownView.buttonView.on( 'open', () => { + // Reset the chooser before showing it to the user. + insertTableView.rows = 0; + insertTableView.columns = 0; + } ); - dropdownView.on( 'execute', () => { - editor.execute( 'insertTable', { rows: insertTableView.rows, columns: insertTableView.columns } ); - editor.editing.view.focus(); + dropdownView.on( 'execute', () => { + editor.execute( 'insertTable', { rows: insertTableView.rows, columns: insertTableView.columns } ); + editor.editing.view.focus(); + } ); } ); return dropdownView; diff --git a/tests/tablecellproperties/ui/tablecellpropertiesview.js b/tests/tablecellproperties/ui/tablecellpropertiesview.js index 62cafe3e..081b7f11 100644 --- 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 index d9dce431..038bc3e8 100644 --- 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', () => { diff --git a/tests/tableui.js b/tests/tableui.js index 15969cf8..b02a05b2 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -11,6 +11,7 @@ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import TableEditing from '../src/tableediting'; import TableUI from '../src/tableui'; +import InsertTableView from '../src/ui/inserttableview'; import SwitchButtonView from '@ckeditor/ckeditor5-ui/src/button/switchbuttonview'; import DropdownView from '@ckeditor/ckeditor5-ui/src/dropdown/dropdownview'; import ListSeparatorView from '@ckeditor/ckeditor5-ui/src/list/listseparatorview'; @@ -53,6 +54,7 @@ describe( 'TableUI', () => { beforeEach( () => { insertTable = editor.ui.componentFactory.create( 'insertTable' ); + insertTable.isOpen = true; // Dropdown is lazy loaded, so make sure its open (#6193). } ); it( 'should register insertTable button', () => { @@ -65,7 +67,7 @@ describe( 'TableUI', () => { const command = editor.commands.get( 'insertTable' ); command.isEnabled = true; - expect( insertTable.buttonView.isOn ).to.be.false; + expect( insertTable.buttonView.isOn ).to.be.true; expect( insertTable.buttonView.isEnabled ).to.be.true; command.isEnabled = false; @@ -87,6 +89,8 @@ describe( 'TableUI', () => { } ); it( 'should reset rows & columns on dropdown open', () => { + insertTable.isOpen = true; + const tableSizeView = insertTable.panelView.children.first; expect( tableSizeView.rows ).to.equal( 0 ); @@ -100,6 +104,14 @@ describe( 'TableUI', () => { expect( tableSizeView.rows ).to.equal( 0 ); expect( tableSizeView.columns ).to.equal( 0 ); } ); + + it( 'is not fully initialized when not open', () => { + const dropdown = editor.ui.componentFactory.create( 'insertTable' ); + + for ( const childView of dropdown.panelView.children ) { + expect( childView ).not.to.be.instanceOf( InsertTableView ); + } + } ); } ); describe( 'tableRow dropdown', () => {