From 39d92286fa569504d430ae4343141cb9384328a8 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 13 Jan 2020 16:23:23 +0100 Subject: [PATCH 01/33] First implementation of the cell properties form. --- src/tablecellproperties.js | 8 + src/tablecellpropertiesui.js | 317 ++++++++++++++ src/tableproperties.js | 8 + src/tablepropertiesui.js | 45 ++ src/ui/tablecellpropertiesview.js | 581 ++++++++++++++++++++++++++ src/ui/utils.js | 54 +++ tests/manual/tableproperties.js | 10 +- theme/form.css | 69 +++ theme/icons/align-bottom.svg | 1 + theme/icons/align-middle.svg | 1 + theme/icons/align-top.svg | 1 + theme/icons/table-cell-properties.svg | 1 + theme/icons/table-properties.svg | 1 + theme/tablecellproperties.css | 84 ++++ 14 files changed, 1177 insertions(+), 4 deletions(-) create mode 100644 src/tablecellpropertiesui.js create mode 100644 src/tablepropertiesui.js create mode 100644 src/ui/tablecellpropertiesview.js create mode 100644 src/ui/utils.js create mode 100644 theme/form.css create mode 100644 theme/icons/align-bottom.svg create mode 100644 theme/icons/align-middle.svg create mode 100644 theme/icons/align-top.svg create mode 100644 theme/icons/table-cell-properties.svg create mode 100644 theme/icons/table-properties.svg create mode 100644 theme/tablecellproperties.css diff --git a/src/tablecellproperties.js b/src/tablecellproperties.js index 0373a404..dc18d20f 100644 --- a/src/tablecellproperties.js +++ b/src/tablecellproperties.js @@ -8,6 +8,7 @@ */ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; +import TableCellPropertiesUI from './tablecellpropertiesui'; import { downcastToStyle, upcastAttribute, upcastBorderStyles } from './tableproperties/utils'; /** @@ -23,6 +24,13 @@ export default class TableCellProperties extends Plugin { return 'TableCellProperties'; } + /** + * @inheritDoc + */ + static get requires() { + return [ TableCellPropertiesUI ]; + } + /** * @inheritDoc */ diff --git a/src/tablecellpropertiesui.js b/src/tablecellpropertiesui.js new file mode 100644 index 00000000..90a7510b --- /dev/null +++ b/src/tablecellpropertiesui.js @@ -0,0 +1,317 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module table/tablecellpropertiesui + */ + +import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; +import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; +import { getTableWidgetAncestor } from './utils'; +import clickOutsideHandler from '@ckeditor/ckeditor5-ui/src/bindings/clickoutsidehandler'; +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, getBalloonPositionData } from './ui/utils'; +import { findAncestor } from './commands/utils'; + +const DEFAULT_BORDER_STYLE = 'none'; +const DEFAULT_HORIZONTAL_ALIGNMENT = 'left'; +const DEFAULT_VERTICAL_ALIGNMENT = 'middle'; + +// Attributes that set the same value for "top", "right", "bottom", and "left". +const QUAD_DIRECTION_ATTRIBUTES = [ 'borderStyle', 'borderWidth', 'borderColor', 'padding' ]; + +/** + * The table cell properties UI plugin. It introduces the `'tableCellProperties'` button + * that opens a form allowing to specify visual styling of a table cell. + * + * It uses the + * {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon contextual balloon plugin}. + * + * @extends module:core/plugin~Plugin + */ +export default class TableCellPropertiesUI extends Plugin { + /** + * @inheritDoc + */ + static get requires() { + return [ ContextualBalloon ]; + } + + /** + * @inheritDoc + */ + static get pluginName() { + return 'TableCellPropertiesUI'; + } + + /** + * @inheritDoc + */ + init() { + const editor = this.editor; + const t = editor.t; + + /** + * The contextual balloon plugin instance. + * + * @private + * @member {module:ui/panel/balloon/contextualballoon~ContextualBalloon} + */ + this._balloon = editor.plugins.get( ContextualBalloon ); + + /** + * The batch used to undo all changes made by the form (which are live) + * if "Cancel" was pressed. Each time the view is shown, a new batch is created. + * + * @private + * @member {module:engine/model/batch~Batch} + */ + this._batch = null; + + // Create the view that displays the properties form. + this._createPropertiesView(); + + // Make the form dynamic, i.e. create bindings between view fields and the model. + this._startRespondingToChangesInView(); + + editor.ui.componentFactory.add( 'tableCellProperties', locale => { + const view = new ButtonView( locale ); + + view.set( { + label: t( 'Cell properties' ), + icon: tableCellProperties, + tooltip: true + } ); + + this.listenTo( view, 'execute', () => this._showView() ); + + return view; + } ); + } + + /** + * @inheritDoc + */ + destroy() { + super.destroy(); + + // Destroy created UI components as they are not automatically destroyed. + // See https://github.com/ckeditor/ckeditor5/issues/1341. + this.view.destroy(); + } + + /** + * Creates the {@link module:table/ui/tablecellpropertiesview~TableCellPropertiesView} instance. + * + * @private + * @returns {module:table/ui/tablecellpropertiesview~TableCellPropertiesView} The cell properties form + * view instance. + */ + _createPropertiesView() { + const editor = this.editor; + const view = editor.editing.view; + const viewDocument = view.document; + + /** + * The properties form view displayed inside the balloon. + * + * @member {module:table/ui/tablecellpropertiesview~TableCellPropertiesView} + */ + this.view = new TableCellPropertiesView( editor.locale ); + + // Render the view so its #element is available for clickOutsideHandler. + this.view.render(); + + this.listenTo( this.view, 'submit', () => { + this._hideView(); + } ); + + this.listenTo( this.view, 'cancel', () => { + editor.execute( 'undo', this._batch ); + this._hideView(); + } ); + + // Close the balloon on Esc key press when the **form has focus**. + this.view.keystrokes.set( 'Esc', ( data, cancel ) => { + this._hideView(); + cancel(); + } ); + + // Reposition the balloon or hide the form if an image widget is no longer selected. + this.listenTo( editor.ui, 'update', () => { + if ( !getTableWidgetAncestor( viewDocument.selection ) ) { + this._hideView(); + } else if ( this._isViewVisible ) { + repositionContextualBalloon( editor ); + } + } ); + + // Close on click outside of balloon panel element. + clickOutsideHandler( { + emitter: this.view, + activator: () => this._isViewInBalloon, + contextElements: [ this._balloon.view.element ], + callback: () => this._hideView() + } ); + } + + _startRespondingToChangesInView() { + const editor = this.editor; + const model = editor.model; + const document = model.document; + const selection = document.selection; + + this.view.on( 'update', ( evt, data ) => { + const firstPosition = selection.getFirstPosition(); + const tableCell = findAncestor( 'tableCell', firstPosition ); + + // Enqueue all changes into a single batch so clicking "Cancel" can undo them + // as a single undo steps. It's a better UX than dozens of undo steps, e.g. each + // for a single value change. + editor.model.enqueueChange( this._batch, writer => { + for ( const property in data ) { + const value = data[ property ]; + + if ( QUAD_DIRECTION_ATTRIBUTES.includes( property ) ) { + writer.setAttribute( property, { + top: value, + right: value, + bottom: value, + left: value + }, tableCell ); + } else { + writer.setAttribute( property, value, tableCell ); + } + } + } ); + } ); + } + + _fillViewFormFromSelectedCell() { + const editor = this.editor; + const model = editor.model; + const document = model.document; + const selection = document.selection; + const firstPosition = selection.getFirstPosition(); + const tableCell = findAncestor( 'tableCell', firstPosition ); + + const borderWidth = unifyQuadDirectionPropertyValue( tableCell.getAttribute( 'borderWidth' ) ) || ''; + const borderColor = unifyQuadDirectionPropertyValue( tableCell.getAttribute( 'borderColor' ) ) || ''; + const borderStyle = unifyQuadDirectionPropertyValue( tableCell.getAttribute( 'borderStyle' ) ) || DEFAULT_BORDER_STYLE; + const padding = unifyQuadDirectionPropertyValue( tableCell.getAttribute( 'padding' ) ) || ''; + const backgroundColor = tableCell.getAttribute( 'backgroundColor' ) || ''; + const horizontalAlignment = tableCell.getAttribute( 'horizontalAlignment' ) || DEFAULT_HORIZONTAL_ALIGNMENT; + const verticalAlignment = tableCell.getAttribute( 'verticalAlignment' ) || DEFAULT_VERTICAL_ALIGNMENT; + + const view = this.view; + + view.borderWidthInput.inputView.element.value = ''; + view.borderColorInput.inputView.element.value = ''; + view.paddingInput.inputView.element.value = ''; + view.backgroundInput.inputView.element.value = ''; + + view.set( { + borderWidth, + borderColor, + borderStyle, + padding, + backgroundColor, + horizontalAlignment, + verticalAlignment + } ); + } + + _showView() { + if ( this._isViewVisible ) { + return; + } + + const editor = this.editor; + + if ( !this._isViewInBalloon ) { + this._balloon.add( { + view: this.view, + position: getBalloonPositionData( editor ) + } ); + } + + // Create a new batch. Clicking "Cancel" will undo this batch. + this._batch = editor.model.createBatch(); + + // Update the view with the model values. + this._fillViewFormFromSelectedCell(); + + // Basic a11y. + this.view.focus(); + } + + /** + * Removes the {@link #view} from the {@link #_balloon}. + * + * See {@link #_addFormView}, {@link #_addActionsView}. + * + * @protected + */ + _hideView() { + if ( !this._isViewInBalloon ) { + return; + } + + const editor = this.editor; + + this.stopListening( editor.ui, 'update' ); + this.stopListening( this._balloon, 'change:visibleView' ); + + // Make sure the focus always gets back to the editable _before_ removing the focused properties view. + // Doing otherwise causes issues in some browsers. See https://github.com/ckeditor/ckeditor5-link/issues/193. + editor.editing.view.focus(); + + if ( this._isViewInBalloon ) { + // TODO below + // Blur the input element before removing it from DOM to prevent issues in some browsers. + // See https://github.com/ckeditor/ckeditor5/issues/1501. + // this.formView.saveButtonView.focus(); + + this._balloon.remove( this.view ); + + // Because the form has an input which has focus, the focus must be brought back + // to the editor. Otherwise, it would be lost. + this.editor.editing.view.focus(); + } + } + + /** + * Returns `true` when the {@link #view} is the visible view in the {@link #_balloon}. + * + * @private + * @type {Boolean} + */ + get _isViewVisible() { + return this._balloon.visibleView === this.view; + } + + /** + * Returns `true` when the {@link #view} is in the {@link #_balloon}. + * + * @private + * @type {Boolean} + */ + get _isViewInBalloon() { + return this._balloon.hasView( this.view ); + } +} + +function unifyQuadDirectionPropertyValue( value ) { + if ( !value ) { + return; + } + + // Unify width to one value. If different values are set default to top (or right, etc). + value = value.top || value.right || value.bottom || value.left; + + return value; +} diff --git a/src/tableproperties.js b/src/tableproperties.js index dfce8b6f..a87784e6 100644 --- a/src/tableproperties.js +++ b/src/tableproperties.js @@ -8,6 +8,7 @@ */ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; +import TablePropertiesUI from './tablepropertiesui'; import { downcastTableAttribute, upcastAttribute, upcastBorderStyles } from './tableproperties/utils'; /** @@ -23,6 +24,13 @@ export default class TableProperties extends Plugin { return 'TableProperties'; } + /** + * @inheritDoc + */ + static get requires() { + return [ TablePropertiesUI ]; + } + /** * @inheritDoc */ diff --git a/src/tablepropertiesui.js b/src/tablepropertiesui.js new file mode 100644 index 00000000..8a4d5700 --- /dev/null +++ b/src/tablepropertiesui.js @@ -0,0 +1,45 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module table/tablepropertiesui + */ + +import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; +import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; + +import tableProperties from './../theme/icons/table-properties.svg'; + +/** + * TODO + * + * @extends module:core/plugin~Plugin + */ +export default class TablePropertiesUI extends Plugin { + /** + * @inheritDoc + */ + init() { + const editor = this.editor; + const t = editor.t; + + editor.ui.componentFactory.add( 'tableProperties', locale => { + const view = new ButtonView( locale ); + + view.set( { + label: t( 'Table properties' ), + icon: tableProperties, + tooltip: true + } ); + + this.listenTo( view, 'execute', () => this._showUI() ); + + return view; + } ); + } + + _showUI() { + } +} diff --git a/src/ui/tablecellpropertiesview.js b/src/ui/tablecellpropertiesview.js new file mode 100644 index 00000000..6c2d5261 --- /dev/null +++ b/src/ui/tablecellpropertiesview.js @@ -0,0 +1,581 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module table/ui/tablecellpropertiesview + */ + +import View from '@ckeditor/ckeditor5-ui/src/view'; +import Model from '@ckeditor/ckeditor5-ui/src/model'; +import Collection from '@ckeditor/ckeditor5-utils/src/collection'; +import ViewCollection from '@ckeditor/ckeditor5-ui/src/viewcollection'; +import submitHandler from '@ckeditor/ckeditor5-ui/src/bindings/submithandler'; + +import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler'; +import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; +import FocusCycler from '@ckeditor/ckeditor5-ui/src/focuscycler'; + +import InputTextView from '@ckeditor/ckeditor5-ui/src/inputtext/inputtextview'; +import LabeledInputView from '@ckeditor/ckeditor5-ui/src/labeledinput/labeledinputview'; +import LabelView from '@ckeditor/ckeditor5-ui/src/label/labelview'; +import { createDropdown, addListToDropdown } from '@ckeditor/ckeditor5-ui/src/dropdown/utils'; +import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; +import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; + +import uid from '@ckeditor/ckeditor5-utils/src/uid'; + +import checkIcon from '@ckeditor/ckeditor5-core/theme/icons/check.svg'; +import cancelIcon from '@ckeditor/ckeditor5-core/theme/icons/cancel.svg'; + +// TODO: These **must** be transferred to ckeditor5-core. +import alignLeftIcon from '@ckeditor/ckeditor5-alignment/theme/icons/align-left.svg'; +import alignRightIcon from '@ckeditor/ckeditor5-alignment/theme/icons/align-right.svg'; +import alignCenterIcon from '@ckeditor/ckeditor5-alignment/theme/icons/align-center.svg'; +import alignJustifyIcon from '@ckeditor/ckeditor5-alignment/theme/icons/align-justify.svg'; + +import alignTopIcon from '../../theme/icons/align-top.svg'; +import alignMiddleIcon from '../../theme/icons/align-middle.svg'; +import alignBottomIcon from '../../theme/icons/align-bottom.svg'; + +import '../../theme/form.css'; +import '../../theme/tablecellproperties.css'; + +const ALIGNMENT_ICONS = { + left: alignLeftIcon, + center: alignCenterIcon, + right: alignRightIcon, + justify: alignJustifyIcon, + top: alignTopIcon, + middle: alignMiddleIcon, + bottom: alignBottomIcon +}; + +/** + * TODO + * + * @extends module:ui/view~View + */ +export default class TableCellPropertiesView extends View { + /** + * @inheritDoc + */ + constructor( locale ) { + super( locale ); + + /** + * 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 views that can be focused in the form. + * + * @readonly + * @protected + * @member {module:ui/viewcollection~ViewCollection} + */ + this._focusables = new ViewCollection(); + + /** + * Helps cycling over {@link #_focusables} in the form. + * + * @readonly + * @protected + * @member {module:ui/focuscycler~FocusCycler} + */ + this._focusCycler = new FocusCycler( { + focusables: this._focusables, + focusTracker: this.focusTracker, + keystrokeHandler: this.keystrokes, + actions: { + // Navigate form fields backwards using the Shift + Tab keystroke. + focusPrevious: 'shift + tab', + + // Navigate form fields forwards using the Tab key. + focusNext: 'tab' + } + } ); + + this._createBorderFields(); + this._createBackgroundField(); + this._createPaddingField(); + this._createAlignmentFields(); + this._createActionButtons(); + + this.set( { + borderStyle: 'none', + borderWidth: null, + borderColor: null, + padding: null, + backgroundColor: null, + horizontalAlignment: 'left', + verticalAlignment: 'middle' + } ); + + this.setTemplate( { + tag: 'form', + attributes: { + class: [ + 'ck', + 'ck-form', + 'ck-table-cell-properties-form' + ], + // https://github.com/ckeditor/ckeditor5-link/issues/90 + tabindex: '-1' + }, + children: [ + { + tag: 'div', + attributes: { + class: [ + 'ck', + 'ck-form__header' + ] + }, + children: [ + 'Cell properties' + ] + }, + + // Border + createFormRowDefinition( { + ariaLabelledBy: this.borderRowLabel, + className: 'ck-table-cell-properties-form__border-row', + children: [ + this.borderRowLabel, + + // TODO: This should become a new component or be integrated into LabeledInputView. + { + tag: 'div', + attributes: { + class: [ + 'ck', + 'ck-labeled-dropdown', + 'ck-table-cell-properties-form__border-style' + ], + }, + children: [ + this.borderStyleDropdownLabel, + this.borderStyleDropdown, + ] + }, + this.borderWidthInput, + this.borderColorInput + ] + } ), + + // Background & Padding + createFormRowDefinition( { + children: [ + this.paddingInput, + this.backgroundInput, + ] + } ), + + // Alignment + createFormRowDefinition( { + ariaLabelledBy: this.alignmentLabel, + className: 'ck-table-cell-properties-form__alignment-row', + children: [ + this.alignmentLabel, + this.horizontalAlignmentToolbar, + this.verticalAlignmentToolbar + ] + } ), + + // Action buttons + createFormRowDefinition( { + className: 'ck-table-cell-properties-form__action-row', + children: [ + this.saveButtonView, + this.cancelButtonView + ] + } ) + ] + } ); + } + + /** + * @inheritDoc + */ + render() { + super.render(); + + submitHandler( { + view: this + } ); + + const focusableChildViews = [ + this.borderStyleDropdown, + this.borderWidthInput, + this.borderColorInput, + this.paddingInput, + this.backgroundInput, + this.horizontalAlignmentToolbar, + this.verticalAlignmentToolbar, + this.saveButtonView, + this.cancelButtonView + ]; + + focusableChildViews.forEach( v => { + // Register the view as focusable. + this._focusables.add( v ); + + // Register the view in the focus tracker. + this.focusTracker.add( v.element ); + } ); + + this.keystrokes.listenTo( this.element ); + } + + focus() { + this._focusCycler.focusFirst(); + } + + /** + * TODO + */ + _createBorderFields() { + const locale = this.locale; + const t = this.t; + + // -- Group label --------------------------------------------- + + const borderRowLabel = this.borderRowLabel = new LabelView( locale ); + borderRowLabel.text = t( 'Border' ); + + // -- Style --------------------------------------------------- + + const borderStyleDropdown = this.borderStyleDropdown = createDropdown( locale ); + borderStyleDropdown.buttonView.set( { + isOn: false, + withText: true, + tooltip: t( 'Style' ) + } ); + + borderStyleDropdown.buttonView.bind( 'label' ).to( this, 'borderStyle', value => { + return this._borderStyleLabels[ value ]; + } ); + + borderStyleDropdown.on( 'execute', evt => { + const value = evt.source._borderStyleValue; + + // Update the UI. + this.borderStyle = value; + + // Update the editor model. + this.fire( 'update', { + borderStyle: evt.source._borderStyleValue + } ); + } ); + + addListToDropdown( borderStyleDropdown, this._getBorderStyleDefinitions() ); + + this.borderStyleDropdownLabel = new LabelView( locale ); + this.borderStyleDropdownLabel.text = t( 'Style' ); + + // -- Width --------------------------------------------------- + + const borderWidthInput = this.borderWidthInput = new LabeledInputView( locale, InputTextView ); + + borderWidthInput.set( { + label: t( 'Width' ), + class: 'ck-table-cell-properties-form__border-width', + } ); + + borderWidthInput.bind( 'value' ).to( this, 'borderWidth' ); + borderWidthInput.bind( 'isReadOnly' ).to( this, 'borderStyle', value => { + return value === 'none'; + } ); + borderWidthInput.inputView.on( 'input', () => { + this.fire( 'update', { + borderWidth: borderWidthInput.inputView.element.value + } ); + } ); + + // -- Color --------------------------------------------------- + + const borderColorInput = this.borderColorInput = new LabeledInputView( locale, InputTextView ); + borderColorInput.label = t( 'Color' ); + borderColorInput.bind( 'value' ).to( this, 'borderColor' ); + borderColorInput.bind( 'isReadOnly' ).to( this, 'borderStyle', value => { + return value === 'none'; + } ); + + borderColorInput.inputView.on( 'input', () => { + this.fire( 'update', { + borderColor: borderColorInput.inputView.element.value + } ); + } ); + } + + /** + * TODO + */ + _createBackgroundField() { + const locale = this.locale; + const t = this.t; + const backgroundInput = this.backgroundInput = new LabeledInputView( locale, InputTextView ); + + backgroundInput.label = t( 'Background' ); + backgroundInput.bind( 'value' ).to( this, 'backgroundColor' ); + + backgroundInput.inputView.on( 'input', () => { + this.fire( 'update', { + backgroundColor: backgroundInput.inputView.element.value + } ); + } ); + } + + /** + * TODO + */ + _createPaddingField() { + const locale = this.locale; + const t = this.t; + const paddingInput = this.paddingInput = new LabeledInputView( locale, InputTextView ); + + paddingInput.set( { + label: t( 'Padding' ), + class: 'ck-table-cell-properties-form__padding', + } ); + + paddingInput.bind( 'value' ).to( this, 'padding' ); + paddingInput.inputView.on( 'input', () => { + this.fire( 'update', { + padding: paddingInput.inputView.element.value + } ); + } ); + } + + /** + * TODO + */ + _createAlignmentFields() { + const locale = this.locale; + const t = this.t; + + this.alignmentLabel = new LabelView( locale ); + this.alignmentLabel.text = t( 'Text alignment' ); + + // -- Horizontal --------------------------------------------------- + + this.horizontalAlignmentToolbar = new ToolbarView( locale ); + this.horizontalAlignmentToolbar.ariaLabel = t( 'Horizontal text alignment toolbar' ); + this._fillAlignmentToolbar( this.horizontalAlignmentToolbar, this._horizontalAlignmentLabels, 'horizontalAlignment' ); + + // -- Vertical ----------------------------------------------------- + + this.verticalAlignmentToolbar = new ToolbarView( locale ); + this.verticalAlignmentToolbar.ariaLabel = t( 'Vertical text alignment toolbar' ); + this._fillAlignmentToolbar( this.verticalAlignmentToolbar, this._verticalAlignmentLabels, 'verticalAlignment' ); + } + + /** + * + */ + _createActionButtons() { + const locale = this.locale; + const t = this.t; + + /** + * The Save button view. + * + * @member {module:ui/button/buttonview~ButtonView} + */ + const saveButtonView = this.saveButtonView = new ButtonView( locale ); + + saveButtonView.set( { + label: t( 'Save' ), + icon: checkIcon, + class: 'ck-button-save', + type: 'submit', + withText: true, + } ); + + /** + * The Cancel button view. + * + * @member {module:ui/button/buttonview~ButtonView} + */ + const cancelButtonView = this.cancelButtonView = new ButtonView( locale ); + + cancelButtonView.set( { + label: t( 'Cancel' ), + icon: cancelIcon, + class: 'ck-button-cancel', + type: 'cancel', + withText: true, + } ); + + cancelButtonView.delegate( 'execute' ).to( this, 'cancel' ); + } + + /** + * TODO + */ + _getBorderStyleDefinitions() { + const itemDefinitions = new Collection(); + + for ( const style in this._borderStyleLabels ) { + const definition = { + type: 'button', + model: new Model( { + _borderStyleValue: style, + label: this._borderStyleLabels[ style ], + withText: true, + } ) + }; + + definition.model.bind( 'isOn' ).to( this, 'borderStyle', value => { + return value === style; + } ); + + itemDefinitions.add( definition ); + } + + return itemDefinitions; + } + + /** + * TODO + * + * @param {*} toolbar + * @param {*} labels + * @param {*} propertyName + */ + _fillAlignmentToolbar( toolbar, labels, propertyName ) { + for ( const alignment in labels ) { + const button = createAlignmentButton( + this.locale, + labels[ alignment ], + ALIGNMENT_ICONS[ alignment ] + ); + + button.bind( 'isOn' ).to( this, propertyName, value => { + return value === alignment; + } ); + + button.on( 'execute', () => { + // Update the UI. + this[ propertyName ] = alignment; + + // Update the editor model. + this.fire( 'update', { + [ propertyName ]: alignment + } ); + } ); + + toolbar.items.add( button ); + } + } + + /** + * TODO + */ + get _borderStyleLabels() { + const t = this.t; + + return { + none: t( 'None' ), + solid: t( 'Solid' ), + dotted: t( 'Dotted' ), + dashed: t( 'Dashed' ), + double: t( 'Double' ), + groove: t( 'Groove' ), + ridge: t( 'Ridge' ), + inset: t( 'Inset' ), + outset: t( 'Outset' ), + }; + } + + /** + * TODO + */ + get _horizontalAlignmentLabels() { + const t = this.t; + + return { + left: t( 'Align cell text to the left' ), + center: t( 'Align cell text to the center' ), + right: t( 'Align cell text to the right' ), + justify: t( 'Justify cell text' ), + }; + } + + /** + * TODO + */ + get _verticalAlignmentLabels() { + const t = this.t; + + return { + top: t( 'Align cell text to the top' ), + middle: t( 'Align cell text to the middle' ), + bottom: t( 'Align cell text to the bottom' ) + }; + } +} + +function createAlignmentButton( locale, label, icon ) { + const button = new ButtonView( locale ); + + button.set( { + label, + icon, + } ); + + return button; +} + +function createFormRowDefinition( { + children, + className, + ariaLabelledBy +} ) { + const def = { + tag: 'div', + attributes: { + class: [ + 'ck', + 'ck-form__row' + ] + }, + children + }; + + // Note: Flexbox does not work on fieldset elements in Chrome + // (https://bugs.chromium.org/p/chromium/issues/detail?id=375693). + // This is why "role" is used and the label has an id. It's a hack but better than nothing. + if ( ariaLabelledBy ) { + const id = `ck-editor__aria-label_${ uid() }`; + + ariaLabelledBy.extendTemplate( { + attributes: { + id + } + } ); + + def.attributes.role = 'group'; + def.attributes[ 'aria-labelledby' ] = id; + } + + if ( className ) { + def.attributes.class.push( className ); + } + + return def; +} diff --git a/src/ui/utils.js b/src/ui/utils.js new file mode 100644 index 00000000..4c0aaea9 --- /dev/null +++ b/src/ui/utils.js @@ -0,0 +1,54 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module table/ui/utils + */ + +import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview'; +import { getTableWidgetAncestor } from '../utils'; + +/** + * A helper utility that positions the + * {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon contextual balloon} instance + * with respect to the table in the editor content, if one is selected. + * + * @param {module:core/editor/editor~Editor} editor The editor instance. + */ +export function repositionContextualBalloon( editor ) { + const balloon = editor.plugins.get( 'ContextualBalloon' ); + + if ( getTableWidgetAncestor( editor.editing.view.document.selection ) ) { + const position = getBalloonPositionData( editor ); + + balloon.updatePosition( position ); + } +} + +/** + * Returns the positioning options that control the geometry of the + * {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon contextual balloon} with respect + * to the selected element in the editor content. + * + * @param {module:core/editor/editor~Editor} editor The editor instance. + * @returns {module:utils/dom/position~Options} + */ +export function getBalloonPositionData( editor ) { + const editingView = editor.editing.view; + const defaultPositions = BalloonPanelView.defaultPositions; + const modelWidget = getTableWidgetAncestor( editor.editing.view.document.selection ); + + return { + target: editingView.domConverter.viewToDom( modelWidget ), + positions: [ + defaultPositions.northArrowSouth, + defaultPositions.northArrowSouthWest, + defaultPositions.northArrowSouthEast, + defaultPositions.southArrowNorth, + defaultPositions.southArrowNorthWest, + defaultPositions.southArrowNorthEast + ] + }; +} diff --git a/tests/manual/tableproperties.js b/tests/manual/tableproperties.js index 04092ec6..70887c51 100644 --- a/tests/manual/tableproperties.js +++ b/tests/manual/tableproperties.js @@ -13,7 +13,6 @@ import Indent from '@ckeditor/ckeditor5-indent/src/indent'; import TableProperties from '../../src/tableproperties'; import TableCellProperties from '../../src/tablecellproperties'; import TableColumnRowProperties from '../../src/tablecolumnrowproperties'; -import TableStyleUI from '../../src/tablestyleui'; const sourceElement = document.querySelector( '#editor' ); const clonedSource = sourceElement.cloneNode( true ); @@ -22,13 +21,16 @@ document.querySelector( '#cloned-source' ).append( ...clonedSource.childNodes ); ClassicEditor .create( sourceElement, { - plugins: [ ArticlePluginSet, Alignment, Indent, IndentBlock, TableProperties, TableColumnRowProperties, TableCellProperties, - TableStyleUI ], + plugins: [ + ArticlePluginSet, Alignment, Indent, IndentBlock, + + TableProperties, TableColumnRowProperties, TableCellProperties + ], toolbar: [ 'heading', '|', 'insertTable', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' ], table: { - contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells', 'tableCellStyle' ], + contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells', 'tableProperties', 'tableCellProperties' ], tableToolbar: [ 'bold', 'italic' ] } } ) diff --git a/theme/form.css b/theme/form.css new file mode 100644 index 00000000..670cb97f --- /dev/null +++ b/theme/form.css @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +.ck.ck-form { + padding: 0 0 var(--ck-spacing-large); + + &:focus { + /* https://github.com/ckeditor/ckeditor5-link/issues/90 */ + outline: none; + } + + & .ck.ck-form__header { + font-weight: bold; + padding: 0 var(--ck-spacing-large); + height: 38px; + line-height: 38px; + border-bottom: 1px solid var(--ck-color-base-border); + } + + & .ck.ck-input-text { + min-width: 100%; + width: 0; + } + + & .ck.ck-dropdown { + min-width: 100%; + + & .ck-dropdown__button { + &:not(:focus) { + border: 1px solid var(--ck-color-base-border); + } + + & .ck-button__label { + width: 4em; + } + } + } + + & .ck-form__row { + display: flex; + flex-direction: row; + flex-wrap: nowrap; + justify-content: space-between; + + padding: var(--ck-spacing-standard) var(--ck-spacing-large) 0; + + /* Ignore labels that work as fieldset legends */ + & > *:not(.ck-label) { + flex-grow: 1; + + & + * { + padding-left: var(--ck-spacing-large); + } + } + + &.ck-table-cell-properties-form__action-row { + & .ck-button-save, + & .ck-button-cancel { + justify-content: center; + } + + & .ck-button .ck-button__label { + color: var(--ck-color-text); + } + } + } +} diff --git a/theme/icons/align-bottom.svg b/theme/icons/align-bottom.svg new file mode 100644 index 00000000..6136224c --- /dev/null +++ b/theme/icons/align-bottom.svg @@ -0,0 +1 @@ + diff --git a/theme/icons/align-middle.svg b/theme/icons/align-middle.svg new file mode 100644 index 00000000..99cb5bb4 --- /dev/null +++ b/theme/icons/align-middle.svg @@ -0,0 +1 @@ + diff --git a/theme/icons/align-top.svg b/theme/icons/align-top.svg new file mode 100644 index 00000000..a6f8e947 --- /dev/null +++ b/theme/icons/align-top.svg @@ -0,0 +1 @@ + diff --git a/theme/icons/table-cell-properties.svg b/theme/icons/table-cell-properties.svg new file mode 100644 index 00000000..4b11c643 --- /dev/null +++ b/theme/icons/table-cell-properties.svg @@ -0,0 +1 @@ + diff --git a/theme/icons/table-properties.svg b/theme/icons/table-properties.svg new file mode 100644 index 00000000..363821a8 --- /dev/null +++ b/theme/icons/table-properties.svg @@ -0,0 +1 @@ + diff --git a/theme/tablecellproperties.css b/theme/tablecellproperties.css new file mode 100644 index 00000000..d6cb79a0 --- /dev/null +++ b/theme/tablecellproperties.css @@ -0,0 +1,84 @@ +/* + * Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +.ck.ck-table-cell-properties-form { + width: 280px; + + & .ck-form__row { + &.ck-table-cell-properties-form__border-row { + flex-wrap: wrap; + + & > .ck.ck-label { + width: 100%; + min-width: 100%; + } + + & .ck-labeled-input, + & .ck-labeled-dropdown { + display: flex; + flex-direction: column-reverse; + align-items: center; + + & .ck-label { + font-size: 10px; + } + } + + & .ck-labeled-dropdown { + flex-grow: 0; + } + + & .ck-table-cell-properties-form__border-style { + width: 80px; + min-width: 80px; + } + + & .ck-table-cell-properties-form__border-width { + width: 55px; + min-width: 55px; + flex-grow: 0; + } + } + + & .ck-table-cell-properties-form__padding { + width: 135px; + min-width: 135px; + flex-grow: 0; + } + + &.ck-table-cell-properties-form__alignment-row { + flex-wrap: wrap; + + & > .ck.ck-label { + width: 100%; + min-width: 100%; + } + + & .ck.ck-toolbar { + padding: 0; + background: 0; + flex-grow: 0; + + & .ck-toolbar__items > * { + margin: 0; + + &:first-child { + border-top-right-radius: 0; + border-bottom-right-radius: 0; + } + + &:last-child { + border-top-left-radius: 0; + border-bottom-left-radius: 0; + } + + &:not(:first-child):not(:last-child) { + border-radius: 0; + } + } + } + } + } +} From 1c236cea4ebcc74ef11ad754257192dbd75fdaf7 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 14 Jan 2020 12:23:01 +0100 Subject: [PATCH 02/33] Added some space to the table cell form. Attached the form to the edited cell. --- src/tablecellpropertiesui.js | 14 ++++++++------ src/ui/tablecellpropertiesview.js | 6 +++--- src/ui/utils.js | 16 +++++++++++----- theme/form.css | 4 +++- theme/tablecellproperties.css | 12 +++--------- 5 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/tablecellpropertiesui.js b/src/tablecellpropertiesui.js index 90a7510b..7d90d0df 100644 --- a/src/tablecellpropertiesui.js +++ b/src/tablecellpropertiesui.js @@ -14,7 +14,7 @@ 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, getBalloonPositionData } from './ui/utils'; +import { repositionContextualBalloon, getBalloonCellPositionData } from './ui/utils'; import { findAncestor } from './commands/utils'; const DEFAULT_BORDER_STYLE = 'none'; @@ -235,7 +235,7 @@ export default class TableCellPropertiesUI extends Plugin { if ( !this._isViewInBalloon ) { this._balloon.add( { view: this.view, - position: getBalloonPositionData( editor ) + position: getBalloonCellPositionData( editor ) } ); } @@ -310,8 +310,10 @@ function unifyQuadDirectionPropertyValue( value ) { return; } - // Unify width to one value. If different values are set default to top (or right, etc). - value = value.top || value.right || value.bottom || value.left; - - return value; + // Unify to one value. If different values are set default to top (or right, etc). + for ( const prop in value ) { + if ( value[ prop ] && value[ prop ] !== 'none' ) { + return value[ prop ]; + } + } } diff --git a/src/ui/tablecellpropertiesview.js b/src/ui/tablecellpropertiesview.js index 6c2d5261..bf22bdfb 100644 --- a/src/ui/tablecellpropertiesview.js +++ b/src/ui/tablecellpropertiesview.js @@ -172,16 +172,16 @@ export default class TableCellPropertiesView extends View { this.borderStyleDropdown, ] }, + this.borderColorInput, this.borderWidthInput, - this.borderColorInput ] } ), // Background & Padding createFormRowDefinition( { children: [ - this.paddingInput, this.backgroundInput, + this.paddingInput, ] } ), @@ -198,7 +198,7 @@ export default class TableCellPropertiesView extends View { // Action buttons createFormRowDefinition( { - className: 'ck-table-cell-properties-form__action-row', + className: 'ck-table-form__action-row', children: [ this.saveButtonView, this.cancelButtonView diff --git a/src/ui/utils.js b/src/ui/utils.js index 4c0aaea9..58a510af 100644 --- a/src/ui/utils.js +++ b/src/ui/utils.js @@ -9,6 +9,7 @@ import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview'; import { getTableWidgetAncestor } from '../utils'; +import { findAncestor } from '../commands/utils'; /** * A helper utility that positions the @@ -21,7 +22,7 @@ export function repositionContextualBalloon( editor ) { const balloon = editor.plugins.get( 'ContextualBalloon' ); if ( getTableWidgetAncestor( editor.editing.view.document.selection ) ) { - const position = getBalloonPositionData( editor ); + const position = getBalloonCellPositionData( editor ); balloon.updatePosition( position ); } @@ -30,18 +31,23 @@ export function repositionContextualBalloon( editor ) { /** * Returns the positioning options that control the geometry of the * {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon contextual balloon} with respect - * to the selected element in the editor content. + * to the selected table cell in the editor content. * * @param {module:core/editor/editor~Editor} editor The editor instance. * @returns {module:utils/dom/position~Options} */ -export function getBalloonPositionData( editor ) { +export function getBalloonCellPositionData( editor ) { + const model = editor.model; + const document = model.document; + const selection = document.selection; const editingView = editor.editing.view; + const firstPosition = selection.getFirstPosition(); + const modelTableCell = findAncestor( 'tableCell', firstPosition ); + const viewTableCell = editor.editing.mapper.toViewElement( modelTableCell ); const defaultPositions = BalloonPanelView.defaultPositions; - const modelWidget = getTableWidgetAncestor( editor.editing.view.document.selection ); return { - target: editingView.domConverter.viewToDom( modelWidget ), + target: editingView.domConverter.viewToDom( viewTableCell ), positions: [ defaultPositions.northArrowSouth, defaultPositions.northArrowSouthWest, diff --git a/theme/form.css b/theme/form.css index 670cb97f..a18832d3 100644 --- a/theme/form.css +++ b/theme/form.css @@ -55,7 +55,9 @@ } } - &.ck-table-cell-properties-form__action-row { + &.ck-table-form__action-row { + margin-top: var(--ck-spacing-large); + & .ck-button-save, & .ck-button-cancel { justify-content: center; diff --git a/theme/tablecellproperties.css b/theme/tablecellproperties.css index d6cb79a0..d461a712 100644 --- a/theme/tablecellproperties.css +++ b/theme/tablecellproperties.css @@ -4,7 +4,7 @@ */ .ck.ck-table-cell-properties-form { - width: 280px; + width: 320px; & .ck-form__row { &.ck-table-cell-properties-form__border-row { @@ -36,18 +36,12 @@ } & .ck-table-cell-properties-form__border-width { - width: 55px; - min-width: 55px; + width: 70px; + min-width: 70px; flex-grow: 0; } } - & .ck-table-cell-properties-form__padding { - width: 135px; - min-width: 135px; - flex-grow: 0; - } - &.ck-table-cell-properties-form__alignment-row { flex-wrap: wrap; From ab6f0cf58f7a423a71b76d7dfc2d256e149aed48 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 14 Jan 2020 12:59:00 +0100 Subject: [PATCH 03/33] Code refactoring. --- src/tablecellpropertiesui.js | 5 +- src/ui/formrowview.js | 82 ++++++++++++++ src/ui/tablecellpropertiesview.js | 170 ++++++++++++------------------ 3 files changed, 154 insertions(+), 103 deletions(-) create mode 100644 src/ui/formrowview.js diff --git a/src/tablecellpropertiesui.js b/src/tablecellpropertiesui.js index 7d90d0df..7a533439 100644 --- a/src/tablecellpropertiesui.js +++ b/src/tablecellpropertiesui.js @@ -271,10 +271,9 @@ export default class TableCellPropertiesUI extends Plugin { editor.editing.view.focus(); if ( this._isViewInBalloon ) { - // TODO below - // Blur the input element before removing it from DOM to prevent issues in some browsers. + // Blur any input element before removing it from DOM to prevent issues in some browsers. // See https://github.com/ckeditor/ckeditor5/issues/1501. - // this.formView.saveButtonView.focus(); + this.view.saveButtonView.focus(); this._balloon.remove( this.view ); diff --git a/src/ui/formrowview.js b/src/ui/formrowview.js new file mode 100644 index 00000000..f7f38d4a --- /dev/null +++ b/src/ui/formrowview.js @@ -0,0 +1,82 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module table/ui/formrowview + */ + +import View from '@ckeditor/ckeditor5-ui/src/view'; + +/** + * TODO + * + * @extends module:ui/view~View + */ +export default class FormRowView extends View { + /** + * @inheritDoc + */ + constructor( locale, options = {} ) { + super( locale ); + + const bind = this.bindTemplate; + + /** + * An additional CSS class added to the {@link #element}. + * + * @observable + * @member {String} #class + */ + this.set( 'class', options.class || null ); + + /** + * TODO + * + * @observable + * @member {String} #role + */ + this.set( '_role', null ); + + /** + * TODO + * + * @member {String} #ariaLabelledBy + */ + this.set( '_ariaLabelledBy', null ); + + if ( options.labelView ) { + this.set( { + _role: 'group', + _ariaLabelledBy: options.labelView.id + } ); + } + + /** + * A collection of row items (buttons, dropdowns, etc.). + * + * @readonly + * @member {module:ui/viewcollection~ViewCollection} + */ + this.children = this.createCollection(); + + if ( options.children ) { + options.children.forEach( child => this.children.add( child ) ); + } + + this.setTemplate( { + tag: 'div', + attributes: { + class: [ + 'ck', + 'ck-form__row', + bind.to( 'class' ) + ], + role: bind.to( '_role' ), + 'aria-labelledby': bind.to( '_ariaLabelledBy' ) + }, + children: this.children + } ); + } +} diff --git a/src/ui/tablecellpropertiesview.js b/src/ui/tablecellpropertiesview.js index bf22bdfb..b5618768 100644 --- a/src/ui/tablecellpropertiesview.js +++ b/src/ui/tablecellpropertiesview.js @@ -24,8 +24,6 @@ import { createDropdown, addListToDropdown } from '@ckeditor/ckeditor5-ui/src/dr import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; -import uid from '@ckeditor/ckeditor5-utils/src/uid'; - import checkIcon from '@ckeditor/ckeditor5-core/theme/icons/check.svg'; import cancelIcon from '@ckeditor/ckeditor5-core/theme/icons/cancel.svg'; @@ -41,6 +39,7 @@ import alignBottomIcon from '../../theme/icons/align-bottom.svg'; import '../../theme/form.css'; import '../../theme/tablecellproperties.css'; +import FormRowView from './formrowview'; const ALIGNMENT_ICONS = { left: alignLeftIcon, @@ -125,6 +124,45 @@ export default class TableCellPropertiesView extends View { verticalAlignment: 'middle' } ); + const borderRowView = new FormRowView( locale, { + labelView: this.borderRowLabel, + children: [ + this.borderRowLabel, + this.borderStyleDropdown, + this.borderColorInput, + this.borderWidthInput + ] + } ); + + borderRowView.class = 'ck-table-cell-properties-form__border-row'; + + const paddingBackgroundRowView = new FormRowView( locale, { + children: [ + this.backgroundInput, + this.paddingInput, + ] + } ); + + const alignmentRowView = new FormRowView( locale, { + labelView: this.alignmentLabel, + children: [ + this.alignmentLabel, + this.horizontalAlignmentToolbar, + this.verticalAlignmentToolbar, + ] + } ); + + alignmentRowView.class = 'ck-table-cell-properties-form__alignment-row'; + + const actionRowView = new FormRowView( locale, { + children: [ + this.saveButtonView, + this.cancelButtonView, + ] + } ); + + actionRowView.class = 'ck-table-form__action-row'; + this.setTemplate( { tag: 'form', attributes: { @@ -150,60 +188,10 @@ export default class TableCellPropertiesView extends View { ] }, - // Border - createFormRowDefinition( { - ariaLabelledBy: this.borderRowLabel, - className: 'ck-table-cell-properties-form__border-row', - children: [ - this.borderRowLabel, - - // TODO: This should become a new component or be integrated into LabeledInputView. - { - tag: 'div', - attributes: { - class: [ - 'ck', - 'ck-labeled-dropdown', - 'ck-table-cell-properties-form__border-style' - ], - }, - children: [ - this.borderStyleDropdownLabel, - this.borderStyleDropdown, - ] - }, - this.borderColorInput, - this.borderWidthInput, - ] - } ), - - // Background & Padding - createFormRowDefinition( { - children: [ - this.backgroundInput, - this.paddingInput, - ] - } ), - - // Alignment - createFormRowDefinition( { - ariaLabelledBy: this.alignmentLabel, - className: 'ck-table-cell-properties-form__alignment-row', - children: [ - this.alignmentLabel, - this.horizontalAlignmentToolbar, - this.verticalAlignmentToolbar - ] - } ), - - // Action buttons - createFormRowDefinition( { - className: 'ck-table-form__action-row', - children: [ - this.saveButtonView, - this.cancelButtonView - ] - } ) + borderRowView, + paddingBackgroundRowView, + alignmentRowView, + actionRowView ] } ); } @@ -220,10 +208,10 @@ export default class TableCellPropertiesView extends View { const focusableChildViews = [ this.borderStyleDropdown, - this.borderWidthInput, this.borderColorInput, - this.paddingInput, + this.borderWidthInput, this.backgroundInput, + this.paddingInput, this.horizontalAlignmentToolbar, this.verticalAlignmentToolbar, this.saveButtonView, @@ -259,7 +247,7 @@ export default class TableCellPropertiesView extends View { // -- Style --------------------------------------------------- - const borderStyleDropdown = this.borderStyleDropdown = createDropdown( locale ); + const borderStyleDropdown = createDropdown( locale ); borderStyleDropdown.buttonView.set( { isOn: false, withText: true, @@ -284,8 +272,29 @@ export default class TableCellPropertiesView extends View { addListToDropdown( borderStyleDropdown, this._getBorderStyleDefinitions() ); - this.borderStyleDropdownLabel = new LabelView( locale ); - this.borderStyleDropdownLabel.text = t( 'Style' ); + const borderStyleLabel = new LabelView( locale ); + borderStyleLabel.text = t( 'Style' ); + + // TODO: This should become a new component or be integrated into LabeledInputView. + this.borderStyleDropdown = new View( locale ); + this.borderStyleDropdown.setTemplate( { + tag: 'div', + attributes: { + class: [ + 'ck', + 'ck-labeled-dropdown', + 'ck-table-cell-properties-form__border-style' + ], + }, + children: [ + borderStyleLabel, + borderStyleDropdown, + ] + } ); + + this.borderStyleDropdown.focus = () => { + borderStyleDropdown.focus(); + }; // -- Width --------------------------------------------------- @@ -540,42 +549,3 @@ function createAlignmentButton( locale, label, icon ) { return button; } - -function createFormRowDefinition( { - children, - className, - ariaLabelledBy -} ) { - const def = { - tag: 'div', - attributes: { - class: [ - 'ck', - 'ck-form__row' - ] - }, - children - }; - - // Note: Flexbox does not work on fieldset elements in Chrome - // (https://bugs.chromium.org/p/chromium/issues/detail?id=375693). - // This is why "role" is used and the label has an id. It's a hack but better than nothing. - if ( ariaLabelledBy ) { - const id = `ck-editor__aria-label_${ uid() }`; - - ariaLabelledBy.extendTemplate( { - attributes: { - id - } - } ); - - def.attributes.role = 'group'; - def.attributes[ 'aria-labelledby' ] = id; - } - - if ( className ) { - def.attributes.class.push( className ); - } - - return def; -} From b96df24bf2267e922383936a07efff0d9926226e Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 14 Jan 2020 16:48:03 +0100 Subject: [PATCH 04/33] Used LabeledView in the cell properties form. --- src/tablecellpropertiesui.js | 8 +-- src/ui/tablecellpropertiesview.js | 90 +++++++++++++------------------ theme/form.css | 2 +- theme/tablecellproperties.css | 21 +++++--- 4 files changed, 55 insertions(+), 66 deletions(-) diff --git a/src/tablecellpropertiesui.js b/src/tablecellpropertiesui.js index 7a533439..9ffeb251 100644 --- a/src/tablecellpropertiesui.js +++ b/src/tablecellpropertiesui.js @@ -209,10 +209,10 @@ export default class TableCellPropertiesUI extends Plugin { const view = this.view; - view.borderWidthInput.inputView.element.value = ''; - view.borderColorInput.inputView.element.value = ''; - view.paddingInput.inputView.element.value = ''; - view.backgroundInput.inputView.element.value = ''; + view.borderWidthInput.view.element.value = + view.borderColorInput.view.element.value = + view.paddingInput.view.element.value = + view.backgroundInput.view.element.value = ''; view.set( { borderWidth, diff --git a/src/ui/tablecellpropertiesview.js b/src/ui/tablecellpropertiesview.js index b5618768..74304655 100644 --- a/src/ui/tablecellpropertiesview.js +++ b/src/ui/tablecellpropertiesview.js @@ -17,10 +17,10 @@ import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler'; import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; import FocusCycler from '@ckeditor/ckeditor5-ui/src/focuscycler'; -import InputTextView from '@ckeditor/ckeditor5-ui/src/inputtext/inputtextview'; -import LabeledInputView from '@ckeditor/ckeditor5-ui/src/labeledinput/labeledinputview'; +import LabeledView from '@ckeditor/ckeditor5-ui/src/labeledview/labeledview'; +import { labeledInputCreator, labeledDropdownCreator } from '@ckeditor/ckeditor5-ui/src/labeledview/creators'; import LabelView from '@ckeditor/ckeditor5-ui/src/label/labelview'; -import { createDropdown, addListToDropdown } from '@ckeditor/ckeditor5-ui/src/dropdown/utils'; +import { addListToDropdown } from '@ckeditor/ckeditor5-ui/src/dropdown/utils'; import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; @@ -247,18 +247,23 @@ export default class TableCellPropertiesView extends View { // -- Style --------------------------------------------------- - const borderStyleDropdown = createDropdown( locale ); - borderStyleDropdown.buttonView.set( { + const borderStyleDropdown = this.borderStyleDropdown = new LabeledView( locale, labeledDropdownCreator ); + borderStyleDropdown.set( { + label: t( 'Style' ), + class: 'ck-table-cell-properties-form__border-style' + } ); + + borderStyleDropdown.view.buttonView.set( { isOn: false, withText: true, tooltip: t( 'Style' ) } ); - borderStyleDropdown.buttonView.bind( 'label' ).to( this, 'borderStyle', value => { + borderStyleDropdown.view.buttonView.bind( 'label' ).to( this, 'borderStyle', value => { return this._borderStyleLabels[ value ]; } ); - borderStyleDropdown.on( 'execute', evt => { + borderStyleDropdown.view.on( 'execute', evt => { const value = evt.source._borderStyleValue; // Update the UI. @@ -270,63 +275,39 @@ export default class TableCellPropertiesView extends View { } ); } ); - addListToDropdown( borderStyleDropdown, this._getBorderStyleDefinitions() ); - - const borderStyleLabel = new LabelView( locale ); - borderStyleLabel.text = t( 'Style' ); - - // TODO: This should become a new component or be integrated into LabeledInputView. - this.borderStyleDropdown = new View( locale ); - this.borderStyleDropdown.setTemplate( { - tag: 'div', - attributes: { - class: [ - 'ck', - 'ck-labeled-dropdown', - 'ck-table-cell-properties-form__border-style' - ], - }, - children: [ - borderStyleLabel, - borderStyleDropdown, - ] - } ); - - this.borderStyleDropdown.focus = () => { - borderStyleDropdown.focus(); - }; + addListToDropdown( borderStyleDropdown.view, this._getBorderStyleDefinitions() ); // -- Width --------------------------------------------------- - const borderWidthInput = this.borderWidthInput = new LabeledInputView( locale, InputTextView ); + const borderWidthInput = this.borderWidthInput = new LabeledView( locale, labeledInputCreator ); borderWidthInput.set( { label: t( 'Width' ), class: 'ck-table-cell-properties-form__border-width', } ); - borderWidthInput.bind( 'value' ).to( this, 'borderWidth' ); - borderWidthInput.bind( 'isReadOnly' ).to( this, 'borderStyle', value => { - return value === 'none'; + borderWidthInput.view.bind( 'value' ).to( this, 'borderWidth' ); + borderWidthInput.bind( 'isEnabled' ).to( this, 'borderStyle', value => { + return value !== 'none'; } ); - borderWidthInput.inputView.on( 'input', () => { + borderWidthInput.view.on( 'input', () => { this.fire( 'update', { - borderWidth: borderWidthInput.inputView.element.value + borderWidth: borderWidthInput.view.element.value } ); } ); // -- Color --------------------------------------------------- - const borderColorInput = this.borderColorInput = new LabeledInputView( locale, InputTextView ); + const borderColorInput = this.borderColorInput = new LabeledView( locale, labeledInputCreator ); borderColorInput.label = t( 'Color' ); - borderColorInput.bind( 'value' ).to( this, 'borderColor' ); - borderColorInput.bind( 'isReadOnly' ).to( this, 'borderStyle', value => { - return value === 'none'; + borderColorInput.view.bind( 'value' ).to( this, 'borderColor' ); + borderColorInput.bind( 'isEnabled' ).to( this, 'borderStyle', value => { + return value !== 'none'; } ); - borderColorInput.inputView.on( 'input', () => { + borderColorInput.view.on( 'input', () => { this.fire( 'update', { - borderColor: borderColorInput.inputView.element.value + borderColor: borderColorInput.view.element.value } ); } ); } @@ -337,14 +318,17 @@ export default class TableCellPropertiesView extends View { _createBackgroundField() { const locale = this.locale; const t = this.t; - const backgroundInput = this.backgroundInput = new LabeledInputView( locale, InputTextView ); + const backgroundInput = this.backgroundInput = new LabeledView( locale, labeledInputCreator ); - backgroundInput.label = t( 'Background' ); - backgroundInput.bind( 'value' ).to( this, 'backgroundColor' ); + backgroundInput.set( { + label: t( 'Background' ), + class: 'ck-table-cell-properties-form__background', + } ); - backgroundInput.inputView.on( 'input', () => { + backgroundInput.view.bind( 'value' ).to( this, 'backgroundColor' ); + backgroundInput.view.on( 'input', () => { this.fire( 'update', { - backgroundColor: backgroundInput.inputView.element.value + backgroundColor: backgroundInput.view.element.value } ); } ); } @@ -355,17 +339,17 @@ export default class TableCellPropertiesView extends View { _createPaddingField() { const locale = this.locale; const t = this.t; - const paddingInput = this.paddingInput = new LabeledInputView( locale, InputTextView ); + const paddingInput = this.paddingInput = new LabeledView( locale, labeledInputCreator ); paddingInput.set( { label: t( 'Padding' ), class: 'ck-table-cell-properties-form__padding', } ); - paddingInput.bind( 'value' ).to( this, 'padding' ); - paddingInput.inputView.on( 'input', () => { + paddingInput.view.bind( 'value' ).to( this, 'padding' ); + paddingInput.view.on( 'input', () => { this.fire( 'update', { - padding: paddingInput.inputView.element.value + padding: paddingInput.view.element.value } ); } ); } diff --git a/theme/form.css b/theme/form.css index a18832d3..7315538c 100644 --- a/theme/form.css +++ b/theme/form.css @@ -33,7 +33,7 @@ } & .ck-button__label { - width: 4em; + width: 100%; } } } diff --git a/theme/tablecellproperties.css b/theme/tablecellproperties.css index d461a712..7ce29164 100644 --- a/theme/tablecellproperties.css +++ b/theme/tablecellproperties.css @@ -15,33 +15,38 @@ min-width: 100%; } - & .ck-labeled-input, - & .ck-labeled-dropdown { + & .ck-labeled-view { display: flex; flex-direction: column-reverse; align-items: center; - & .ck-label { + & > .ck-label { font-size: 10px; } - } - & .ck-labeled-dropdown { - flex-grow: 0; + & .ck.ck-dropdown { + flex-grow: 0; + } } & .ck-table-cell-properties-form__border-style { width: 80px; min-width: 80px; + flex-grow: 0; } & .ck-table-cell-properties-form__border-width { - width: 70px; - min-width: 70px; + width: 80px; + min-width: 80px; flex-grow: 0; } } + & .ck-table-cell-properties-form__background, + & .ck-table-cell-properties-form__padding { + width: 50%; + } + &.ck-table-cell-properties-form__alignment-row { flex-wrap: wrap; From 9453c5138b8db4d4ed36e7163a63d990a7d70717 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 15 Jan 2020 10:00:24 +0100 Subject: [PATCH 05/33] Code refactoring. --- src/tablecellpropertiesui.js | 6 +----- src/ui/tablecellpropertiesview.js | 7 +++++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/tablecellpropertiesui.js b/src/tablecellpropertiesui.js index 9ffeb251..280fb14d 100644 --- a/src/tablecellpropertiesui.js +++ b/src/tablecellpropertiesui.js @@ -209,11 +209,7 @@ export default class TableCellPropertiesUI extends Plugin { const view = this.view; - view.borderWidthInput.view.element.value = - view.borderColorInput.view.element.value = - view.paddingInput.view.element.value = - view.backgroundInput.view.element.value = ''; - + view.resetFields(); view.set( { borderWidth, borderColor, diff --git a/src/ui/tablecellpropertiesview.js b/src/ui/tablecellpropertiesview.js index 74304655..095ac751 100644 --- a/src/ui/tablecellpropertiesview.js +++ b/src/ui/tablecellpropertiesview.js @@ -233,6 +233,13 @@ export default class TableCellPropertiesView extends View { this._focusCycler.focusFirst(); } + resetFields() { + this.borderWidthInput.view.element.value = + this.borderColorInput.view.element.value = + this.paddingInput.view.element.value = + this.backgroundInput.view.element.value = ''; + } + /** * TODO */ From afb59d0c401772aeca350f1307a8590c1309764b Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 15 Jan 2020 14:53:19 +0100 Subject: [PATCH 06/33] Code refactoring and docs. --- src/tablecellpropertiesui.js | 25 ++- src/ui/tablecellpropertiesview.js | 289 ++++++++++++++++++------------ 2 files changed, 181 insertions(+), 133 deletions(-) diff --git a/src/tablecellpropertiesui.js b/src/tablecellpropertiesui.js index 280fb14d..75d44415 100644 --- a/src/tablecellpropertiesui.js +++ b/src/tablecellpropertiesui.js @@ -165,7 +165,7 @@ export default class TableCellPropertiesUI extends Plugin { const document = model.document; const selection = document.selection; - this.view.on( 'update', ( evt, data ) => { + this.view.on( 'change', ( evt, property, value ) => { const firstPosition = selection.getFirstPosition(); const tableCell = findAncestor( 'tableCell', firstPosition ); @@ -173,19 +173,15 @@ export default class TableCellPropertiesUI extends Plugin { // as a single undo steps. It's a better UX than dozens of undo steps, e.g. each // for a single value change. editor.model.enqueueChange( this._batch, writer => { - for ( const property in data ) { - const value = data[ property ]; - - if ( QUAD_DIRECTION_ATTRIBUTES.includes( property ) ) { - writer.setAttribute( property, { - top: value, - right: value, - bottom: value, - left: value - }, tableCell ); - } else { - writer.setAttribute( property, value, tableCell ); - } + if ( QUAD_DIRECTION_ATTRIBUTES.includes( property ) ) { + writer.setAttribute( property, { + top: value, + right: value, + bottom: value, + left: value + }, tableCell ); + } else { + writer.setAttribute( property, value, tableCell ); } } ); } ); @@ -209,7 +205,6 @@ export default class TableCellPropertiesUI extends Plugin { const view = this.view; - view.resetFields(); view.set( { borderWidth, borderColor, diff --git a/src/ui/tablecellpropertiesview.js b/src/ui/tablecellpropertiesview.js index 095ac751..3d33b257 100644 --- a/src/ui/tablecellpropertiesview.js +++ b/src/ui/tablecellpropertiesview.js @@ -52,7 +52,8 @@ const ALIGNMENT_ICONS = { }; /** - * TODO + * The class representing a table cell properties form, allowing users to customize + * certain style aspects of a table cell, for instance, border, padding, text alignment, etc.. * * @extends module:ui/view~View */ @@ -79,6 +80,71 @@ export default class TableCellPropertiesView extends View { */ this.keystrokes = new KeystrokeHandler(); + this.set( { + /** + * The value of the cell border style. + * + * @observable + * @default 'none' + * @member #borderStyle + */ + borderStyle: 'none', + + /** + * The value of the cell border width style. + * + * @observable + * @default null + * @member #borderWidth + */ + borderWidth: null, + + /** + * The value of the cell border color style. + * + * @observable + * @default null + * @member #borderColor + */ + borderColor: null, + + /** + * The value of the cell padding style. + * + * @observable + * @default null + * @member #padding + */ + padding: null, + + /** + * The value of the cell background color style. + * + * @observable + * @default null + * @member #backgroundColor + */ + backgroundColor: null, + + /** + * The value of the horizontal text alignment style. + * + * @observable + * @default 'left' + * @member #horizontalAlignment + */ + horizontalAlignment: 'left', + + /** + * The value of the vertical text alignment style. + * + * @observable + * @default 'middle' + * @member #verticalAlignment + */ + verticalAlignment: 'middle' + } ); + /** * A collection of views that can be focused in the form. * @@ -114,55 +180,6 @@ export default class TableCellPropertiesView extends View { this._createAlignmentFields(); this._createActionButtons(); - this.set( { - borderStyle: 'none', - borderWidth: null, - borderColor: null, - padding: null, - backgroundColor: null, - horizontalAlignment: 'left', - verticalAlignment: 'middle' - } ); - - const borderRowView = new FormRowView( locale, { - labelView: this.borderRowLabel, - children: [ - this.borderRowLabel, - this.borderStyleDropdown, - this.borderColorInput, - this.borderWidthInput - ] - } ); - - borderRowView.class = 'ck-table-cell-properties-form__border-row'; - - const paddingBackgroundRowView = new FormRowView( locale, { - children: [ - this.backgroundInput, - this.paddingInput, - ] - } ); - - const alignmentRowView = new FormRowView( locale, { - labelView: this.alignmentLabel, - children: [ - this.alignmentLabel, - this.horizontalAlignmentToolbar, - this.verticalAlignmentToolbar, - ] - } ); - - alignmentRowView.class = 'ck-table-cell-properties-form__alignment-row'; - - const actionRowView = new FormRowView( locale, { - children: [ - this.saveButtonView, - this.cancelButtonView, - ] - } ); - - actionRowView.class = 'ck-table-form__action-row'; - this.setTemplate( { tag: 'form', attributes: { @@ -188,10 +205,38 @@ export default class TableCellPropertiesView extends View { ] }, - borderRowView, - paddingBackgroundRowView, - alignmentRowView, - actionRowView + new FormRowView( locale, { + labelView: this._borderRowLabel, + children: [ + this._borderRowLabel, + this.borderStyleDropdown, + this.borderColorInput, + this.borderWidthInput + ], + class: 'ck-table-cell-properties-form__border-row' + } ), + new FormRowView( locale, { + children: [ + this.backgroundInput, + this.paddingInput, + ] + } ), + new FormRowView( locale, { + labelView: this._alignmentLabel, + children: [ + this._alignmentLabel, + this.horizontalAlignmentToolbar, + this.verticalAlignmentToolbar, + ], + class: 'ck-table-cell-properties-form__alignment-row' + } ), + new FormRowView( locale, { + children: [ + this.saveButtonView, + this.cancelButtonView, + ], + class: 'ck-table-form__action-row' + } ) ] } ); } @@ -202,11 +247,13 @@ export default class TableCellPropertiesView extends View { render() { super.render(); + // Enable the "submit" event for this view. It can be triggered by the #saveButtonView + // which is of the "submit" DOM "type". submitHandler( { view: this } ); - const focusableChildViews = [ + [ this.borderStyleDropdown, this.borderColorInput, this.borderWidthInput, @@ -216,9 +263,7 @@ export default class TableCellPropertiesView extends View { this.verticalAlignmentToolbar, this.saveButtonView, this.cancelButtonView - ]; - - focusableChildViews.forEach( v => { + ].forEach( v => { // Register the view as focusable. this._focusables.add( v ); @@ -226,22 +271,25 @@ export default class TableCellPropertiesView extends View { this.focusTracker.add( v.element ); } ); + // Mainly for closing using "Esc" and navigation using "Tab". this.keystrokes.listenTo( this.element ); } + /** + * Focuses the fist focusable field in the form. + */ focus() { this._focusCycler.focusFirst(); } - resetFields() { - this.borderWidthInput.view.element.value = - this.borderColorInput.view.element.value = - this.paddingInput.view.element.value = - this.backgroundInput.view.element.value = ''; - } - /** - * TODO + * Creates the following form fields: + * + * * {@link #borderStyleDropdown}, + * * {@link #borderWidthInput}, + * * {@link #borderColorInput}. + * + * @private */ _createBorderFields() { const locale = this.locale; @@ -249,7 +297,7 @@ export default class TableCellPropertiesView extends View { // -- Group label --------------------------------------------- - const borderRowLabel = this.borderRowLabel = new LabelView( locale ); + const borderRowLabel = this._borderRowLabel = new LabelView( locale ); borderRowLabel.text = t( 'Border' ); // -- Style --------------------------------------------------- @@ -275,11 +323,6 @@ export default class TableCellPropertiesView extends View { // Update the UI. this.borderStyle = value; - - // Update the editor model. - this.fire( 'update', { - borderStyle: evt.source._borderStyleValue - } ); } ); addListToDropdown( borderStyleDropdown.view, this._getBorderStyleDefinitions() ); @@ -298,9 +341,7 @@ export default class TableCellPropertiesView extends View { return value !== 'none'; } ); borderWidthInput.view.on( 'input', () => { - this.fire( 'update', { - borderWidth: borderWidthInput.view.element.value - } ); + this.borderWidth = borderWidthInput.view.element.value; } ); // -- Color --------------------------------------------------- @@ -313,14 +354,16 @@ export default class TableCellPropertiesView extends View { } ); borderColorInput.view.on( 'input', () => { - this.fire( 'update', { - borderColor: borderColorInput.view.element.value - } ); + this.borderColor = borderColorInput.view.element.value; } ); } /** - * TODO + * Creates the following form fields: + * + * * {@link #backgroundInput}. + * + * @private */ _createBackgroundField() { const locale = this.locale; @@ -334,14 +377,16 @@ export default class TableCellPropertiesView extends View { backgroundInput.view.bind( 'value' ).to( this, 'backgroundColor' ); backgroundInput.view.on( 'input', () => { - this.fire( 'update', { - backgroundColor: backgroundInput.view.element.value - } ); + this.backgroundColor = backgroundInput.view.element.value; } ); } /** - * TODO + * Creates the following form fields: + * + * * {@link #paddingInput}. + * + * @private */ _createPaddingField() { const locale = this.locale; @@ -355,21 +400,24 @@ export default class TableCellPropertiesView extends View { paddingInput.view.bind( 'value' ).to( this, 'padding' ); paddingInput.view.on( 'input', () => { - this.fire( 'update', { - padding: paddingInput.view.element.value - } ); + this.padding = paddingInput.view.element.value; } ); } /** - * TODO + * Creates the following form fields: + * + * * {@link #horizontalAlignmentToolbar}, + * * {@link #verticalAlignmentToolbar}. + * + * @private */ _createAlignmentFields() { const locale = this.locale; const t = this.t; - this.alignmentLabel = new LabelView( locale ); - this.alignmentLabel.text = t( 'Text alignment' ); + this._alignmentLabel = new LabelView( locale ); + this._alignmentLabel.text = t( 'Text alignment' ); // -- Horizontal --------------------------------------------------- @@ -385,7 +433,12 @@ export default class TableCellPropertiesView extends View { } /** + * Creates the following form controls: * + * * {@link #saveButtonView}, + * * {@link #cancelButtonView}. + * + * @private */ _createActionButtons() { const locale = this.locale; @@ -425,7 +478,10 @@ export default class TableCellPropertiesView extends View { } /** - * TODO + * Provides a set of {@link #borderStyleDropdown} item definitions. + * + * @private + * @returns {Iterable.} */ _getBorderStyleDefinitions() { const itemDefinitions = new Collection(); @@ -451,19 +507,23 @@ export default class TableCellPropertiesView extends View { } /** - * TODO + * Fills an alignment (either horizontal or vertical) with buttons + * that have certain labels and interact with a certain view property + * upon execution. * - * @param {*} toolbar - * @param {*} labels - * @param {*} propertyName + * @private + * @param {module:ui/toolbar/toolbarview~ToolbarView} toolbar + * @param {Array.} labels + * @param {String} propertyName */ _fillAlignmentToolbar( toolbar, labels, propertyName ) { for ( const alignment in labels ) { - const button = createAlignmentButton( - this.locale, - labels[ alignment ], - ALIGNMENT_ICONS[ alignment ] - ); + const button = new ButtonView( this.locale ); + + button.set( { + label: labels[ alignment ], + icon: ALIGNMENT_ICONS[ alignment ], + } ); button.bind( 'isOn' ).to( this, propertyName, value => { return value === alignment; @@ -472,11 +532,6 @@ export default class TableCellPropertiesView extends View { button.on( 'execute', () => { // Update the UI. this[ propertyName ] = alignment; - - // Update the editor model. - this.fire( 'update', { - [ propertyName ]: alignment - } ); } ); toolbar.items.add( button ); @@ -484,7 +539,10 @@ export default class TableCellPropertiesView extends View { } /** - * TODO + * Provides localized labels for {@link #borderStyleDropdown} items. + * + * @private + * @type {Object.} */ get _borderStyleLabels() { const t = this.t; @@ -503,7 +561,10 @@ export default class TableCellPropertiesView extends View { } /** - * TODO + * Provides localized labels for {@link #horizontalAlignmentToolbar} buttons. + * + * @private + * @type {Object.} */ get _horizontalAlignmentLabels() { const t = this.t; @@ -517,7 +578,10 @@ export default class TableCellPropertiesView extends View { } /** - * TODO + * Provides localized labels for {@link #verticalAlignmentToolbar} buttons. + * + * @private + * @type {Object.} */ get _verticalAlignmentLabels() { const t = this.t; @@ -529,14 +593,3 @@ export default class TableCellPropertiesView extends View { }; } } - -function createAlignmentButton( locale, label, icon ) { - const button = new ButtonView( locale ); - - button.set( { - label, - icon, - } ); - - return button; -} From b9f2c491c5086bd2ca461de3468ca7fc35748b59 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 15 Jan 2020 15:23:30 +0100 Subject: [PATCH 07/33] Moved alignment icons to ckeditor5-core. Updated paths to icons. --- src/ui/tablecellpropertiesview.js | 17 +++++++---------- theme/icons/align-bottom.svg | 1 - theme/icons/align-middle.svg | 1 - theme/icons/align-top.svg | 1 - 4 files changed, 7 insertions(+), 13 deletions(-) delete mode 100644 theme/icons/align-bottom.svg delete mode 100644 theme/icons/align-middle.svg delete mode 100644 theme/icons/align-top.svg diff --git a/src/ui/tablecellpropertiesview.js b/src/ui/tablecellpropertiesview.js index 3d33b257..6b30637a 100644 --- a/src/ui/tablecellpropertiesview.js +++ b/src/ui/tablecellpropertiesview.js @@ -26,16 +26,13 @@ import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; import checkIcon from '@ckeditor/ckeditor5-core/theme/icons/check.svg'; import cancelIcon from '@ckeditor/ckeditor5-core/theme/icons/cancel.svg'; - -// TODO: These **must** be transferred to ckeditor5-core. -import alignLeftIcon from '@ckeditor/ckeditor5-alignment/theme/icons/align-left.svg'; -import alignRightIcon from '@ckeditor/ckeditor5-alignment/theme/icons/align-right.svg'; -import alignCenterIcon from '@ckeditor/ckeditor5-alignment/theme/icons/align-center.svg'; -import alignJustifyIcon from '@ckeditor/ckeditor5-alignment/theme/icons/align-justify.svg'; - -import alignTopIcon from '../../theme/icons/align-top.svg'; -import alignMiddleIcon from '../../theme/icons/align-middle.svg'; -import alignBottomIcon from '../../theme/icons/align-bottom.svg'; +import alignLeftIcon from '@ckeditor/ckeditor5-core/theme/icons/align-left.svg'; +import alignRightIcon from '@ckeditor/ckeditor5-core/theme/icons/align-right.svg'; +import alignCenterIcon from '@ckeditor/ckeditor5-core/theme/icons/align-center.svg'; +import alignJustifyIcon from '@ckeditor/ckeditor5-core/theme/icons/align-justify.svg'; +import alignTopIcon from '@ckeditor/ckeditor5-core/theme/icons/align-top.svg'; +import alignMiddleIcon from '@ckeditor/ckeditor5-core/theme/icons/align-middle.svg'; +import alignBottomIcon from '@ckeditor/ckeditor5-core/theme/icons/align-bottom.svg'; import '../../theme/form.css'; import '../../theme/tablecellproperties.css'; diff --git a/theme/icons/align-bottom.svg b/theme/icons/align-bottom.svg deleted file mode 100644 index 6136224c..00000000 --- a/theme/icons/align-bottom.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/theme/icons/align-middle.svg b/theme/icons/align-middle.svg deleted file mode 100644 index 99cb5bb4..00000000 --- a/theme/icons/align-middle.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/theme/icons/align-top.svg b/theme/icons/align-top.svg deleted file mode 100644 index a6f8e947..00000000 --- a/theme/icons/align-top.svg +++ /dev/null @@ -1 +0,0 @@ - From 176d8f73a9115a289d923a0b4e037a25a89db434 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 15 Jan 2020 15:33:07 +0100 Subject: [PATCH 08/33] Docs. --- src/tablecellpropertiesui.js | 42 +++++++++++++++++++++++-------- src/ui/tablecellpropertiesview.js | 6 +---- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/tablecellpropertiesui.js b/src/tablecellpropertiesui.js index 75d44415..6b321d51 100644 --- a/src/tablecellpropertiesui.js +++ b/src/tablecellpropertiesui.js @@ -64,8 +64,8 @@ export default class TableCellPropertiesUI extends Plugin { this._balloon = editor.plugins.get( ContextualBalloon ); /** - * The batch used to undo all changes made by the form (which are live) - * if "Cancel" was pressed. Each time the view is shown, a new batch is created. + * The batch used to undo all changes made by the form (which are live, as the user types) + * when "Cancel" was pressed. Each time the view is shown, a new batch is created. * * @private * @member {module:engine/model/batch~Batch} @@ -123,7 +123,7 @@ export default class TableCellPropertiesUI extends Plugin { */ this.view = new TableCellPropertiesView( editor.locale ); - // Render the view so its #element is available for clickOutsideHandler. + // Render the view so its #element is available for the clickOutsideHandler. this.view.render(); this.listenTo( this.view, 'submit', () => { @@ -159,6 +159,15 @@ export default class TableCellPropertiesUI extends Plugin { } ); } + /** + * In this method the UI -> editor data binding is registered. + * + * Registers a listener that updates the editor model when any observable property of + * the {@link #view} has changed. This makes the view live, which means the changes are + * visible in the editing as soon as the user types or changes fields' values. + * + * @private + */ _startRespondingToChangesInView() { const editor = this.editor; const model = editor.model; @@ -187,6 +196,14 @@ export default class TableCellPropertiesUI extends Plugin { } ); } + /** + * In this method the editor data -> UI binding is created. + * + * When executed, this method obtains the value attribute values of the cell the selection is anchored + * to and passed them to the {@link #view}. This way, the UI stays up–to–date with the editor data. + * + * @private + */ _fillViewFormFromSelectedCell() { const editor = this.editor; const model = editor.model; @@ -203,9 +220,7 @@ export default class TableCellPropertiesUI extends Plugin { const horizontalAlignment = tableCell.getAttribute( 'horizontalAlignment' ) || DEFAULT_HORIZONTAL_ALIGNMENT; const verticalAlignment = tableCell.getAttribute( 'verticalAlignment' ) || DEFAULT_VERTICAL_ALIGNMENT; - const view = this.view; - - view.set( { + this.view.set( { borderWidth, borderColor, borderStyle, @@ -216,6 +231,15 @@ export default class TableCellPropertiesUI extends Plugin { } ); } + /** + * Shows the {@link #view} in the {@link #_balloon}. + * + * **Note**: Each time a view is shown, the new {@link #_batch} is created that contains + * all changes made to the document when the view is visible, allowing a single undo step + * for all of them. + * + * @private + */ _showView() { if ( this._isViewVisible ) { return; @@ -243,9 +267,7 @@ export default class TableCellPropertiesUI extends Plugin { /** * Removes the {@link #view} from the {@link #_balloon}. * - * See {@link #_addFormView}, {@link #_addActionsView}. - * - * @protected + * @private */ _hideView() { if ( !this._isViewInBalloon ) { @@ -275,7 +297,7 @@ export default class TableCellPropertiesUI extends Plugin { } /** - * Returns `true` when the {@link #view} is the visible view in the {@link #_balloon}. + * Returns `true` when the {@link #view} is the visible in the {@link #_balloon}. * * @private * @type {Boolean} diff --git a/src/ui/tablecellpropertiesview.js b/src/ui/tablecellpropertiesview.js index 6b30637a..06244af2 100644 --- a/src/ui/tablecellpropertiesview.js +++ b/src/ui/tablecellpropertiesview.js @@ -316,10 +316,7 @@ export default class TableCellPropertiesView extends View { } ); borderStyleDropdown.view.on( 'execute', evt => { - const value = evt.source._borderStyleValue; - - // Update the UI. - this.borderStyle = value; + this.borderStyle = evt.source._borderStyleValue; } ); addListToDropdown( borderStyleDropdown.view, this._getBorderStyleDefinitions() ); @@ -527,7 +524,6 @@ export default class TableCellPropertiesView extends View { } ); button.on( 'execute', () => { - // Update the UI. this[ propertyName ] = alignment; } ); From df19000f8a75bbcaa48e50cebe670bcbf8e80e9e Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 15 Jan 2020 16:29:54 +0100 Subject: [PATCH 09/33] Docs. --- src/ui/formrowview.js | 51 +++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/src/ui/formrowview.js b/src/ui/formrowview.js index f7f38d4a..10b79dce 100644 --- a/src/ui/formrowview.js +++ b/src/ui/formrowview.js @@ -10,13 +10,25 @@ import View from '@ckeditor/ckeditor5-ui/src/view'; /** - * TODO + * The class representing a single row in the complex form, + * used by {@link module:table/ui/tablecellpropertiesview~TableCellPropertiesView}. * + * **Note**: For now this class is private. When more use cases arrive (beyond ckeditor5-table), + * it will become a component in ckeditor5-ui. + * + * @private * @extends module:ui/view~View */ export default class FormRowView extends View { /** - * @inheritDoc + * Creates an instance of the form row class. + * + * @param {module:utils/locale~Locale} locale The locale instance. + * @param {Object} options + * @param {Array.} options.children + * @param {String} [options.class] + * @param {module:ui/view~View} [options.labelView] When passed, the row gets the `group` and `aria-labelledby` + * DOM attributes and get described by the label. */ constructor( locale, options = {} ) { super( locale ); @@ -32,16 +44,35 @@ export default class FormRowView extends View { this.set( 'class', options.class || null ); /** - * TODO + * A collection of row items (buttons, dropdowns, etc.). + * + * @readonly + * @member {module:ui/viewcollection~ViewCollection} + */ + this.children = this.createCollection(); + + if ( options.children ) { + options.children.forEach( child => this.children.add( child ) ); + } + + /** + * The role property reflected by the `role` DOM attribute of the {@link #element}. + * + * **Note**: Used only when a `labelView` is passed to constructor `options`. * + * @private * @observable * @member {String} #role */ this.set( '_role', null ); /** - * TODO + * The ARIA property reflected by the `aria-labelledby` DOM attribute of the {@link #element}. * + * **Note**: Used only when a `labelView` is passed to constructor `options`. + * + * @private + * @observable * @member {String} #ariaLabelledBy */ this.set( '_ariaLabelledBy', null ); @@ -53,18 +84,6 @@ export default class FormRowView extends View { } ); } - /** - * A collection of row items (buttons, dropdowns, etc.). - * - * @readonly - * @member {module:ui/viewcollection~ViewCollection} - */ - this.children = this.createCollection(); - - if ( options.children ) { - options.children.forEach( child => this.children.add( child ) ); - } - this.setTemplate( { tag: 'div', attributes: { From 128edef3141979e21ab2611493b2609ac5559e25 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 15 Jan 2020 16:31:21 +0100 Subject: [PATCH 10/33] Removed the obsolete TableStyleUI plugin. --- src/tablestyleui.js | 270 -------------------------------------------- 1 file changed, 270 deletions(-) delete mode 100644 src/tablestyleui.js diff --git a/src/tablestyleui.js b/src/tablestyleui.js deleted file mode 100644 index e76d4b49..00000000 --- a/src/tablestyleui.js +++ /dev/null @@ -1,270 +0,0 @@ -/** - * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license - */ - -/** - * @module table/tablestyleui - */ - -import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; -import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; -import { findAncestor } from './commands/utils'; -import TableUI from './tableui'; -import tableMergeCellIcon from '../theme/icons/table-merge-cell.svg'; - -/** - * The table style UI feature. - * - * @extends module:core/plugin~Plugin - */ -export default class TableStyleUI extends Plugin { - /** - * @inheritDoc - */ - init() { - const editor = this.editor; - - this._setupUI(); - - editor.ui.componentFactory.add( 'tableCellStyle', locale => { - const t = this.editor.t; - - const options = [ - { - type: 'ui', - name: 'borderWidth' - }, - { - type: 'ui', - name: 'borderColor' - }, - { - type: 'ui', - name: 'borderStyle' - }, - { type: 'separator' }, - { - type: 'ui', - name: 'backgroundColor' - }, - { - type: 'ui', - name: 'padding' - } - ]; - - const dropdownView = editor.plugins.get( TableUI )._prepareDropdown( t( 'Merge cells' ), tableMergeCellIcon, options, locale ); - - dropdownView.isEnabled = true; - - return dropdownView; - } ); - } - - _setupUI() { - const editor = this.editor; - const model = editor.model; - const document = model.document; - const selection = document.selection; - - const componentFactory = editor.ui.componentFactory; - - componentFactory.add( 'borderWidth', locale => { - const button = new ButtonView( locale ); - - button.set( { - label: 'Border width', - icon: false, - tooltip: true, - withText: true - } ); - - button.on( 'execute', () => { - const firstPosition = selection.getFirstPosition(); - - const tableCell = findAncestor( 'tableCell', firstPosition ); - - const border = tableCell.getAttribute( 'border' ); - - let currentWidth; - - if ( border ) { - const borderWidth = ( border && border.width ) || {}; - - // Unify width to one value. If different values are set default to top (or right, etc). - currentWidth = borderWidth.top || borderWidth.right || borderWidth.bottom || borderWidth.left; - } - - // eslint-disable-next-line no-undef,no-alert - const newWidth = prompt( 'Set border width:', currentWidth || '' ); - - const parts = /^([0-9]*[.]?[0-9]*)([a-z]{2,4})?/.exec( newWidth ); - const unit = parts[ 2 ]; - - const borderWidthToSet = parts[ 1 ] + ( unit || 'px' ); - - // TODO: Command, setting new value is dumb on `border` object. - editor.model.change( writer => { - writer.setAttribute( 'borderWidth', { - top: borderWidthToSet, - right: borderWidthToSet, - bottom: borderWidthToSet, - left: borderWidthToSet - }, tableCell ); - } ); - } ); - - return button; - } ); - - componentFactory.add( 'borderColor', locale => { - const button = new ButtonView( locale ); - - button.set( { - label: 'Border color', - icon: false, - tooltip: true, - withText: true - } ); - - button.on( 'execute', () => { - const firstPosition = selection.getFirstPosition(); - - const tableCell = findAncestor( 'tableCell', firstPosition ); - - const border = tableCell.getAttribute( 'border' ); - - let currentColor; - - if ( border ) { - const borderColor = ( border && border.color ) || {}; - - // Unify width to one value. If different values are set default to top (or right, etc). - currentColor = borderColor.top || borderColor.right || borderColor.bottom || borderColor.left; - } - - // eslint-disable-next-line no-undef,no-alert - const newColor = prompt( 'Set new border color:', currentColor || '' ); - - // TODO: Command, setting new value is dumb on `border` object. - editor.model.change( writer => { - writer.setAttribute( 'borderColor', { - top: newColor, - right: newColor, - bottom: newColor, - left: newColor - }, tableCell ); - } ); - } ); - - return button; - } ); - - componentFactory.add( 'borderStyle', locale => { - const button = new ButtonView( locale ); - - button.set( { - label: 'Border style', - icon: false, - tooltip: true, - withText: true - } ); - - button.on( 'execute', () => { - const firstPosition = selection.getFirstPosition(); - - const tableCell = findAncestor( 'tableCell', firstPosition ); - - const border = tableCell.getAttribute( 'border' ); - - let currentStyle; - - if ( border ) { - const borderStyle = ( border && border.style ) || {}; - - // Unify width to one value. If different values are set default to top (or right, etc). - currentStyle = borderStyle.top || borderStyle.right || borderStyle.bottom || borderStyle.left; - } - - // eslint-disable-next-line no-undef,no-alert - const newStyle = prompt( 'Set new border style:', currentStyle || '' ); - - // TODO: Command, setting new value is dumb on `border` object. - editor.model.change( writer => { - writer.setAttribute( 'borderStyle', { - top: newStyle, - right: newStyle, - bottom: newStyle, - left: newStyle - }, tableCell ); - } ); - } ); - - return button; - } ); - - componentFactory.add( 'backgroundColor', locale => { - const button = new ButtonView( locale ); - - button.set( { - label: 'Background color', - icon: false, - tooltip: true, - withText: true - } ); - - button.on( 'execute', () => { - const firstPosition = selection.getFirstPosition(); - - const tableCell = findAncestor( 'tableCell', firstPosition ); - - const backgroundColor = tableCell.getAttribute( 'backgroundColor' ); - - // eslint-disable-next-line no-undef,no-alert - const newColor = prompt( 'Set new background color:', backgroundColor || '' ); - - editor.model.change( writer => { - writer.setAttribute( 'backgroundColor', newColor, tableCell ); - } ); - } ); - - return button; - } ); - - componentFactory.add( 'padding', locale => { - const button = new ButtonView( locale ); - - button.set( { - label: 'Cell padding', - icon: false, - tooltip: true, - withText: true - } ); - - button.on( 'execute', () => { - const firstPosition = selection.getFirstPosition(); - - const tableCell = findAncestor( 'tableCell', firstPosition ); - - const padding = tableCell.getAttribute( 'padding' ); - - let currentPadding; - - if ( padding ) { - // Unify width to one value. If different values are set default to top (or right, etc). - currentPadding = padding.top || padding.right || padding.bottom || padding.left; - } - - // eslint-disable-next-line no-undef,no-alert - const newPadding = prompt( 'Set new padding:', currentPadding || '' ); - - editor.model.change( writer => { - writer.setAttribute( 'padding', newPadding, tableCell ); - } ); - } ); - - return button; - } ); - } -} From a86246ab0c705ef37254156e89a729cfcb60b213 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jan 2020 13:41:12 +0100 Subject: [PATCH 11/33] Tests: Added TableCellPropertiesView tests. Code refactoring. --- src/ui/tablecellpropertiesview.js | 125 ++++--- tests/ui/tablecellpropertiesview.js | 534 ++++++++++++++++++++++++++++ 2 files changed, 612 insertions(+), 47 deletions(-) create mode 100644 tests/ui/tablecellpropertiesview.js diff --git a/src/ui/tablecellpropertiesview.js b/src/ui/tablecellpropertiesview.js index 06244af2..871cd0aa 100644 --- a/src/ui/tablecellpropertiesview.js +++ b/src/ui/tablecellpropertiesview.js @@ -77,6 +77,14 @@ export default class TableCellPropertiesView extends View { */ 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. @@ -171,12 +179,56 @@ export default class TableCellPropertiesView extends View { } } ); + this._createHeaderView(); this._createBorderFields(); this._createBackgroundField(); this._createPaddingField(); this._createAlignmentFields(); this._createActionButtons(); + // Form header. + this.children.add( this._headerView ); + + // Border row. + this.children.add( new FormRowView( locale, { + labelView: this._borderRowLabel, + children: [ + this._borderRowLabel, + this.borderStyleDropdown, + this.borderColorInput, + this.borderWidthInput + ], + class: 'ck-table-cell-properties-form__border-row' + } ) ); + + // Background and padding row. + this.children.add( new FormRowView( locale, { + children: [ + this.backgroundInput, + this.paddingInput, + ] + } ) ); + + // Text alignment row. + this.children.add( new FormRowView( locale, { + labelView: this._alignmentLabel, + children: [ + this._alignmentLabel, + this.horizontalAlignmentToolbar, + this.verticalAlignmentToolbar, + ], + class: 'ck-table-cell-properties-form__alignment-row' + } ) ); + + // Action row. + this.children.add( new FormRowView( locale, { + children: [ + this.saveButtonView, + this.cancelButtonView, + ], + class: 'ck-table-form__action-row' + } ) ); + this.setTemplate( { tag: 'form', attributes: { @@ -188,53 +240,7 @@ export default class TableCellPropertiesView extends View { // https://github.com/ckeditor/ckeditor5-link/issues/90 tabindex: '-1' }, - children: [ - { - tag: 'div', - attributes: { - class: [ - 'ck', - 'ck-form__header' - ] - }, - children: [ - 'Cell properties' - ] - }, - - new FormRowView( locale, { - labelView: this._borderRowLabel, - children: [ - this._borderRowLabel, - this.borderStyleDropdown, - this.borderColorInput, - this.borderWidthInput - ], - class: 'ck-table-cell-properties-form__border-row' - } ), - new FormRowView( locale, { - children: [ - this.backgroundInput, - this.paddingInput, - ] - } ), - new FormRowView( locale, { - labelView: this._alignmentLabel, - children: [ - this._alignmentLabel, - this.horizontalAlignmentToolbar, - this.verticalAlignmentToolbar, - ], - class: 'ck-table-cell-properties-form__alignment-row' - } ), - new FormRowView( locale, { - children: [ - this.saveButtonView, - this.cancelButtonView, - ], - class: 'ck-table-form__action-row' - } ) - ] + children: this.children } ); } @@ -279,6 +285,31 @@ export default class TableCellPropertiesView extends View { this._focusCycler.focusFirst(); } + /** + * Creates the header of the form with a localized label. + * + * @private + */ + _createHeaderView() { + const locale = this.locale; + const t = this.t; + + this._headerView = new View( locale ); + + this._headerView.setTemplate( { + tag: 'div', + attributes: { + class: [ + 'ck', + 'ck-form__header' + ] + }, + children: [ + t( 'Cell properties' ) + ] + } ); + } + /** * Creates the following form fields: * diff --git a/tests/ui/tablecellpropertiesview.js b/tests/ui/tablecellpropertiesview.js new file mode 100644 index 00000000..67de18ac --- /dev/null +++ b/tests/ui/tablecellpropertiesview.js @@ -0,0 +1,534 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* globals Event */ + +import TableCellPropertiesView from '../../src/ui/tablecellpropertiesview'; +import LabeledView from '@ckeditor/ckeditor5-ui/src/labeledview/labeledview'; +import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; +import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler'; +import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; +import FocusCycler from '@ckeditor/ckeditor5-ui/src/focuscycler'; +import ViewCollection from '@ckeditor/ckeditor5-ui/src/viewcollection'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; +import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; +import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; +import InputTextView from '@ckeditor/ckeditor5-ui/src/inputtext/inputtextview'; + +describe.only( 'TableCellPropertiesView', () => { + let view; + + testUtils.createSinonSandbox(); + + beforeEach( () => { + view = new TableCellPropertiesView( { t: val => val } ); + view.render(); + } ); + + afterEach( () => { + view.destroy(); + } ); + + describe( 'constructor()', () => { + it( 'creates view#children collection', () => { + expect( view.children ).to.be.instanceOf( ViewCollection ); + } ); + + it( 'defines the public data interface (observable properties)', () => { + expect( view ).to.include( { + borderStyle: 'none', + borderWidth: null, + borderColor: null, + padding: null, + backgroundColor: null, + horizontalAlignment: 'left', + verticalAlignment: 'middle' + } ); + } ); + + it( 'should create element from template', () => { + expect( view.element.classList.contains( 'ck' ) ).to.be.true; + expect( view.element.classList.contains( 'ck-form' ) ).to.be.true; + expect( view.element.classList.contains( 'ck-table-cell-properties-form' ) ).to.be.true; + expect( view.element.getAttribute( 'tabindex' ) ).to.equal( '-1' ); + } ); + + it( 'should create child views (and references)', () => { + expect( view.borderStyleDropdown ).to.be.instanceOf( LabeledView ); + expect( view.borderWidthInput ).to.be.instanceOf( LabeledView ); + expect( view.borderColorInput ).to.be.instanceOf( LabeledView ); + expect( view.backgroundInput ).to.be.instanceOf( LabeledView ); + expect( view.paddingInput ).to.be.instanceOf( LabeledView ); + expect( view.horizontalAlignmentToolbar ).to.be.instanceOf( ToolbarView ); + expect( view.verticalAlignmentToolbar ).to.be.instanceOf( ToolbarView ); + + expect( view.saveButtonView ).to.be.instanceOf( ButtonView ); + expect( view.cancelButtonView ).to.be.instanceOf( ButtonView ); + } ); + + it( 'should have a header', () => { + const header = view.element.firstChild; + + expect( header.classList.contains( 'ck' ) ).to.be.true; + expect( header.classList.contains( 'ck-form__header' ) ).to.be.true; + expect( header.textContent ).to.equal( 'Cell properties' ); + } ); + + describe( 'form rows', () => { + describe( 'border row', () => { + it( 'should be defined', () => { + const row = view.element.childNodes[ 1 ]; + + expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; + expect( row.classList.contains( 'ck-table-cell-properties-form__border-row' ) ).to.be.true; + expect( row.childNodes[ 0 ] ).to.equal( view._borderRowLabel.element ); + expect( row.childNodes[ 1 ] ).to.equal( view.borderStyleDropdown.element ); + expect( row.childNodes[ 2 ] ).to.equal( view.borderColorInput.element ); + expect( row.childNodes[ 3 ] ).to.equal( view.borderWidthInput.element ); + } ); + + describe( 'border style labeled dropdown', () => { + let labeledDropdown; + + beforeEach( () => { + labeledDropdown = view.borderStyleDropdown; + } ); + + it( 'should have properties set', () => { + expect( labeledDropdown.label ).to.equal( 'Style' ); + expect( labeledDropdown.class ).to.equal( 'ck-table-cell-properties-form__border-style' ); + } ); + + it( 'should have a button with properties set', () => { + expect( labeledDropdown.view.buttonView.isOn ).to.be.false; + expect( labeledDropdown.view.buttonView.withText ).to.be.true; + expect( labeledDropdown.view.buttonView.tooltip ).to.equal( 'Style' ); + } ); + + it( 'should bind button\'s label to #borderStyle property', () => { + view.borderStyle = 'dotted'; + expect( labeledDropdown.view.buttonView.label ).to.equal( 'Dotted' ); + + view.borderStyle = 'dashed'; + expect( labeledDropdown.view.buttonView.label ).to.equal( 'Dashed' ); + } ); + + it( 'should change #borderStyle when executed', () => { + labeledDropdown.view.listView.items.first.children.first.fire( 'execute' ); + expect( view.borderStyle ).to.equal( 'none' ); + + labeledDropdown.view.listView.items.last.children.first.fire( 'execute' ); + expect( view.borderStyle ).to.equal( 'outset' ); + } ); + + it( 'should come with a set of pre–defined border styles', () => { + expect( labeledDropdown.view.listView.items.map( item => item.children.first.label ) ).to.have.ordered.members( [ + 'None', 'Solid', 'Dotted', 'Dashed', 'Double', 'Groove', 'Ridge', 'Inset', 'Outset', + ] ); + } ); + } ); + + describe( 'border width input', () => { + let labeledInput; + + beforeEach( () => { + labeledInput = view.borderWidthInput; + } ); + + it( 'should be created', () => { + expect( labeledInput.view ).to.be.instanceOf( InputTextView ); + expect( labeledInput.label ).to.equal( 'Width' ); + expect( labeledInput.class ).to.equal( 'ck-table-cell-properties-form__border-width' ); + } ); + + it( 'should reflect #borderWidth property', () => { + view.borderWidth = 'foo'; + expect( labeledInput.view.value ).to.equal( 'foo' ); + + view.borderWidth = 'bar'; + expect( labeledInput.view.value ).to.equal( 'bar' ); + } ); + + it( 'should be enabled only when #borderStyle is different than "none"', () => { + view.borderStyle = 'none'; + expect( labeledInput.isEnabled ).to.be.false; + + view.borderStyle = 'dotted'; + expect( labeledInput.isEnabled ).to.be.true; + } ); + + it( 'should update #borderWidth on DOM "input" event', () => { + labeledInput.view.element.value = 'foo'; + labeledInput.view.fire( 'input' ); + expect( view.borderWidth ).to.equal( 'foo' ); + + labeledInput.view.element.value = 'bar'; + labeledInput.view.fire( 'input' ); + expect( view.borderWidth ).to.equal( 'bar' ); + } ); + } ); + + describe( 'border color input', () => { + let labeledInput; + + beforeEach( () => { + labeledInput = view.borderColorInput; + } ); + + it( 'should be created', () => { + expect( labeledInput.view ).to.be.instanceOf( InputTextView ); + expect( labeledInput.label ).to.equal( 'Color' ); + } ); + + it( 'should reflect #borderColor property', () => { + view.borderColor = 'foo'; + expect( labeledInput.view.value ).to.equal( 'foo' ); + + view.borderColor = 'bar'; + expect( labeledInput.view.value ).to.equal( 'bar' ); + } ); + + it( 'should be enabled only when #borderStyle is different than "none"', () => { + view.borderStyle = 'none'; + expect( labeledInput.isEnabled ).to.be.false; + + view.borderStyle = 'dotted'; + expect( labeledInput.isEnabled ).to.be.true; + } ); + + it( 'should update #borderColor on DOM "input" event', () => { + labeledInput.view.element.value = 'foo'; + labeledInput.view.fire( 'input' ); + expect( view.borderColor ).to.equal( 'foo' ); + + labeledInput.view.element.value = 'bar'; + labeledInput.view.fire( 'input' ); + expect( view.borderColor ).to.equal( 'bar' ); + } ); + } ); + } ); + + describe( 'background and padding row', () => { + it( 'should be defined', () => { + const row = view.element.childNodes[ 2 ]; + + expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; + expect( row.childNodes[ 0 ] ).to.equal( view.backgroundInput.element ); + expect( row.childNodes[ 1 ] ).to.equal( view.paddingInput.element ); + } ); + + describe( 'background color input', () => { + let labeledInput; + + beforeEach( () => { + labeledInput = view.backgroundInput; + } ); + + it( 'should be created', () => { + expect( labeledInput.view ).to.be.instanceOf( InputTextView ); + expect( labeledInput.label ).to.equal( 'Background' ); + expect( labeledInput.class ).to.equal( 'ck-table-cell-properties-form__background' ); + } ); + + it( 'should reflect #backgroundColor property', () => { + view.backgroundColor = 'foo'; + expect( labeledInput.view.value ).to.equal( 'foo' ); + + view.backgroundColor = 'bar'; + expect( labeledInput.view.value ).to.equal( 'bar' ); + } ); + + it( 'should update #backgroundColor on DOM "input" event', () => { + labeledInput.view.element.value = 'foo'; + labeledInput.view.fire( 'input' ); + expect( view.backgroundColor ).to.equal( 'foo' ); + + labeledInput.view.element.value = 'bar'; + labeledInput.view.fire( 'input' ); + expect( view.backgroundColor ).to.equal( 'bar' ); + } ); + } ); + + describe( 'padding input', () => { + let labeledInput; + + beforeEach( () => { + labeledInput = view.paddingInput; + } ); + + it( 'should be created', () => { + expect( labeledInput.view ).to.be.instanceOf( InputTextView ); + expect( labeledInput.label ).to.equal( 'Padding' ); + expect( labeledInput.class ).to.equal( 'ck-table-cell-properties-form__padding' ); + } ); + + it( 'should reflect #padding property', () => { + view.padding = 'foo'; + expect( labeledInput.view.value ).to.equal( 'foo' ); + + view.padding = 'bar'; + expect( labeledInput.view.value ).to.equal( 'bar' ); + } ); + + it( 'should update #padding on DOM "input" event', () => { + labeledInput.view.element.value = 'foo'; + labeledInput.view.fire( 'input' ); + expect( view.padding ).to.equal( 'foo' ); + + labeledInput.view.element.value = 'bar'; + labeledInput.view.fire( 'input' ); + expect( view.padding ).to.equal( 'bar' ); + } ); + } ); + } ); + + describe( 'text alignment row', () => { + it( 'should be defined', () => { + const row = view.element.childNodes[ 3 ]; + + expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; + expect( row.classList.contains( 'ck-table-cell-properties-form__alignment-row' ) ).to.be.true; + expect( row.childNodes[ 0 ] ).to.equal( view._alignmentLabel.element ); + expect( row.childNodes[ 1 ] ).to.equal( view.horizontalAlignmentToolbar.element ); + expect( row.childNodes[ 2 ] ).to.equal( view.verticalAlignmentToolbar.element ); + } ); + + describe( 'horizontal text alignment toolbar', () => { + let toolbar; + + beforeEach( () => { + toolbar = view.horizontalAlignmentToolbar; + } ); + + it( 'should be defined', () => { + expect( toolbar ).to.be.instanceOf( ToolbarView ); + } ); + + it( 'should have an ARIA label', () => { + expect( toolbar.ariaLabel ).to.equal( 'Horizontal text alignment toolbar' ); + } ); + + it( 'should bring alignment buttons', () => { + expect( toolbar.items.map( ( { label } ) => label ) ).to.have.ordered.members( [ + 'Align cell text to the left', + 'Align cell text to the center', + 'Align cell text to the right', + 'Justify cell text', + ] ); + + expect( toolbar.items.map( ( { isOn } ) => isOn ) ).to.have.ordered.members( [ + true, false, false, false + ] ); + } ); + + it( 'should change the #horizontalAlignment value', () => { + toolbar.items.last.fire( 'execute' ); + expect( view.horizontalAlignment ).to.equal( 'justify' ); + expect( toolbar.items.last.isOn ).to.be.true; + + toolbar.items.first.fire( 'execute' ); + expect( view.horizontalAlignment ).to.equal( 'left' ); + expect( toolbar.items.last.isOn ).to.be.false; + expect( toolbar.items.first.isOn ).to.be.true; + } ); + } ); + + describe( 'vertical text alignment toolbar', () => { + let toolbar; + + beforeEach( () => { + toolbar = view.verticalAlignmentToolbar; + } ); + + it( 'should be defined', () => { + expect( toolbar ).to.be.instanceOf( ToolbarView ); + } ); + + it( 'should have an ARIA label', () => { + expect( toolbar.ariaLabel ).to.equal( 'Vertical text alignment toolbar' ); + } ); + + it( 'should bring alignment buttons', () => { + expect( toolbar.items.map( ( { label } ) => label ) ).to.have.ordered.members( [ + 'Align cell text to the top', + 'Align cell text to the middle', + 'Align cell text to the bottom', + ] ); + + expect( toolbar.items.map( ( { isOn } ) => isOn ) ).to.have.ordered.members( [ + false, true, false + ] ); + } ); + + it( 'should change the #verticalAlignment value', () => { + toolbar.items.last.fire( 'execute' ); + expect( view.verticalAlignment ).to.equal( 'bottom' ); + expect( toolbar.items.last.isOn ).to.be.true; + + toolbar.items.first.fire( 'execute' ); + expect( view.verticalAlignment ).to.equal( 'top' ); + expect( toolbar.items.last.isOn ).to.be.false; + expect( toolbar.items.first.isOn ).to.be.true; + } ); + } ); + } ); + + describe( 'action row', () => { + it( 'should be defined', () => { + const row = view.element.childNodes[ 4 ]; + + expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; + expect( row.classList.contains( 'ck-table-form__action-row' ) ).to.be.true; + expect( row.childNodes[ 0 ] ).to.equal( view.saveButtonView.element ); + expect( row.childNodes[ 1 ] ).to.equal( view.cancelButtonView.element ); + } ); + + it( 'should have buttons with right properties', () => { + expect( view.saveButtonView.label ).to.equal( 'Save' ); + expect( view.saveButtonView.type ).to.equal( 'submit' ); + expect( view.saveButtonView.withText ).to.be.true; + expect( view.saveButtonView.class ).to.equal( 'ck-button-save' ); + + expect( view.cancelButtonView.label ).to.equal( 'Cancel' ); + expect( view.cancelButtonView.withText ).to.be.true; + expect( view.cancelButtonView.class ).to.equal( 'ck-button-cancel' ); + } ); + + it( 'should make the cancel button fire the #cancel event when executed', () => { + const spy = sinon.spy(); + + view.on( 'cancel', spy ); + + view.cancelButtonView.fire( 'execute' ); + + expect( spy.calledOnce ).to.be.true; + } ); + } ); + } ); + + it( 'should create #focusTracker instance', () => { + expect( view.focusTracker ).to.be.instanceOf( FocusTracker ); + } ); + + it( 'should create #keystrokes instance', () => { + expect( view.keystrokes ).to.be.instanceOf( KeystrokeHandler ); + } ); + + it( 'should create #_focusCycler instance', () => { + expect( view._focusCycler ).to.be.instanceOf( FocusCycler ); + } ); + + it( 'should create #_focusables view collection', () => { + expect( view._focusables ).to.be.instanceOf( ViewCollection ); + } ); + } ); + + describe( 'render()', () => { + it( 'should register child views in #_focusables', () => { + expect( view._focusables.map( f => f ) ).to.have.members( [ + view.borderStyleDropdown, + view.borderColorInput, + view.borderWidthInput, + view.backgroundInput, + view.paddingInput, + view.horizontalAlignmentToolbar, + view.verticalAlignmentToolbar, + view.saveButtonView, + view.cancelButtonView + ] ); + } ); + + it( 'should register child views\' #element in #focusTracker', () => { + const spy = testUtils.sinon.spy( FocusTracker.prototype, 'add' ); + const view = new TableCellPropertiesView( { t: val => val } ); + view.render(); + + sinon.assert.calledWith( spy, view.borderStyleDropdown.element ); + sinon.assert.calledWith( spy, view.borderColorInput.element ); + sinon.assert.calledWith( spy, view.borderWidthInput.element ); + sinon.assert.calledWith( spy, view.backgroundInput.element ); + sinon.assert.calledWith( spy, view.paddingInput.element ); + sinon.assert.calledWith( spy, view.horizontalAlignmentToolbar.element ); + sinon.assert.calledWith( spy, view.verticalAlignmentToolbar.element ); + sinon.assert.calledWith( spy, view.saveButtonView.element ); + sinon.assert.calledWith( spy, view.cancelButtonView.element ); + + view.destroy(); + } ); + + it( 'starts listening for #keystrokes coming from #element', () => { + const view = new TableCellPropertiesView( { t: val => val } ); + const spy = sinon.spy( view.keystrokes, 'listenTo' ); + + view.render(); + sinon.assert.calledOnce( spy ); + sinon.assert.calledWithExactly( spy, view.element ); + } ); + + describe( 'activates keyboard navigation for the form', () => { + it( 'so "tab" focuses the next focusable item', () => { + const keyEvtData = { + keyCode: keyCodes.tab, + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; + + // Mock the border style dropdown button is focused. + view.focusTracker.isFocused = true; + view.focusTracker.focusedElement = view.borderStyleDropdown.element; + + const spy = sinon.spy( view.borderColorInput, 'focus' ); + + view.keystrokes.press( keyEvtData ); + sinon.assert.calledOnce( keyEvtData.preventDefault ); + sinon.assert.calledOnce( keyEvtData.stopPropagation ); + sinon.assert.calledOnce( spy ); + } ); + + it( 'so "shift + tab" focuses the previous focusable item', () => { + const keyEvtData = { + keyCode: keyCodes.tab, + shiftKey: true, + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; + + // Mock the border style dropdown button is focused. + view.focusTracker.isFocused = true; + view.focusTracker.focusedElement = view.borderStyleDropdown.element; + + const spy = sinon.spy( view.cancelButtonView, 'focus' ); + + view.keystrokes.press( keyEvtData ); + sinon.assert.calledOnce( keyEvtData.preventDefault ); + sinon.assert.calledOnce( keyEvtData.stopPropagation ); + sinon.assert.calledOnce( spy ); + } ); + } ); + } ); + + describe( 'DOM bindings', () => { + describe( 'submit event', () => { + it( 'should trigger submit event', () => { + const spy = sinon.spy(); + + view.on( 'submit', spy ); + view.element.dispatchEvent( new Event( 'submit' ) ); + + expect( spy.calledOnce ).to.be.true; + } ); + } ); + } ); + + describe( 'focus()', () => { + it( 'focuses the #borderStyleDropdown', () => { + const spy = sinon.spy( view.borderStyleDropdown, 'focus' ); + + view.focus(); + + sinon.assert.calledOnce( spy ); + } ); + } ); +} ); From 268a598689f0cf9fc2fc9e0da0d93a10c297c8e3 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jan 2020 15:04:12 +0100 Subject: [PATCH 12/33] Tests: Added tests for the FormRowView. --- tests/ui/formrowview.js | 96 +++++++++++++++++++++++++++++ tests/ui/tablecellpropertiesview.js | 15 +++-- 2 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 tests/ui/formrowview.js diff --git a/tests/ui/formrowview.js b/tests/ui/formrowview.js new file mode 100644 index 00000000..d9d17795 --- /dev/null +++ b/tests/ui/formrowview.js @@ -0,0 +1,96 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import View from '@ckeditor/ckeditor5-ui/src/view'; +import FormRowView from '../../src/ui/formrowview'; +import ViewCollection from '@ckeditor/ckeditor5-ui/src/viewcollection'; + +describe.only( 'FormRowView', () => { + let view, locale; + + beforeEach( () => { + locale = { t: val => val }; + view = new FormRowView( locale ); + view.render(); + } ); + + afterEach( () => { + view.element.remove(); + } ); + + describe( 'constructor()', () => { + it( 'should set view#locale', () => { + expect( view.locale ).to.equal( locale ); + } ); + + it( 'should create view#children collection', () => { + expect( view.children ).to.be.instanceOf( ViewCollection ); + expect( view.children ).to.have.length( 0 ); + } ); + + it( 'should set view#class', () => { + expect( view.class ).to.be.null; + } ); + + it( 'should set the template', () => { + expect( view.element.classList.contains( 'ck' ) ).to.be.true; + expect( view.element.classList.contains( 'ck-form__row' ) ).to.be.true; + } ); + + describe( 'options', () => { + it( 'should set view#class when class was passed', () => { + const view = new FormRowView( locale, { + class: 'foo' + } ); + + expect( view.class ).to.equal( 'foo' ); + + view.destroy(); + } ); + + it( 'should fill view#children when children were passed', () => { + const view = new FormRowView( locale, { + children: [ + new View() + ] + } ); + + expect( view.children ).to.have.length( 1 ); + + view.destroy(); + } ); + + it( 'should use a label view when passed', () => { + const labelView = new View(); + labelView.id = '123'; + + const view = new FormRowView( locale, { + labelView + } ); + + view.render(); + + expect( view.element.getAttribute( 'role' ) ).to.equal( 'group' ); + expect( view.element.getAttribute( 'aria-labelledby' ) ).to.equal( '123' ); + } ); + } ); + + describe( 'template bindings', () => { + it( 'should bind #class to the template', () => { + view.class = 'foo'; + expect( view.element.classList.contains( 'foo' ) ).to.be.true; + } ); + + it( 'should bind #children to the template', () => { + const child = new View(); + child.setTemplate( { tag: 'div' } ); + + view.children.add( child ); + + expect( view.element.firstChild ).to.equal( child.element ); + } ); + } ); + } ); +} ); diff --git a/tests/ui/tablecellpropertiesview.js b/tests/ui/tablecellpropertiesview.js index 67de18ac..d90cfd0a 100644 --- a/tests/ui/tablecellpropertiesview.js +++ b/tests/ui/tablecellpropertiesview.js @@ -17,13 +17,14 @@ import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; import InputTextView from '@ckeditor/ckeditor5-ui/src/inputtext/inputtextview'; -describe.only( 'TableCellPropertiesView', () => { - let view; +describe( 'TableCellPropertiesView', () => { + let view, locale; testUtils.createSinonSandbox(); beforeEach( () => { - view = new TableCellPropertiesView( { t: val => val } ); + locale = { t: val => val }; + view = new TableCellPropertiesView( locale ); view.render(); } ); @@ -32,11 +33,15 @@ describe.only( 'TableCellPropertiesView', () => { } ); describe( 'constructor()', () => { - it( 'creates view#children collection', () => { + it( 'should set view#locale', () => { + expect( view.locale ).to.equal( locale ); + } ); + + it( 'should create view#children collection', () => { expect( view.children ).to.be.instanceOf( ViewCollection ); } ); - it( 'defines the public data interface (observable properties)', () => { + it( 'should define the public data interface (observable properties)', () => { expect( view ).to.include( { borderStyle: 'none', borderWidth: null, From e1b263f3274321b7ca4ed49bcd931e85717e1f84 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jan 2020 15:16:06 +0100 Subject: [PATCH 13/33] Tests: Added TableCellPropertiesUI presence assertion to TableCellProperties tests. --- tests/tablecellproperties.js | 20 ++++++++++++++++---- tests/ui/formrowview.js | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/tests/tablecellproperties.js b/tests/tablecellproperties.js index c43247fc..a05808ef 100644 --- a/tests/tablecellproperties.js +++ b/tests/tablecellproperties.js @@ -3,21 +3,28 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ +/* global document */ + import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; -import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import TableEditing from '../src/tableediting'; import TableCellProperties from '../src/tablecellproperties'; +import TableCellPropertiesUI from '../src/tablecellpropertiesui'; import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; import { assertTableCellStyle, assertTRBLAttribute } from './_utils/utils'; describe( 'TableCellProperties', () => { - let editor, model; + let element, editor, model; beforeEach( () => { - return VirtualTestEditor - .create( { + element = document.createElement( 'div' ); + + document.body.appendChild( element ); + + return ClassicTestEditor + .create( element, { plugins: [ TableCellProperties, Paragraph, TableEditing ] } ) .then( newEditor => { @@ -29,12 +36,17 @@ describe( 'TableCellProperties', () => { afterEach( () => { editor.destroy(); + element.remove(); } ); it( 'should have pluginName', () => { expect( TableCellProperties.pluginName ).to.equal( 'TableCellProperties' ); } ); + it( 'should require TableCellPropertiesUI', () => { + expect( TableCellProperties.requires ).to.deep.equal( [ TableCellPropertiesUI ] ); + } ); + describe( 'border', () => { it( 'should set proper schema rules', () => { expect( model.schema.checkAttribute( [ '$root', 'tableCell' ], 'borderColor' ) ).to.be.true; diff --git a/tests/ui/formrowview.js b/tests/ui/formrowview.js index d9d17795..56bfeb6a 100644 --- a/tests/ui/formrowview.js +++ b/tests/ui/formrowview.js @@ -7,7 +7,7 @@ import View from '@ckeditor/ckeditor5-ui/src/view'; import FormRowView from '../../src/ui/formrowview'; import ViewCollection from '@ckeditor/ckeditor5-ui/src/viewcollection'; -describe.only( 'FormRowView', () => { +describe( 'FormRowView', () => { let view, locale; beforeEach( () => { From 158179d91dec238210d439029d2df5ead6338693 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jan 2020 15:28:14 +0100 Subject: [PATCH 14/33] Tests: Added tests for UI utils. --- tests/ui/utils.js | 115 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 tests/ui/utils.js diff --git a/tests/ui/utils.js b/tests/ui/utils.js new file mode 100644 index 00000000..e477bfbe --- /dev/null +++ b/tests/ui/utils.js @@ -0,0 +1,115 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; +import Table from '../../src/table'; +import TableCellProperties from '../../src/tablecellproperties'; +import global from '@ckeditor/ckeditor5-utils/src/dom/global'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import View from '@ckeditor/ckeditor5-ui/src/view'; +import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview'; +import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import { repositionContextualBalloon, getBalloonCellPositionData } from '../../src/ui/utils'; +import { findAncestor } from '../../src/commands/utils'; + +describe( 'Utils', () => { + let editor, editingView, balloon, editorElement; + + beforeEach( () => { + editorElement = global.document.createElement( 'div' ); + global.document.body.appendChild( editorElement ); + + return ClassicEditor + .create( editorElement, { + plugins: [ Table, TableCellProperties, Paragraph ] + } ) + .then( newEditor => { + editor = newEditor; + editingView = editor.editing.view; + balloon = editor.plugins.get( 'ContextualBalloon' ); + } ); + } ); + + afterEach( () => { + editorElement.remove(); + + return editor.destroy(); + } ); + + describe( 'repositionContextualBalloon', () => { + it( 'should re-position the ContextualBalloon when the table cell is selected', () => { + const spy = sinon.spy( balloon, 'updatePosition' ); + const defaultPositions = BalloonPanelView.defaultPositions; + const view = new View(); + + view.element = global.document.createElement( 'div' ); + + balloon.add( { + view, + position: { + target: global.document.body + } + } ); + + setData( editor.model, + '' + + 'foo' + + '[bar]' + + '
' ); + repositionContextualBalloon( editor ); + + const modelCell = findAncestor( 'tableCell', editor.model.document.selection.getFirstPosition() ); + const viewCell = editor.editing.mapper.toViewElement( modelCell ); + + sinon.assert.calledWithExactly( spy, { + target: editingView.domConverter.viewToDom( viewCell ), + positions: [ + defaultPositions.northArrowSouth, + defaultPositions.northArrowSouthWest, + defaultPositions.northArrowSouthEast, + defaultPositions.southArrowNorth, + defaultPositions.southArrowNorthWest, + defaultPositions.southArrowNorthEast + ] + } ); + } ); + + it( 'should not engage with no table is selected', () => { + const spy = sinon.spy( balloon, 'updatePosition' ); + + setData( editor.model, 'foo' ); + + repositionContextualBalloon( editor ); + sinon.assert.notCalled( spy ); + } ); + } ); + + describe( 'getBalloonCellPositionData', () => { + it( 'returns the position data', () => { + const defaultPositions = BalloonPanelView.defaultPositions; + + setData( editor.model, '' + + 'foo' + + '[bar]' + + '
' ); + + const data = getBalloonCellPositionData( editor ); + const modelCell = findAncestor( 'tableCell', editor.model.document.selection.getFirstPosition() ); + const viewCell = editor.editing.mapper.toViewElement( modelCell ); + + expect( data ).to.deep.equal( { + target: editingView.domConverter.viewToDom( viewCell ), + positions: [ + defaultPositions.northArrowSouth, + defaultPositions.northArrowSouthWest, + defaultPositions.northArrowSouthEast, + defaultPositions.southArrowNorth, + defaultPositions.southArrowNorthWest, + defaultPositions.southArrowNorthEast + ] + } ); + } ); + } ); +} ); From 19963df738a390cc78753fc920b2f1e45c6cbab6 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 17 Jan 2020 15:30:07 +0100 Subject: [PATCH 15/33] Code refactoring. --- src/tableproperties.js | 8 ------ src/tablepropertiesui.js | 45 --------------------------------- tests/manual/tableproperties.js | 2 +- 3 files changed, 1 insertion(+), 54 deletions(-) delete mode 100644 src/tablepropertiesui.js diff --git a/src/tableproperties.js b/src/tableproperties.js index a87784e6..dfce8b6f 100644 --- a/src/tableproperties.js +++ b/src/tableproperties.js @@ -8,7 +8,6 @@ */ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; -import TablePropertiesUI from './tablepropertiesui'; import { downcastTableAttribute, upcastAttribute, upcastBorderStyles } from './tableproperties/utils'; /** @@ -24,13 +23,6 @@ export default class TableProperties extends Plugin { return 'TableProperties'; } - /** - * @inheritDoc - */ - static get requires() { - return [ TablePropertiesUI ]; - } - /** * @inheritDoc */ diff --git a/src/tablepropertiesui.js b/src/tablepropertiesui.js deleted file mode 100644 index 8a4d5700..00000000 --- a/src/tablepropertiesui.js +++ /dev/null @@ -1,45 +0,0 @@ -/** - * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license - */ - -/** - * @module table/tablepropertiesui - */ - -import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; -import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; - -import tableProperties from './../theme/icons/table-properties.svg'; - -/** - * TODO - * - * @extends module:core/plugin~Plugin - */ -export default class TablePropertiesUI extends Plugin { - /** - * @inheritDoc - */ - init() { - const editor = this.editor; - const t = editor.t; - - editor.ui.componentFactory.add( 'tableProperties', locale => { - const view = new ButtonView( locale ); - - view.set( { - label: t( 'Table properties' ), - icon: tableProperties, - tooltip: true - } ); - - this.listenTo( view, 'execute', () => this._showUI() ); - - return view; - } ); - } - - _showUI() { - } -} diff --git a/tests/manual/tableproperties.js b/tests/manual/tableproperties.js index 70887c51..860848b0 100644 --- a/tests/manual/tableproperties.js +++ b/tests/manual/tableproperties.js @@ -30,7 +30,7 @@ ClassicEditor 'heading', '|', 'insertTable', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' ], table: { - contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells', 'tableProperties', 'tableCellProperties' ], + contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells', 'tableCellProperties' ], tableToolbar: [ 'bold', 'italic' ] } } ) From a19f0c975a3fe575e75529621df995ad0482cfea Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 20 Jan 2020 14:13:46 +0100 Subject: [PATCH 16/33] Aligned cell properties UI to the latest UI framework API. --- src/ui/tablecellpropertiesview.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ui/tablecellpropertiesview.js b/src/ui/tablecellpropertiesview.js index 871cd0aa..86667044 100644 --- a/src/ui/tablecellpropertiesview.js +++ b/src/ui/tablecellpropertiesview.js @@ -18,7 +18,7 @@ import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; import FocusCycler from '@ckeditor/ckeditor5-ui/src/focuscycler'; import LabeledView from '@ckeditor/ckeditor5-ui/src/labeledview/labeledview'; -import { labeledInputCreator, labeledDropdownCreator } from '@ckeditor/ckeditor5-ui/src/labeledview/creators'; +import { createLabeledInputText, createLabeledDropdown } from '@ckeditor/ckeditor5-ui/src/labeledview/utils'; import LabelView from '@ckeditor/ckeditor5-ui/src/label/labelview'; import { addListToDropdown } from '@ckeditor/ckeditor5-ui/src/dropdown/utils'; import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; @@ -330,7 +330,7 @@ export default class TableCellPropertiesView extends View { // -- Style --------------------------------------------------- - const borderStyleDropdown = this.borderStyleDropdown = new LabeledView( locale, labeledDropdownCreator ); + const borderStyleDropdown = this.borderStyleDropdown = new LabeledView( locale, createLabeledDropdown ); borderStyleDropdown.set( { label: t( 'Style' ), class: 'ck-table-cell-properties-form__border-style' @@ -354,7 +354,7 @@ export default class TableCellPropertiesView extends View { // -- Width --------------------------------------------------- - const borderWidthInput = this.borderWidthInput = new LabeledView( locale, labeledInputCreator ); + const borderWidthInput = this.borderWidthInput = new LabeledView( locale, createLabeledInputText ); borderWidthInput.set( { label: t( 'Width' ), @@ -371,7 +371,7 @@ export default class TableCellPropertiesView extends View { // -- Color --------------------------------------------------- - const borderColorInput = this.borderColorInput = new LabeledView( locale, labeledInputCreator ); + const borderColorInput = this.borderColorInput = new LabeledView( locale, createLabeledInputText ); borderColorInput.label = t( 'Color' ); borderColorInput.view.bind( 'value' ).to( this, 'borderColor' ); borderColorInput.bind( 'isEnabled' ).to( this, 'borderStyle', value => { @@ -393,7 +393,7 @@ export default class TableCellPropertiesView extends View { _createBackgroundField() { const locale = this.locale; const t = this.t; - const backgroundInput = this.backgroundInput = new LabeledView( locale, labeledInputCreator ); + const backgroundInput = this.backgroundInput = new LabeledView( locale, createLabeledInputText ); backgroundInput.set( { label: t( 'Background' ), @@ -416,7 +416,7 @@ export default class TableCellPropertiesView extends View { _createPaddingField() { const locale = this.locale; const t = this.t; - const paddingInput = this.paddingInput = new LabeledView( locale, labeledInputCreator ); + const paddingInput = this.paddingInput = new LabeledView( locale, createLabeledInputText ); paddingInput.set( { label: t( 'Padding' ), From b8461a916e83038cf363063b295f1f0c574d1968 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 21 Jan 2020 10:53:21 +0100 Subject: [PATCH 17/33] Tests: Partial TableCellPropertiesUI tests. --- src/tablecellpropertiesui.js | 33 ++++++----- tests/tablecellpropertiesui.js | 101 +++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 17 deletions(-) create mode 100644 tests/tablecellpropertiesui.js diff --git a/src/tablecellpropertiesui.js b/src/tablecellpropertiesui.js index 6b321d51..91b02be2 100644 --- a/src/tablecellpropertiesui.js +++ b/src/tablecellpropertiesui.js @@ -63,6 +63,13 @@ export default class TableCellPropertiesUI extends Plugin { */ this._balloon = editor.plugins.get( ContextualBalloon ); + /** + * The properties form view displayed inside the balloon. + * + * @member {module:table/ui/tablecellpropertiesview~TableCellPropertiesView} + */ + this.view = this._createPropertiesView(); + /** * The batch used to undo all changes made by the form (which are live, as the user types) * when "Cancel" was pressed. Each time the view is shown, a new batch is created. @@ -72,9 +79,6 @@ export default class TableCellPropertiesUI extends Plugin { */ this._batch = null; - // Create the view that displays the properties form. - this._createPropertiesView(); - // Make the form dynamic, i.e. create bindings between view fields and the model. this._startRespondingToChangesInView(); @@ -113,30 +117,23 @@ export default class TableCellPropertiesUI extends Plugin { */ _createPropertiesView() { const editor = this.editor; - const view = editor.editing.view; - const viewDocument = view.document; - - /** - * The properties form view displayed inside the balloon. - * - * @member {module:table/ui/tablecellpropertiesview~TableCellPropertiesView} - */ - this.view = new TableCellPropertiesView( editor.locale ); + const viewDocument = editor.editing.view.document; + const view = new TableCellPropertiesView( editor.locale ); // Render the view so its #element is available for the clickOutsideHandler. - this.view.render(); + view.render(); - this.listenTo( this.view, 'submit', () => { + this.listenTo( view, 'submit', () => { this._hideView(); } ); - this.listenTo( this.view, 'cancel', () => { + this.listenTo( view, 'cancel', () => { editor.execute( 'undo', this._batch ); this._hideView(); } ); // Close the balloon on Esc key press when the **form has focus**. - this.view.keystrokes.set( 'Esc', ( data, cancel ) => { + view.keystrokes.set( 'Esc', ( data, cancel ) => { this._hideView(); cancel(); } ); @@ -152,11 +149,13 @@ export default class TableCellPropertiesUI extends Plugin { // Close on click outside of balloon panel element. clickOutsideHandler( { - emitter: this.view, + emitter: view, activator: () => this._isViewInBalloon, contextElements: [ this._balloon.view.element ], callback: () => this._hideView() } ); + + return view; } /** diff --git a/tests/tablecellpropertiesui.js b/tests/tablecellpropertiesui.js new file mode 100644 index 00000000..54215f90 --- /dev/null +++ b/tests/tablecellpropertiesui.js @@ -0,0 +1,101 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* globals document */ + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; +// import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; +// import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +// import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; + +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon'; + +import Table from '@ckeditor/ckeditor5-table/src/table'; +import TableCellPropertiesUI from '@ckeditor/ckeditor5-table/src/tablecellpropertiesui'; +import TableCellPropertiesUIView from '@ckeditor/ckeditor5-table/src/ui/tablecellpropertiesview'; +import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; +// import View from '@ckeditor/ckeditor5-ui/src/view'; + +describe.only( 'TableCellPropertiesUI', () => { + let editor, editorElement, contextualBalloon, + tableCellPropertiesUI, tableCellPropertiesButton; + + testUtils.createSinonSandbox(); + + beforeEach( () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor + .create( editorElement, { + plugins: [ Table, TableCellPropertiesUI, Paragraph ] + } ) + .then( newEditor => { + editor = newEditor; + + tableCellPropertiesUI = editor.plugins.get( TableCellPropertiesUI ); + tableCellPropertiesButton = editor.ui.componentFactory.create( 'tableCellProperties' ); + contextualBalloon = editor.plugins.get( ContextualBalloon ); + // tableCellPropertiesView = tableCellPropertiesUI.view; + + // There is no point to execute BalloonPanelView attachTo and pin methods so lets override it. + testUtils.sinon.stub( contextualBalloon.view, 'attachTo' ).returns( {} ); + testUtils.sinon.stub( contextualBalloon.view, 'pin' ).returns( {} ); + } ); + } ); + + afterEach( () => { + editorElement.remove(); + + return editor.destroy(); + } ); + + it( 'should be named', () => { + expect( TableCellPropertiesUI.pluginName ).to.equal( 'TableCellPropertiesUI' ); + } ); + + it( 'should load ContextualBalloon', () => { + expect( editor.plugins.get( ContextualBalloon ) ).to.be.instanceOf( ContextualBalloon ); + } ); + + describe( 'init()', () => { + it( 'should set a batch', () => { + expect( tableCellPropertiesUI._batch ).to.be.null; + } ); + + describe( '#view', () => { + it( 'should be created', () => { + expect( tableCellPropertiesUI.view ).to.be.instanceOf( TableCellPropertiesUIView ); + } ); + + it( 'should be rendered', () => { + expect( tableCellPropertiesUI.view.isRendered ).to.be.true; + } ); + } ); + + describe( 'toolbar button', () => { + it( 'should be registered', () => { + expect( tableCellPropertiesButton ).to.be.instanceOf( ButtonView ); + } ); + + it( 'should have a label', () => { + expect( tableCellPropertiesButton.label ).to.equal( 'Cell properties' ); + } ); + + it( 'should have a tooltip', () => { + expect( tableCellPropertiesButton.tooltip ).to.be.true; + } ); + + it( 'should call #_showView upon #execute', () => { + const spy = testUtils.sinon.stub( tableCellPropertiesUI, '_showView' ).returns( {} ); + + tableCellPropertiesButton.fire( 'execute' ); + sinon.assert.calledOnce( spy ); + } ); + } ); + } ); +} ); From f8689d9c507a032a4423dada2c90012e83bb64f0 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 21 Jan 2020 11:33:55 +0100 Subject: [PATCH 18/33] Fixed wrong module paths. --- src/tablecellproperties.js | 2 +- src/{ => tablecellproperties}/tablecellpropertiesui.js | 8 ++++---- .../ui/tablecellpropertiesview.js | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) rename src/{ => tablecellproperties}/tablecellpropertiesui.js (97%) rename src/{ => tablecellproperties}/ui/tablecellpropertiesview.js (99%) diff --git a/src/tablecellproperties.js b/src/tablecellproperties.js index 27c5379c..55ea511d 100644 --- a/src/tablecellproperties.js +++ b/src/tablecellproperties.js @@ -8,7 +8,7 @@ */ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; -import TableCellPropertiesUI from './tablecellpropertiesui'; +import TableCellPropertiesUI from './tablecellproperties/tablecellpropertiesui'; import TableCellPropertiesEditing from './tablecellproperties/tablecellpropertiesediting'; /** diff --git a/src/tablecellpropertiesui.js b/src/tablecellproperties/tablecellpropertiesui.js similarity index 97% rename from src/tablecellpropertiesui.js rename to src/tablecellproperties/tablecellpropertiesui.js index 91b02be2..f2c6e6ad 100644 --- a/src/tablecellpropertiesui.js +++ b/src/tablecellproperties/tablecellpropertiesui.js @@ -9,13 +9,13 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; -import { getTableWidgetAncestor } from './utils'; +import { getTableWidgetAncestor } from '../utils'; import clickOutsideHandler from '@ckeditor/ckeditor5-ui/src/bindings/clickoutsidehandler'; 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 { findAncestor } from './commands/utils'; +import tableCellProperties from './../../theme/icons/table-cell-properties.svg'; +import { repositionContextualBalloon, getBalloonCellPositionData } from '../ui/utils'; +import { findAncestor } from '../commands/utils'; const DEFAULT_BORDER_STYLE = 'none'; const DEFAULT_HORIZONTAL_ALIGNMENT = 'left'; diff --git a/src/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js similarity index 99% rename from src/ui/tablecellpropertiesview.js rename to src/tablecellproperties/ui/tablecellpropertiesview.js index 86667044..c9b0826d 100644 --- a/src/ui/tablecellpropertiesview.js +++ b/src/tablecellproperties/ui/tablecellpropertiesview.js @@ -34,9 +34,9 @@ import alignTopIcon from '@ckeditor/ckeditor5-core/theme/icons/align-top.svg'; import alignMiddleIcon from '@ckeditor/ckeditor5-core/theme/icons/align-middle.svg'; import alignBottomIcon from '@ckeditor/ckeditor5-core/theme/icons/align-bottom.svg'; -import '../../theme/form.css'; -import '../../theme/tablecellproperties.css'; -import FormRowView from './formrowview'; +import '../../../theme/form.css'; +import '../../../theme/tablecellproperties.css'; +import FormRowView from '../../ui/formrowview'; const ALIGNMENT_ICONS = { left: alignLeftIcon, From 7be03482bd7dcff1e3caa3d518ab2bda1e89d4c4 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 21 Jan 2020 16:11:35 +0100 Subject: [PATCH 19/33] Code refactoring. --- .../tablecellpropertiesui.js | 120 ++++++++---------- .../ui/tablecellpropertiesview.js | 16 +-- 2 files changed, 61 insertions(+), 75 deletions(-) diff --git a/src/tablecellproperties/tablecellpropertiesui.js b/src/tablecellproperties/tablecellpropertiesui.js index f2c6e6ad..1d8984e6 100644 --- a/src/tablecellproperties/tablecellpropertiesui.js +++ b/src/tablecellproperties/tablecellpropertiesui.js @@ -15,14 +15,15 @@ import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextu import TableCellPropertiesView from './ui/tablecellpropertiesview'; import tableCellProperties from './../../theme/icons/table-cell-properties.svg'; import { repositionContextualBalloon, getBalloonCellPositionData } from '../ui/utils'; -import { findAncestor } from '../commands/utils'; const DEFAULT_BORDER_STYLE = 'none'; const DEFAULT_HORIZONTAL_ALIGNMENT = 'left'; const DEFAULT_VERTICAL_ALIGNMENT = 'middle'; - -// Attributes that set the same value for "top", "right", "bottom", and "left". -const QUAD_DIRECTION_ATTRIBUTES = [ 'borderStyle', 'borderWidth', 'borderColor', 'padding' ]; +const CELL_PROPERTIES = [ + 'borderStyle', 'borderColor', 'borderWidth', + 'padding', 'backgroundColor', + 'horizontalAlignment', 'verticalAlignment', +]; /** * The table cell properties UI plugin. It introduces the `'tableCellProperties'` button @@ -64,7 +65,7 @@ export default class TableCellPropertiesUI extends Plugin { this._balloon = editor.plugins.get( ContextualBalloon ); /** - * The properties form view displayed inside the balloon. + * The cell properties form view displayed inside the balloon. * * @member {module:table/ui/tablecellpropertiesview~TableCellPropertiesView} */ @@ -161,7 +162,7 @@ export default class TableCellPropertiesUI extends Plugin { /** * In this method the UI -> editor data binding is registered. * - * Registers a listener that updates the editor model when any observable property of + * Registers a listener that updates the editor data when any observable property of * the {@link #view} has changed. This makes the view live, which means the changes are * visible in the editing as soon as the user types or changes fields' values. * @@ -169,65 +170,54 @@ export default class TableCellPropertiesUI extends Plugin { */ _startRespondingToChangesInView() { const editor = this.editor; - const model = editor.model; - const document = model.document; - const selection = document.selection; - - this.view.on( 'change', ( evt, property, value ) => { - const firstPosition = selection.getFirstPosition(); - const tableCell = findAncestor( 'tableCell', firstPosition ); - - // Enqueue all changes into a single batch so clicking "Cancel" can undo them - // as a single undo steps. It's a better UX than dozens of undo steps, e.g. each - // for a single value change. - editor.model.enqueueChange( this._batch, writer => { - if ( QUAD_DIRECTION_ATTRIBUTES.includes( property ) ) { - writer.setAttribute( property, { - top: value, - right: value, - bottom: value, - left: value - }, tableCell ); - } else { - writer.setAttribute( property, value, tableCell ); - } + + this.view.on( 'change', ( evt, propertyName, newValue ) => { + // Not all observable properties of the #view must be related to the cell editing. + // For instance, they can belong to some internal logic. + if ( !CELL_PROPERTIES.includes( propertyName ) ) { + return; + } + + editor.execute( propertyNameToCommandName( propertyName ), { + value: newValue, + batch: this._batch } ); } ); } /** - * In this method the editor data -> UI binding is created. + * In this method the editor data -> UI binding is happening. + * + * When executed, this method obtains selected cell property values from various table commands + * and passes them to the {@link #view}. * - * When executed, this method obtains the value attribute values of the cell the selection is anchored - * to and passed them to the {@link #view}. This way, the UI stays up–to–date with the editor data. + * This way, the UI stays up–to–date with the editor data. * * @private */ - _fillViewFormFromSelectedCell() { + _fillViewFormFromCommandValues() { const editor = this.editor; - const model = editor.model; - const document = model.document; - const selection = document.selection; - const firstPosition = selection.getFirstPosition(); - const tableCell = findAncestor( 'tableCell', firstPosition ); - - const borderWidth = unifyQuadDirectionPropertyValue( tableCell.getAttribute( 'borderWidth' ) ) || ''; - const borderColor = unifyQuadDirectionPropertyValue( tableCell.getAttribute( 'borderColor' ) ) || ''; - const borderStyle = unifyQuadDirectionPropertyValue( tableCell.getAttribute( 'borderStyle' ) ) || DEFAULT_BORDER_STYLE; - const padding = unifyQuadDirectionPropertyValue( tableCell.getAttribute( 'padding' ) ) || ''; - const backgroundColor = tableCell.getAttribute( 'backgroundColor' ) || ''; - const horizontalAlignment = tableCell.getAttribute( 'horizontalAlignment' ) || DEFAULT_HORIZONTAL_ALIGNMENT; - const verticalAlignment = tableCell.getAttribute( 'verticalAlignment' ) || DEFAULT_VERTICAL_ALIGNMENT; - - this.view.set( { - borderWidth, - borderColor, - borderStyle, - padding, - backgroundColor, - horizontalAlignment, - verticalAlignment - } ); + const data = {}; + + for ( const propertyName of CELL_PROPERTIES ) { + let value = editor.commands.get( propertyNameToCommandName( propertyName ) ).value; + + if ( !value ) { + if ( propertyName === 'borderStyle' ) { + value = DEFAULT_BORDER_STYLE; + } else if ( propertyName === 'horizontalAlignment' ) { + value = DEFAULT_HORIZONTAL_ALIGNMENT; + } else if ( propertyName === 'verticalAlignment' ) { + value = DEFAULT_VERTICAL_ALIGNMENT; + } else { + value = ''; + } + } + + data[ propertyName ] = value; + } + + this.view.set( data ); } /** @@ -257,7 +247,7 @@ export default class TableCellPropertiesUI extends Plugin { this._batch = editor.model.createBatch(); // Update the view with the model values. - this._fillViewFormFromSelectedCell(); + this._fillViewFormFromCommandValues(); // Basic a11y. this.view.focus(); @@ -316,15 +306,11 @@ export default class TableCellPropertiesUI extends Plugin { } } -function unifyQuadDirectionPropertyValue( value ) { - if ( !value ) { - return; - } - - // Unify to one value. If different values are set default to top (or right, etc). - for ( const prop in value ) { - if ( value[ prop ] && value[ prop ] !== 'none' ) { - return value[ prop ]; - } - } +// Translates view's properties into command names. +// +// 'borderWidth' -> 'tableCellBorderWidth' +// +// @param {String} propertyName +function propertyNameToCommandName( propertyName ) { + return `tableCell${ propertyName[ 0 ].toUpperCase() }${ propertyName.slice( 1 ) }`; } diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js index c9b0826d..14f6a781 100644 --- a/src/tablecellproperties/ui/tablecellpropertiesview.js +++ b/src/tablecellproperties/ui/tablecellpropertiesview.js @@ -99,37 +99,37 @@ export default class TableCellPropertiesView extends View { * The value of the cell border width style. * * @observable - * @default null + * @default '' * @member #borderWidth */ - borderWidth: null, + borderWidth: '', /** * The value of the cell border color style. * * @observable - * @default null + * @default '' * @member #borderColor */ - borderColor: null, + borderColor: '', /** * The value of the cell padding style. * * @observable - * @default null + * @default '' * @member #padding */ - padding: null, + padding: '', /** * The value of the cell background color style. * * @observable - * @default null + * @default '' * @member #backgroundColor */ - backgroundColor: null, + backgroundColor: '', /** * The value of the horizontal text alignment style. From f165c9590b40193cf349958ebbb775b643666b7d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 21 Jan 2020 16:18:19 +0100 Subject: [PATCH 20/33] Tests: Code refactoring. --- tests/{ => tablecellproperties}/tablecellpropertiesui.js | 6 +++--- .../{ => tablecellproperties}/ui/tablecellpropertiesview.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) rename tests/{ => tablecellproperties}/tablecellpropertiesui.js (93%) rename tests/{ => tablecellproperties}/ui/tablecellpropertiesview.js (99%) diff --git a/tests/tablecellpropertiesui.js b/tests/tablecellproperties/tablecellpropertiesui.js similarity index 93% rename from tests/tablecellpropertiesui.js rename to tests/tablecellproperties/tablecellpropertiesui.js index 54215f90..c60646f8 100644 --- a/tests/tablecellpropertiesui.js +++ b/tests/tablecellproperties/tablecellpropertiesui.js @@ -14,9 +14,9 @@ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon'; -import Table from '@ckeditor/ckeditor5-table/src/table'; -import TableCellPropertiesUI from '@ckeditor/ckeditor5-table/src/tablecellpropertiesui'; -import TableCellPropertiesUIView from '@ckeditor/ckeditor5-table/src/ui/tablecellpropertiesview'; +import Table from '../../src/table'; +import TableCellPropertiesUI from '../../src/tablecellproperties/tablecellpropertiesui'; +import TableCellPropertiesUIView from '../../src/tablecellproperties/ui/tablecellpropertiesview'; import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; // import View from '@ckeditor/ckeditor5-ui/src/view'; diff --git a/tests/ui/tablecellpropertiesview.js b/tests/tablecellproperties/ui/tablecellpropertiesview.js similarity index 99% rename from tests/ui/tablecellpropertiesview.js rename to tests/tablecellproperties/ui/tablecellpropertiesview.js index d90cfd0a..0b4c873f 100644 --- a/tests/ui/tablecellpropertiesview.js +++ b/tests/tablecellproperties/ui/tablecellpropertiesview.js @@ -5,7 +5,7 @@ /* globals Event */ -import TableCellPropertiesView from '../../src/ui/tablecellpropertiesview'; +import TableCellPropertiesView from '../../../src/tablecellproperties/ui/tablecellpropertiesview'; import LabeledView from '@ckeditor/ckeditor5-ui/src/labeledview/labeledview'; import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler'; From e4dfae1328e02713c7e77dcb2d4995ab82ca9940 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 21 Jan 2020 17:30:24 +0100 Subject: [PATCH 21/33] Tests: Finished the TableCellPropertiesUI tests. --- .../tablecellpropertiesui.js | 83 ++---- .../tablecellpropertiesui.js | 265 +++++++++++++++++- .../ui/tablecellpropertiesview.js | 8 +- 3 files changed, 287 insertions(+), 69 deletions(-) diff --git a/src/tablecellproperties/tablecellpropertiesui.js b/src/tablecellproperties/tablecellpropertiesui.js index 1d8984e6..4d197889 100644 --- a/src/tablecellproperties/tablecellpropertiesui.js +++ b/src/tablecellproperties/tablecellpropertiesui.js @@ -75,13 +75,10 @@ export default class TableCellPropertiesUI extends Plugin { * The batch used to undo all changes made by the form (which are live, as the user types) * when "Cancel" was pressed. Each time the view is shown, a new batch is created. * - * @private + * @protected * @member {module:engine/model/batch~Batch} */ - this._batch = null; - - // Make the form dynamic, i.e. create bindings between view fields and the model. - this._startRespondingToChangesInView(); + this._undoStepBatch = null; editor.ui.componentFactory.add( 'tableCellProperties', locale => { const view = new ButtonView( locale ); @@ -129,17 +126,17 @@ export default class TableCellPropertiesUI extends Plugin { } ); this.listenTo( view, 'cancel', () => { - editor.execute( 'undo', this._batch ); + editor.execute( 'undo', this._undoStepBatch ); this._hideView(); } ); - // Close the balloon on Esc key press when the **form has focus**. + // Close the balloon on Esc key press. view.keystrokes.set( 'Esc', ( data, cancel ) => { this._hideView(); cancel(); } ); - // Reposition the balloon or hide the form if an image widget is no longer selected. + // Reposition the balloon or hide the form if a table cell is no longer selected. this.listenTo( editor.ui, 'update', () => { if ( !getTableWidgetAncestor( viewDocument.selection ) ) { this._hideView(); @@ -156,22 +153,11 @@ export default class TableCellPropertiesUI extends Plugin { callback: () => this._hideView() } ); - return view; - } - - /** - * In this method the UI -> editor data binding is registered. - * - * Registers a listener that updates the editor data when any observable property of - * the {@link #view} has changed. This makes the view live, which means the changes are - * visible in the editing as soon as the user types or changes fields' values. - * - * @private - */ - _startRespondingToChangesInView() { - const editor = this.editor; - - this.view.on( 'change', ( evt, propertyName, newValue ) => { + // Create the "UI -> editor data" binding. + // This listener updates 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 + // visible in the editing as soon as the user types or changes fields' values. + view.on( 'change', ( evt, propertyName, newValue ) => { // Not all observable properties of the #view must be related to the cell editing. // For instance, they can belong to some internal logic. if ( !CELL_PROPERTIES.includes( propertyName ) ) { @@ -180,13 +166,15 @@ export default class TableCellPropertiesUI extends Plugin { editor.execute( propertyNameToCommandName( propertyName ), { value: newValue, - batch: this._batch + batch: this._undoStepBatch } ); } ); + + return view; } /** - * In this method the editor data -> UI binding is happening. + * In this method the "editor data -> UI" binding is happening. * * When executed, this method obtains selected cell property values from various table commands * and passes them to the {@link #view}. @@ -223,28 +211,22 @@ export default class TableCellPropertiesUI extends Plugin { /** * Shows the {@link #view} in the {@link #_balloon}. * - * **Note**: Each time a view is shown, the new {@link #_batch} is created that contains + * **Note**: Each time a view is shown, the new {@link #_undoStepBatch} is created that contains * all changes made to the document when the view is visible, allowing a single undo step * for all of them. * - * @private + * @protected */ _showView() { - if ( this._isViewVisible ) { - return; - } - const editor = this.editor; - if ( !this._isViewInBalloon ) { - this._balloon.add( { - view: this.view, - position: getBalloonCellPositionData( editor ) - } ); - } + this._balloon.add( { + view: this.view, + position: getBalloonCellPositionData( editor ) + } ); // Create a new batch. Clicking "Cancel" will undo this batch. - this._batch = editor.model.createBatch(); + this._undoStepBatch = editor.model.createBatch(); // Update the view with the model values. this._fillViewFormFromCommandValues(); @@ -256,7 +238,7 @@ export default class TableCellPropertiesUI extends Plugin { /** * Removes the {@link #view} from the {@link #_balloon}. * - * @private + * @protected */ _hideView() { if ( !this._isViewInBalloon ) { @@ -266,23 +248,16 @@ export default class TableCellPropertiesUI extends Plugin { const editor = this.editor; this.stopListening( editor.ui, 'update' ); - this.stopListening( this._balloon, 'change:visibleView' ); - - // Make sure the focus always gets back to the editable _before_ removing the focused properties view. - // Doing otherwise causes issues in some browsers. See https://github.com/ckeditor/ckeditor5-link/issues/193. - editor.editing.view.focus(); - if ( this._isViewInBalloon ) { - // Blur any input element before removing it from DOM to prevent issues in some browsers. - // See https://github.com/ckeditor/ckeditor5/issues/1501. - this.view.saveButtonView.focus(); + // Blur any input element before removing it from DOM to prevent issues in some browsers. + // See https://github.com/ckeditor/ckeditor5/issues/1501. + this.view.saveButtonView.focus(); - this._balloon.remove( this.view ); + this._balloon.remove( this.view ); - // Because the form has an input which has focus, the focus must be brought back - // to the editor. Otherwise, it would be lost. - this.editor.editing.view.focus(); - } + // Make sure the focus is not lost in the process by putting it directly + // into the editing view. + this.editor.editing.view.focus(); } /** diff --git a/tests/tablecellproperties/tablecellpropertiesui.js b/tests/tablecellproperties/tablecellpropertiesui.js index c60646f8..843653d1 100644 --- a/tests/tablecellproperties/tablecellpropertiesui.js +++ b/tests/tablecellproperties/tablecellpropertiesui.js @@ -3,26 +3,27 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* globals document */ +/* globals document, Event */ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; -// import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; -// import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; -// import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; +import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; +import { getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import Undo from '@ckeditor/ckeditor5-undo/src/undo'; +import Batch from '@ckeditor/ckeditor5-engine/src/model/batch'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon'; import Table from '../../src/table'; +import TableCellPropertiesEditing from '../../src/tablecellproperties/tablecellpropertiesediting'; import TableCellPropertiesUI from '../../src/tablecellproperties/tablecellpropertiesui'; import TableCellPropertiesUIView from '../../src/tablecellproperties/ui/tablecellpropertiesview'; -import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; -// import View from '@ckeditor/ckeditor5-ui/src/view'; -describe.only( 'TableCellPropertiesUI', () => { +describe( 'TableCellPropertiesUI', () => { let editor, editorElement, contextualBalloon, - tableCellPropertiesUI, tableCellPropertiesButton; + tableCellPropertiesUI, tableCellPropertiesView, tableCellPropertiesButton; testUtils.createSinonSandbox(); @@ -32,7 +33,8 @@ describe.only( 'TableCellPropertiesUI', () => { return ClassicTestEditor .create( editorElement, { - plugins: [ Table, TableCellPropertiesUI, Paragraph ] + plugins: [ Table, TableCellPropertiesEditing, TableCellPropertiesUI, Paragraph, Undo ], + initialData: '
foo

bar

' } ) .then( newEditor => { editor = newEditor; @@ -40,7 +42,7 @@ describe.only( 'TableCellPropertiesUI', () => { tableCellPropertiesUI = editor.plugins.get( TableCellPropertiesUI ); tableCellPropertiesButton = editor.ui.componentFactory.create( 'tableCellProperties' ); contextualBalloon = editor.plugins.get( ContextualBalloon ); - // tableCellPropertiesView = tableCellPropertiesUI.view; + tableCellPropertiesView = tableCellPropertiesUI.view; // There is no point to execute BalloonPanelView attachTo and pin methods so lets override it. testUtils.sinon.stub( contextualBalloon.view, 'attachTo' ).returns( {} ); @@ -64,7 +66,7 @@ describe.only( 'TableCellPropertiesUI', () => { describe( 'init()', () => { it( 'should set a batch', () => { - expect( tableCellPropertiesUI._batch ).to.be.null; + expect( tableCellPropertiesUI._undoStepBatch ).to.be.null; } ); describe( '#view', () => { @@ -98,4 +100,245 @@ describe.only( 'TableCellPropertiesUI', () => { } ); } ); } ); + + describe( 'destroy()', () => { + + } ); + + describe( 'Properties #view', () => { + beforeEach( () => { + editor.model.change( writer => { + writer.setSelection( editor.model.document.getRoot().getChild( 0 ).getChild( 0 ).getChild( 0 ), 0 ); + } ); + } ); + + it( 'should hide on #submit', () => { + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + + tableCellPropertiesView.fire( 'submit' ); + expect( contextualBalloon.visibleView ).to.be.null; + } ); + + it( 'should undo the entire batch of changes on #cancel', () => { + // Show the view. New batch will be created. + tableCellPropertiesButton.fire( 'execute' ); + + // Do the changes like a user. + tableCellPropertiesView.borderStyle = 'dotted'; + tableCellPropertiesView.backgroundColor = 'red'; + + expect( getModelData( editor.model ) ).to.equal( + '' + + '' + + '' + + '[]foo' + + '' + + '' + + '
' + + 'bar' + ); + + tableCellPropertiesView.fire( 'cancel' ); + + expect( getModelData( editor.model ) ).to.equal( + '' + + '' + + '' + + '[]foo' + + '' + + '' + + '
' + + 'bar' + ); + } ); + + it( 'should hide on #cancel', () => { + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + + tableCellPropertiesView.fire( 'cancel' ); + expect( contextualBalloon.visibleView ).to.be.null; + } ); + + it( 'should hide on the Esc key press', () => { + const keyEvtData = { + keyCode: keyCodes.esc, + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; + + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + + tableCellPropertiesView.keystrokes.press( keyEvtData ); + expect( contextualBalloon.visibleView ).to.be.null; + } ); + + it( 'should hide if the table cell is no longer selected on EditorUI#update', () => { + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + + editor.model.change( writer => { + // Set selection in the paragraph. + writer.setSelection( editor.model.document.getRoot().getChild( 1 ), 0 ); + } ); + + expect( contextualBalloon.visibleView ).to.be.null; + } ); + + it( 'should reposition if table cell is still selected on on EditorUI#update', () => { + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + + editor.model.change( writer => { + writer.insertText( 'qux', editor.model.document.selection.getFirstPosition() ); + } ); + + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + } ); + + it( 'should hide if clicked outside the balloon', () => { + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + + document.body.dispatchEvent( new Event( 'mousedown', { bubbles: true } ) ); + + expect( contextualBalloon.visibleView ).to.be.null; + } ); + + describe( 'property changes', () => { + it( 'should affect the editor state', () => { + const spy = testUtils.sinon.stub( editor, 'execute' ); + + tableCellPropertiesUI._undoStepBatch = 'foo'; + tableCellPropertiesView.fire( 'change', 'borderStyle', 'dotted' ); + + sinon.assert.calledOnce( spy ); + sinon.assert.calledWithExactly( spy, 'tableCellBorderStyle', { value: 'dotted', batch: 'foo' } ); + } ); + + it( 'should not affect the editor state if internal property has changed', () => { + const spy = testUtils.sinon.stub( editor, 'execute' ); + + tableCellPropertiesUI._undoStepBatch = 'foo'; + tableCellPropertiesView.fire( 'change', 'internalProp', 'foo' ); + + sinon.assert.notCalled( spy ); + } ); + } ); + } ); + + describe( 'Showing the #view', () => { + beforeEach( () => { + editor.model.change( writer => { + writer.setSelection( editor.model.document.getRoot().getChild( 0 ).getChild( 0 ).getChild( 0 ), 0 ); + } ); + } ); + + it( 'should create a new undoable batch for further #view cancel', () => { + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + + const firstBatch = tableCellPropertiesUI._undoStepBatch; + expect( firstBatch ).to.be.instanceOf( Batch ); + + tableCellPropertiesView.fire( 'submit' ); + expect( contextualBalloon.visibleView ).to.be.null; + + tableCellPropertiesButton.fire( 'execute' ); + + const secondBatch = tableCellPropertiesUI._undoStepBatch; + expect( secondBatch ).to.be.instanceOf( Batch ); + expect( firstBatch ).to.not.equal( secondBatch ); + } ); + + describe( 'initial data', () => { + it( 'should be set from the command values', () => { + editor.commands.get( 'tableCellBorderStyle' ).value = 'a'; + editor.commands.get( 'tableCellBorderColor' ).value = 'b'; + editor.commands.get( 'tableCellBorderWidth' ).value = 'c'; + editor.commands.get( 'tableCellPadding' ).value = 'd'; + editor.commands.get( 'tableCellBackgroundColor' ).value = 'e'; + editor.commands.get( 'tableCellHorizontalAlignment' ).value = 'f'; + editor.commands.get( 'tableCellVerticalAlignment' ).value = 'g'; + + tableCellPropertiesButton.fire( 'execute' ); + + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + expect( tableCellPropertiesView ).to.include( { + borderStyle: 'a', + borderColor: 'b', + borderWidth: 'c', + padding: 'd', + backgroundColor: 'e', + horizontalAlignment: 'f', + verticalAlignment: 'g' + } ); + } ); + + it( 'should use default values when command has no value', () => { + editor.commands.get( 'tableCellBorderStyle' ).value = null; + editor.commands.get( 'tableCellBorderColor' ).value = null; + editor.commands.get( 'tableCellBorderWidth' ).value = null; + editor.commands.get( 'tableCellPadding' ).value = null; + editor.commands.get( 'tableCellBackgroundColor' ).value = null; + editor.commands.get( 'tableCellHorizontalAlignment' ).value = null; + editor.commands.get( 'tableCellVerticalAlignment' ).value = null; + + tableCellPropertiesButton.fire( 'execute' ); + + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + expect( tableCellPropertiesView ).to.include( { + borderStyle: 'none', + borderColor: '', + borderWidth: '', + padding: '', + backgroundColor: '', + horizontalAlignment: 'left', + verticalAlignment: 'middle' + } ); + } ); + } ); + + it( 'should focus the form view', () => { + const spy = testUtils.sinon.spy( tableCellPropertiesView, 'focus' ); + + tableCellPropertiesButton.fire( 'execute' ); + + sinon.assert.calledOnce( spy ); + } ); + } ); + + describe( 'Hiding the #view', () => { + beforeEach( () => { + editor.model.change( writer => { + writer.setSelection( editor.model.document.getRoot().getChild( 0 ).getChild( 0 ).getChild( 0 ), 0 ); + } ); + } ); + + it( 'should stop listening to EditorUI#update', () => { + const spy = testUtils.sinon.spy( tableCellPropertiesUI, 'stopListening' ); + + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + + tableCellPropertiesView.fire( 'submit' ); + expect( contextualBalloon.visibleView ).to.be.null; + + sinon.assert.calledOnce( spy ); + sinon.assert.calledWithExactly( spy, editor.ui, 'update' ); + } ); + + it( 'should focus the editing view so the focus is not lost', () => { + const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); + + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + + tableCellPropertiesView.fire( 'submit' ); + + sinon.assert.calledOnce( spy ); + } ); + } ); } ); diff --git a/tests/tablecellproperties/ui/tablecellpropertiesview.js b/tests/tablecellproperties/ui/tablecellpropertiesview.js index 0b4c873f..1d6ff663 100644 --- a/tests/tablecellproperties/ui/tablecellpropertiesview.js +++ b/tests/tablecellproperties/ui/tablecellpropertiesview.js @@ -44,10 +44,10 @@ describe( 'TableCellPropertiesView', () => { it( 'should define the public data interface (observable properties)', () => { expect( view ).to.include( { borderStyle: 'none', - borderWidth: null, - borderColor: null, - padding: null, - backgroundColor: null, + borderWidth: '', + borderColor: '', + padding: '', + backgroundColor: '', horizontalAlignment: 'left', verticalAlignment: 'middle' } ); From 3b139f01785aaae334ebda7d69fc7dafcfc5c803 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 21 Jan 2020 18:15:59 +0100 Subject: [PATCH 22/33] Docs: Fixed broken module paths. --- src/tablecellproperties/tablecellpropertiesui.js | 2 +- src/tablecellproperties/ui/tablecellpropertiesview.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tablecellproperties/tablecellpropertiesui.js b/src/tablecellproperties/tablecellpropertiesui.js index 4d197889..cfedc96b 100644 --- a/src/tablecellproperties/tablecellpropertiesui.js +++ b/src/tablecellproperties/tablecellpropertiesui.js @@ -4,7 +4,7 @@ */ /** - * @module table/tablecellpropertiesui + * @module table/tablecellproperties/tablecellpropertiesui */ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js index 14f6a781..611185e8 100644 --- a/src/tablecellproperties/ui/tablecellpropertiesview.js +++ b/src/tablecellproperties/ui/tablecellpropertiesview.js @@ -4,7 +4,7 @@ */ /** - * @module table/ui/tablecellpropertiesview + * @module table/tablecellproperties/ui/tablecellpropertiesview */ import View from '@ckeditor/ckeditor5-ui/src/view'; From 6580667ae3a00e68bd7c5a24d5590cbe02e71578 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 21 Jan 2020 18:18:21 +0100 Subject: [PATCH 23/33] Docs: Fixed broken links. --- src/tablecellproperties/tablecellpropertiesui.js | 8 ++++---- src/ui/formrowview.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tablecellproperties/tablecellpropertiesui.js b/src/tablecellproperties/tablecellpropertiesui.js index cfedc96b..d14bef28 100644 --- a/src/tablecellproperties/tablecellpropertiesui.js +++ b/src/tablecellproperties/tablecellpropertiesui.js @@ -67,7 +67,7 @@ export default class TableCellPropertiesUI extends Plugin { /** * The cell properties form view displayed inside the balloon. * - * @member {module:table/ui/tablecellpropertiesview~TableCellPropertiesView} + * @member {module:table/tablecellproperties/ui/tablecellpropertiesview~TableCellPropertiesView} */ this.view = this._createPropertiesView(); @@ -107,11 +107,11 @@ export default class TableCellPropertiesUI extends Plugin { } /** - * Creates the {@link module:table/ui/tablecellpropertiesview~TableCellPropertiesView} instance. + * Creates the {@link module:table/tablecellproperties/ui/tablecellpropertiesview~TableCellPropertiesView} instance. * * @private - * @returns {module:table/ui/tablecellpropertiesview~TableCellPropertiesView} The cell properties form - * view instance. + * @returns {module:table/tablecellproperties/ui/tablecellpropertiesview~TableCellPropertiesView} The cell + * properties form view instance. */ _createPropertiesView() { const editor = this.editor; diff --git a/src/ui/formrowview.js b/src/ui/formrowview.js index 10b79dce..0d6e9dc9 100644 --- a/src/ui/formrowview.js +++ b/src/ui/formrowview.js @@ -11,7 +11,7 @@ import View from '@ckeditor/ckeditor5-ui/src/view'; /** * The class representing a single row in the complex form, - * used by {@link module:table/ui/tablecellpropertiesview~TableCellPropertiesView}. + * used by {@link module:table/tablecellproperties/ui/tablecellpropertiesview~TableCellPropertiesView}. * * **Note**: For now this class is private. When more use cases arrive (beyond ckeditor5-table), * it will become a component in ckeditor5-ui. From 85d0ce661f2374473f51df4473b58daa5d6a9e6d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 21 Jan 2020 19:03:50 +0100 Subject: [PATCH 24/33] Docs: Improved TableCellPropertiesView docs. --- .../ui/tablecellpropertiesview.js | 180 +++++++++++++----- 1 file changed, 134 insertions(+), 46 deletions(-) diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js index 611185e8..ce96814e 100644 --- a/src/tablecellproperties/ui/tablecellpropertiesview.js +++ b/src/tablecellproperties/ui/tablecellpropertiesview.js @@ -61,6 +61,10 @@ export default class TableCellPropertiesView extends View { constructor( locale ) { super( locale ); + const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields(); + const { horizontalAlignmentToolbar, verticalAlignmentToolbar, alignmentLabel } = this._createAlignmentFields(); + const { saveButtonView, cancelButtonView } = this._createActionButtons(); + /** * Tracks information about the DOM focus in the form. * @@ -150,6 +154,76 @@ export default class TableCellPropertiesView extends View { verticalAlignment: 'middle' } ); + /** + * A dropdown that allows selecting the style of the table cell border. + * + * @readonly + * @member {module:ui/dropdown/dropdownview~DropdownView} + */ + this.borderStyleDropdown = borderStyleDropdown; + + /** + * An input that allows specifying the width of the table cell border. + * + * @readonly + * @member {module:ui/inputtext/inputtextview~InputTextView} + */ + this.borderWidthInput = borderWidthInput; + + /** + * An input that allows specifying the color of the table cell border. + * + * @readonly + * @member {module:ui/inputtext/inputtextview~InputTextView} + */ + this.borderColorInput = borderColorInput; + + /** + * An input that allows specifying the table cell background color. + * + * @readonly + * @member {module:ui/inputtext/inputtextview~InputTextView} + */ + this.backgroundInput = this._createBackgroundField(); + + /** + * An input that allows specifying the table cell padding. + * + * @readonly + * @member {module:ui/inputtext/inputtextview~InputTextView} + */ + this.paddingInput = this._createPaddingField(); + + /** + * A toolbar with buttons that allow changing the horizontal text alignment in a table cell. + * + * @readonly + * @member {module:ui/toolbar/toolbar~ToolbarView} + */ + this.horizontalAlignmentToolbar = horizontalAlignmentToolbar; + + /** + * A toolbar with buttons that allow changing the vertical text alignment in a table cell. + * + * @readonly + * @member {module:ui/toolbar/toolbar~ToolbarView} + */ + this.verticalAlignmentToolbar = verticalAlignmentToolbar; + + /** + * The "Save" button view. + * + * @member {module:ui/button/buttonview~ButtonView} + */ + this.saveButtonView = saveButtonView; + + /** + * The "Cancel" button view. + * + * @member {module:ui/button/buttonview~ButtonView} + */ + this.cancelButtonView = cancelButtonView; + /** * A collection of views that can be focused in the form. * @@ -179,24 +253,17 @@ export default class TableCellPropertiesView extends View { } } ); - this._createHeaderView(); - this._createBorderFields(); - this._createBackgroundField(); - this._createPaddingField(); - this._createAlignmentFields(); - this._createActionButtons(); - // Form header. - this.children.add( this._headerView ); + this.children.add( this._createHeaderView() ); // Border row. this.children.add( new FormRowView( locale, { - labelView: this._borderRowLabel, + labelView: borderRowLabel, children: [ - this._borderRowLabel, - this.borderStyleDropdown, - this.borderColorInput, - this.borderWidthInput + borderRowLabel, + borderStyleDropdown, + borderColorInput, + borderWidthInput ], class: 'ck-table-cell-properties-form__border-row' } ) ); @@ -211,11 +278,11 @@ export default class TableCellPropertiesView extends View { // Text alignment row. this.children.add( new FormRowView( locale, { - labelView: this._alignmentLabel, + labelView: alignmentLabel, children: [ - this._alignmentLabel, - this.horizontalAlignmentToolbar, - this.verticalAlignmentToolbar, + alignmentLabel, + horizontalAlignmentToolbar, + verticalAlignmentToolbar, ], class: 'ck-table-cell-properties-form__alignment-row' } ) ); @@ -289,14 +356,15 @@ export default class TableCellPropertiesView extends View { * Creates the header of the form with a localized label. * * @private + * @returns {module:ui/view~View} */ _createHeaderView() { const locale = this.locale; const t = this.t; - this._headerView = new View( locale ); + const headerView = new View( locale ); - this._headerView.setTemplate( { + headerView.setTemplate( { tag: 'div', attributes: { class: [ @@ -308,6 +376,8 @@ export default class TableCellPropertiesView extends View { t( 'Cell properties' ) ] } ); + + return headerView; } /** @@ -318,6 +388,7 @@ export default class TableCellPropertiesView extends View { * * {@link #borderColorInput}. * * @private + * @returns {Object.} */ _createBorderFields() { const locale = this.locale; @@ -325,12 +396,12 @@ export default class TableCellPropertiesView extends View { // -- Group label --------------------------------------------- - const borderRowLabel = this._borderRowLabel = new LabelView( locale ); + const borderRowLabel = new LabelView( locale ); borderRowLabel.text = t( 'Border' ); // -- Style --------------------------------------------------- - const borderStyleDropdown = this.borderStyleDropdown = new LabeledView( locale, createLabeledDropdown ); + const borderStyleDropdown = new LabeledView( locale, createLabeledDropdown ); borderStyleDropdown.set( { label: t( 'Style' ), class: 'ck-table-cell-properties-form__border-style' @@ -354,7 +425,7 @@ export default class TableCellPropertiesView extends View { // -- Width --------------------------------------------------- - const borderWidthInput = this.borderWidthInput = new LabeledView( locale, createLabeledInputText ); + const borderWidthInput = new LabeledView( locale, createLabeledInputText ); borderWidthInput.set( { label: t( 'Width' ), @@ -371,7 +442,7 @@ export default class TableCellPropertiesView extends View { // -- Color --------------------------------------------------- - const borderColorInput = this.borderColorInput = new LabeledView( locale, createLabeledInputText ); + const borderColorInput = new LabeledView( locale, createLabeledInputText ); borderColorInput.label = t( 'Color' ); borderColorInput.view.bind( 'value' ).to( this, 'borderColor' ); borderColorInput.bind( 'isEnabled' ).to( this, 'borderStyle', value => { @@ -381,6 +452,13 @@ export default class TableCellPropertiesView extends View { borderColorInput.view.on( 'input', () => { this.borderColor = borderColorInput.view.element.value; } ); + + return { + borderRowLabel, + borderStyleDropdown, + borderColorInput, + borderWidthInput, + }; } /** @@ -389,11 +467,13 @@ export default class TableCellPropertiesView extends View { * * {@link #backgroundInput}. * * @private + * @returns {module:ui/labeledview/labeledview~LabeledView} */ _createBackgroundField() { const locale = this.locale; const t = this.t; - const backgroundInput = this.backgroundInput = new LabeledView( locale, createLabeledInputText ); + + const backgroundInput = new LabeledView( locale, createLabeledInputText ); backgroundInput.set( { label: t( 'Background' ), @@ -404,6 +484,8 @@ export default class TableCellPropertiesView extends View { backgroundInput.view.on( 'input', () => { this.backgroundColor = backgroundInput.view.element.value; } ); + + return backgroundInput; } /** @@ -412,11 +494,13 @@ export default class TableCellPropertiesView extends View { * * {@link #paddingInput}. * * @private + * @returns {module:ui/labeledview/labeledview~LabeledView} */ _createPaddingField() { const locale = this.locale; const t = this.t; - const paddingInput = this.paddingInput = new LabeledView( locale, createLabeledInputText ); + + const paddingInput = new LabeledView( locale, createLabeledInputText ); paddingInput.set( { label: t( 'Padding' ), @@ -427,6 +511,8 @@ export default class TableCellPropertiesView extends View { paddingInput.view.on( 'input', () => { this.padding = paddingInput.view.element.value; } ); + + return paddingInput; } /** @@ -436,25 +522,33 @@ export default class TableCellPropertiesView extends View { * * {@link #verticalAlignmentToolbar}. * * @private + * @returns {Object.} */ _createAlignmentFields() { const locale = this.locale; const t = this.t; - this._alignmentLabel = new LabelView( locale ); - this._alignmentLabel.text = t( 'Text alignment' ); + const alignmentLabel = new LabelView( locale ); + + alignmentLabel.text = t( 'Text alignment' ); // -- Horizontal --------------------------------------------------- - this.horizontalAlignmentToolbar = new ToolbarView( locale ); - this.horizontalAlignmentToolbar.ariaLabel = t( 'Horizontal text alignment toolbar' ); - this._fillAlignmentToolbar( this.horizontalAlignmentToolbar, this._horizontalAlignmentLabels, 'horizontalAlignment' ); + const horizontalAlignmentToolbar = new ToolbarView( locale ); + horizontalAlignmentToolbar.ariaLabel = t( 'Horizontal text alignment toolbar' ); + this._fillAlignmentToolbar( horizontalAlignmentToolbar, this._horizontalAlignmentLabels, 'horizontalAlignment' ); // -- Vertical ----------------------------------------------------- - this.verticalAlignmentToolbar = new ToolbarView( locale ); - this.verticalAlignmentToolbar.ariaLabel = t( 'Vertical text alignment toolbar' ); - this._fillAlignmentToolbar( this.verticalAlignmentToolbar, this._verticalAlignmentLabels, 'verticalAlignment' ); + const verticalAlignmentToolbar = new ToolbarView( locale ); + verticalAlignmentToolbar.ariaLabel = t( 'Vertical text alignment toolbar' ); + this._fillAlignmentToolbar( verticalAlignmentToolbar, this._verticalAlignmentLabels, 'verticalAlignment' ); + + return { + horizontalAlignmentToolbar, + verticalAlignmentToolbar, + alignmentLabel + }; } /** @@ -464,17 +558,14 @@ export default class TableCellPropertiesView extends View { * * {@link #cancelButtonView}. * * @private + * @returns {Object.} */ _createActionButtons() { const locale = this.locale; const t = this.t; - /** - * The Save button view. - * - * @member {module:ui/button/buttonview~ButtonView} - */ - const saveButtonView = this.saveButtonView = new ButtonView( locale ); + const saveButtonView = new ButtonView( locale ); + const cancelButtonView = new ButtonView( locale ); saveButtonView.set( { label: t( 'Save' ), @@ -484,13 +575,6 @@ export default class TableCellPropertiesView extends View { withText: true, } ); - /** - * The Cancel button view. - * - * @member {module:ui/button/buttonview~ButtonView} - */ - const cancelButtonView = this.cancelButtonView = new ButtonView( locale ); - cancelButtonView.set( { label: t( 'Cancel' ), icon: cancelIcon, @@ -500,6 +584,10 @@ export default class TableCellPropertiesView extends View { } ); cancelButtonView.delegate( 'execute' ).to( this, 'cancel' ); + + return { + saveButtonView, cancelButtonView + }; } /** From 82dae2dbec4e3cd95eade7f42985eb222bb54a99 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 22 Jan 2020 10:53:59 +0100 Subject: [PATCH 25/33] Tests: Aligned tests to the latest TableCellPropertiesView API. --- tests/tablecellproperties/ui/tablecellpropertiesview.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tablecellproperties/ui/tablecellpropertiesview.js b/tests/tablecellproperties/ui/tablecellpropertiesview.js index 1d6ff663..e4255ca9 100644 --- a/tests/tablecellproperties/ui/tablecellpropertiesview.js +++ b/tests/tablecellproperties/ui/tablecellpropertiesview.js @@ -88,7 +88,7 @@ describe( 'TableCellPropertiesView', () => { expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; expect( row.classList.contains( 'ck-table-cell-properties-form__border-row' ) ).to.be.true; - expect( row.childNodes[ 0 ] ).to.equal( view._borderRowLabel.element ); + expect( row.childNodes[ 0 ].textContent ).to.equal( 'Border' ); expect( row.childNodes[ 1 ] ).to.equal( view.borderStyleDropdown.element ); expect( row.childNodes[ 2 ] ).to.equal( view.borderColorInput.element ); expect( row.childNodes[ 3 ] ).to.equal( view.borderWidthInput.element ); @@ -295,7 +295,7 @@ describe( 'TableCellPropertiesView', () => { expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; expect( row.classList.contains( 'ck-table-cell-properties-form__alignment-row' ) ).to.be.true; - expect( row.childNodes[ 0 ] ).to.equal( view._alignmentLabel.element ); + expect( row.childNodes[ 0 ].textContent ).to.equal( 'Text alignment' ); expect( row.childNodes[ 1 ] ).to.equal( view.horizontalAlignmentToolbar.element ); expect( row.childNodes[ 2 ] ).to.equal( view.verticalAlignmentToolbar.element ); } ); From 2aee83a326296ee0ffd237f3cf0ca37cc360af6e Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 22 Jan 2020 11:46:17 +0100 Subject: [PATCH 26/33] Added contexts.json entries for the table cell properties UI. --- lang/contexts.json | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/lang/contexts.json b/lang/contexts.json index b14a6c17..db2fb732 100644 --- a/lang/contexts.json +++ b/lang/contexts.json @@ -17,5 +17,33 @@ "Split cell vertically": "Label for the split table cell vertically button.", "Split cell horizontally": "Label for the split table cell horizontally button.", "Merge cells": "Label for the merge table cells button.", - "Table toolbar": "The label used by assistive technologies describing a table toolbar attached to a table widget." + "Table toolbar": "The label used by assistive technologies describing a table toolbar attached to a table widget.", + "Cell properties": "The label describing the form allowing to specify the properties of a selected table cell.", + "Border": "The label describing a group of border–related form fields (border style, color, etc.).", + "Style": "The label for the dropdown that allows configuring the border style of a table or a table cell.", + "Width": "The label for the input that allows configuring the border width of a table or a table cell.", + "Color": "The label for the input that allows configuring the border color of a table or a table cell.", + "Background": "The label for the input that allows configuring the background color of a table or a table cell.", + "Padding": "The label for the input that allows configuring the padding of a table cell.", + "Text alignment": "The label for the group of toolbars that allows configuring the text alignment in a table cell.", + "Horizontal text alignment toolbar": "The label used by assistive technologies describing a toolbar that allows configuring the horizontal text alignment in a table cell.", + "Vertical text alignment toolbar": "The label used by assistive technologies describing a toolbar that allows configuring the vertical text alignment in a table cell.", + "Save": "The label for the button that saves the changes made to the table or table cell properties.", + "Cancel": "The label for the button that rejects the changes made to the table or table cell properties.", + "None": "The label for the border style dropdown when no style is applied to a table or a table cell.", + "Solid": "The label for the border style dropdown when the solid border is applied to a table or a table cell.", + "Dotted": "The label for the border style dropdown when the dotted border is applied to a table or a table cell.", + "Dashed": "The label for the border style dropdown when the dashed border is applied to a table or a table cell.", + "Double": "The label for the border style dropdown when the double border is applied to a table or a table cell.", + "Groove": "The label for the border style dropdown when the groove border is applied to a table or a table cell.", + "Ridge": "The label for the border style dropdown when the ridge border is applied to a table or a table cell.", + "Inset": "The label for the border style dropdown when the inset border is applied to a table or a table cell.", + "Outset": "The label for the border style dropdown when the outset border is applied to a table or a table cell.", + "Align cell text to the left": "The label used by assistive technologies describing a button that aligns the table cell text to the left.", + "Align cell text to the center": "The label used by assistive technologies describing a button that aligns the table cell text to the center.", + "Align cell text to the right": "The label used by assistive technologies describing a button that aligns the table cell text to the right.", + "Justify cell text": "The label used by assistive technologies describing a button that justifies the table cell text.", + "Align cell text to the top": "The label used by assistive technologies describing a button that aligns the table cell text to the top.", + "Align cell text to the middle": "The label used by assistive technologies describing a button that aligns the table cell text to the middle.", + "Align cell text to the bottom": "The label used by assistive technologies describing a button that aligns the table cell text to the bottom." } From 782e3c574bdf0485e9cc93f1918b5686d51d5f68 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 22 Jan 2020 13:11:49 +0100 Subject: [PATCH 27/33] Code refactoring. --- src/ui/utils.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/ui/utils.js b/src/ui/utils.js index 58a510af..4124278a 100644 --- a/src/ui/utils.js +++ b/src/ui/utils.js @@ -37,17 +37,13 @@ export function repositionContextualBalloon( editor ) { * @returns {module:utils/dom/position~Options} */ export function getBalloonCellPositionData( editor ) { - const model = editor.model; - const document = model.document; - const selection = document.selection; - const editingView = editor.editing.view; - const firstPosition = selection.getFirstPosition(); + const firstPosition = editor.model.document.selection.getFirstPosition(); const modelTableCell = findAncestor( 'tableCell', firstPosition ); const viewTableCell = editor.editing.mapper.toViewElement( modelTableCell ); const defaultPositions = BalloonPanelView.defaultPositions; return { - target: editingView.domConverter.viewToDom( viewTableCell ), + target: editor.editing.view.domConverter.viewToDom( viewTableCell ), positions: [ defaultPositions.northArrowSouth, defaultPositions.northArrowSouthWest, From 6bea9324337fb265180c12ecd7fdaafd9195c1fa Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 22 Jan 2020 13:15:14 +0100 Subject: [PATCH 28/33] Tests: Code refactoring. --- .../tablecellpropertiesui.js | 484 ++++++----- .../ui/tablecellpropertiesview.js | 820 +++++++++--------- 2 files changed, 655 insertions(+), 649 deletions(-) diff --git a/tests/tablecellproperties/tablecellpropertiesui.js b/tests/tablecellproperties/tablecellpropertiesui.js index 843653d1..3c94afe9 100644 --- a/tests/tablecellproperties/tablecellpropertiesui.js +++ b/tests/tablecellproperties/tablecellpropertiesui.js @@ -21,324 +21,326 @@ import TableCellPropertiesEditing from '../../src/tablecellproperties/tablecellp import TableCellPropertiesUI from '../../src/tablecellproperties/tablecellpropertiesui'; import TableCellPropertiesUIView from '../../src/tablecellproperties/ui/tablecellpropertiesview'; -describe( 'TableCellPropertiesUI', () => { - let editor, editorElement, contextualBalloon, - tableCellPropertiesUI, tableCellPropertiesView, tableCellPropertiesButton; - - testUtils.createSinonSandbox(); - - beforeEach( () => { - editorElement = document.createElement( 'div' ); - document.body.appendChild( editorElement ); - - return ClassicTestEditor - .create( editorElement, { - plugins: [ Table, TableCellPropertiesEditing, TableCellPropertiesUI, Paragraph, Undo ], - initialData: '
foo

bar

' - } ) - .then( newEditor => { - editor = newEditor; - - tableCellPropertiesUI = editor.plugins.get( TableCellPropertiesUI ); - tableCellPropertiesButton = editor.ui.componentFactory.create( 'tableCellProperties' ); - contextualBalloon = editor.plugins.get( ContextualBalloon ); - tableCellPropertiesView = tableCellPropertiesUI.view; - - // There is no point to execute BalloonPanelView attachTo and pin methods so lets override it. - testUtils.sinon.stub( contextualBalloon.view, 'attachTo' ).returns( {} ); - testUtils.sinon.stub( contextualBalloon.view, 'pin' ).returns( {} ); - } ); - } ); - - afterEach( () => { - editorElement.remove(); +describe( 'table cell properties', () => { + describe( 'TableCellPropertiesUI', () => { + let editor, editorElement, contextualBalloon, + tableCellPropertiesUI, tableCellPropertiesView, tableCellPropertiesButton; - return editor.destroy(); - } ); + testUtils.createSinonSandbox(); - it( 'should be named', () => { - expect( TableCellPropertiesUI.pluginName ).to.equal( 'TableCellPropertiesUI' ); - } ); + beforeEach( () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor + .create( editorElement, { + plugins: [ Table, TableCellPropertiesEditing, TableCellPropertiesUI, Paragraph, Undo ], + initialData: '
foo

bar

' + } ) + .then( newEditor => { + editor = newEditor; + + tableCellPropertiesUI = editor.plugins.get( TableCellPropertiesUI ); + tableCellPropertiesButton = editor.ui.componentFactory.create( 'tableCellProperties' ); + contextualBalloon = editor.plugins.get( ContextualBalloon ); + tableCellPropertiesView = tableCellPropertiesUI.view; + + // There is no point to execute BalloonPanelView attachTo and pin methods so lets override it. + testUtils.sinon.stub( contextualBalloon.view, 'attachTo' ).returns( {} ); + testUtils.sinon.stub( contextualBalloon.view, 'pin' ).returns( {} ); + } ); + } ); - it( 'should load ContextualBalloon', () => { - expect( editor.plugins.get( ContextualBalloon ) ).to.be.instanceOf( ContextualBalloon ); - } ); + afterEach( () => { + editorElement.remove(); - describe( 'init()', () => { - it( 'should set a batch', () => { - expect( tableCellPropertiesUI._undoStepBatch ).to.be.null; + return editor.destroy(); } ); - describe( '#view', () => { - it( 'should be created', () => { - expect( tableCellPropertiesUI.view ).to.be.instanceOf( TableCellPropertiesUIView ); - } ); + it( 'should be named', () => { + expect( TableCellPropertiesUI.pluginName ).to.equal( 'TableCellPropertiesUI' ); + } ); - it( 'should be rendered', () => { - expect( tableCellPropertiesUI.view.isRendered ).to.be.true; - } ); + it( 'should load ContextualBalloon', () => { + expect( editor.plugins.get( ContextualBalloon ) ).to.be.instanceOf( ContextualBalloon ); } ); - describe( 'toolbar button', () => { - it( 'should be registered', () => { - expect( tableCellPropertiesButton ).to.be.instanceOf( ButtonView ); + describe( 'init()', () => { + it( 'should set a batch', () => { + expect( tableCellPropertiesUI._undoStepBatch ).to.be.null; } ); - it( 'should have a label', () => { - expect( tableCellPropertiesButton.label ).to.equal( 'Cell properties' ); - } ); + describe( '#view', () => { + it( 'should be created', () => { + expect( tableCellPropertiesUI.view ).to.be.instanceOf( TableCellPropertiesUIView ); + } ); - it( 'should have a tooltip', () => { - expect( tableCellPropertiesButton.tooltip ).to.be.true; + it( 'should be rendered', () => { + expect( tableCellPropertiesUI.view.isRendered ).to.be.true; + } ); } ); - it( 'should call #_showView upon #execute', () => { - const spy = testUtils.sinon.stub( tableCellPropertiesUI, '_showView' ).returns( {} ); + describe( 'toolbar button', () => { + it( 'should be registered', () => { + expect( tableCellPropertiesButton ).to.be.instanceOf( ButtonView ); + } ); - tableCellPropertiesButton.fire( 'execute' ); - sinon.assert.calledOnce( spy ); - } ); - } ); - } ); + it( 'should have a label', () => { + expect( tableCellPropertiesButton.label ).to.equal( 'Cell properties' ); + } ); - describe( 'destroy()', () => { + it( 'should have a tooltip', () => { + expect( tableCellPropertiesButton.tooltip ).to.be.true; + } ); - } ); + it( 'should call #_showView upon #execute', () => { + const spy = testUtils.sinon.stub( tableCellPropertiesUI, '_showView' ).returns( {} ); - describe( 'Properties #view', () => { - beforeEach( () => { - editor.model.change( writer => { - writer.setSelection( editor.model.document.getRoot().getChild( 0 ).getChild( 0 ).getChild( 0 ), 0 ); + tableCellPropertiesButton.fire( 'execute' ); + sinon.assert.calledOnce( spy ); + } ); } ); } ); - it( 'should hide on #submit', () => { - tableCellPropertiesButton.fire( 'execute' ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + describe( 'destroy()', () => { - tableCellPropertiesView.fire( 'submit' ); - expect( contextualBalloon.visibleView ).to.be.null; } ); - it( 'should undo the entire batch of changes on #cancel', () => { - // Show the view. New batch will be created. - tableCellPropertiesButton.fire( 'execute' ); - - // Do the changes like a user. - tableCellPropertiesView.borderStyle = 'dotted'; - tableCellPropertiesView.backgroundColor = 'red'; - - expect( getModelData( editor.model ) ).to.equal( - '' + - '' + - '' + - '[]foo' + - '' + - '' + - '
' + - 'bar' - ); - - tableCellPropertiesView.fire( 'cancel' ); - - expect( getModelData( editor.model ) ).to.equal( - '' + - '' + - '' + - '[]foo' + - '' + - '' + - '
' + - 'bar' - ); - } ); - - it( 'should hide on #cancel', () => { - tableCellPropertiesButton.fire( 'execute' ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + describe( 'Properties #view', () => { + beforeEach( () => { + editor.model.change( writer => { + writer.setSelection( editor.model.document.getRoot().getChild( 0 ).getChild( 0 ).getChild( 0 ), 0 ); + } ); + } ); - tableCellPropertiesView.fire( 'cancel' ); - expect( contextualBalloon.visibleView ).to.be.null; - } ); + it( 'should hide on #submit', () => { + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); - it( 'should hide on the Esc key press', () => { - const keyEvtData = { - keyCode: keyCodes.esc, - preventDefault: sinon.spy(), - stopPropagation: sinon.spy() - }; + tableCellPropertiesView.fire( 'submit' ); + expect( contextualBalloon.visibleView ).to.be.null; + } ); - tableCellPropertiesButton.fire( 'execute' ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + it( 'should undo the entire batch of changes on #cancel', () => { + // Show the view. New batch will be created. + tableCellPropertiesButton.fire( 'execute' ); - tableCellPropertiesView.keystrokes.press( keyEvtData ); - expect( contextualBalloon.visibleView ).to.be.null; - } ); + // Do the changes like a user. + tableCellPropertiesView.borderStyle = 'dotted'; + tableCellPropertiesView.backgroundColor = 'red'; + + expect( getModelData( editor.model ) ).to.equal( + '' + + '' + + '' + + '[]foo' + + '' + + '' + + '
' + + 'bar' + ); + + tableCellPropertiesView.fire( 'cancel' ); + + expect( getModelData( editor.model ) ).to.equal( + '' + + '' + + '' + + '[]foo' + + '' + + '' + + '
' + + 'bar' + ); + } ); - it( 'should hide if the table cell is no longer selected on EditorUI#update', () => { - tableCellPropertiesButton.fire( 'execute' ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + it( 'should hide on #cancel', () => { + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); - editor.model.change( writer => { - // Set selection in the paragraph. - writer.setSelection( editor.model.document.getRoot().getChild( 1 ), 0 ); + tableCellPropertiesView.fire( 'cancel' ); + expect( contextualBalloon.visibleView ).to.be.null; } ); - expect( contextualBalloon.visibleView ).to.be.null; - } ); + it( 'should hide on the Esc key press', () => { + const keyEvtData = { + keyCode: keyCodes.esc, + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; - it( 'should reposition if table cell is still selected on on EditorUI#update', () => { - tableCellPropertiesButton.fire( 'execute' ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); - editor.model.change( writer => { - writer.insertText( 'qux', editor.model.document.selection.getFirstPosition() ); + tableCellPropertiesView.keystrokes.press( keyEvtData ); + expect( contextualBalloon.visibleView ).to.be.null; } ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); - } ); - - it( 'should hide if clicked outside the balloon', () => { - tableCellPropertiesButton.fire( 'execute' ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + it( 'should hide if the table cell is no longer selected on EditorUI#update', () => { + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); - document.body.dispatchEvent( new Event( 'mousedown', { bubbles: true } ) ); + editor.model.change( writer => { + // Set selection in the paragraph. + writer.setSelection( editor.model.document.getRoot().getChild( 1 ), 0 ); + } ); - expect( contextualBalloon.visibleView ).to.be.null; - } ); + expect( contextualBalloon.visibleView ).to.be.null; + } ); - describe( 'property changes', () => { - it( 'should affect the editor state', () => { - const spy = testUtils.sinon.stub( editor, 'execute' ); + it( 'should reposition if table cell is still selected on on EditorUI#update', () => { + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); - tableCellPropertiesUI._undoStepBatch = 'foo'; - tableCellPropertiesView.fire( 'change', 'borderStyle', 'dotted' ); + editor.model.change( writer => { + writer.insertText( 'qux', editor.model.document.selection.getFirstPosition() ); + } ); - sinon.assert.calledOnce( spy ); - sinon.assert.calledWithExactly( spy, 'tableCellBorderStyle', { value: 'dotted', batch: 'foo' } ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); } ); - it( 'should not affect the editor state if internal property has changed', () => { - const spy = testUtils.sinon.stub( editor, 'execute' ); + it( 'should hide if clicked outside the balloon', () => { + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); - tableCellPropertiesUI._undoStepBatch = 'foo'; - tableCellPropertiesView.fire( 'change', 'internalProp', 'foo' ); + document.body.dispatchEvent( new Event( 'mousedown', { bubbles: true } ) ); - sinon.assert.notCalled( spy ); + expect( contextualBalloon.visibleView ).to.be.null; } ); - } ); - } ); - describe( 'Showing the #view', () => { - beforeEach( () => { - editor.model.change( writer => { - writer.setSelection( editor.model.document.getRoot().getChild( 0 ).getChild( 0 ).getChild( 0 ), 0 ); - } ); - } ); + describe( 'property changes', () => { + it( 'should affect the editor state', () => { + const spy = testUtils.sinon.stub( editor, 'execute' ); - it( 'should create a new undoable batch for further #view cancel', () => { - tableCellPropertiesButton.fire( 'execute' ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + tableCellPropertiesUI._undoStepBatch = 'foo'; + tableCellPropertiesView.fire( 'change', 'borderStyle', 'dotted' ); - const firstBatch = tableCellPropertiesUI._undoStepBatch; - expect( firstBatch ).to.be.instanceOf( Batch ); + sinon.assert.calledOnce( spy ); + sinon.assert.calledWithExactly( spy, 'tableCellBorderStyle', { value: 'dotted', batch: 'foo' } ); + } ); - tableCellPropertiesView.fire( 'submit' ); - expect( contextualBalloon.visibleView ).to.be.null; + it( 'should not affect the editor state if internal property has changed', () => { + const spy = testUtils.sinon.stub( editor, 'execute' ); - tableCellPropertiesButton.fire( 'execute' ); + tableCellPropertiesUI._undoStepBatch = 'foo'; + tableCellPropertiesView.fire( 'change', 'internalProp', 'foo' ); - const secondBatch = tableCellPropertiesUI._undoStepBatch; - expect( secondBatch ).to.be.instanceOf( Batch ); - expect( firstBatch ).to.not.equal( secondBatch ); + sinon.assert.notCalled( spy ); + } ); + } ); } ); - describe( 'initial data', () => { - it( 'should be set from the command values', () => { - editor.commands.get( 'tableCellBorderStyle' ).value = 'a'; - editor.commands.get( 'tableCellBorderColor' ).value = 'b'; - editor.commands.get( 'tableCellBorderWidth' ).value = 'c'; - editor.commands.get( 'tableCellPadding' ).value = 'd'; - editor.commands.get( 'tableCellBackgroundColor' ).value = 'e'; - editor.commands.get( 'tableCellHorizontalAlignment' ).value = 'f'; - editor.commands.get( 'tableCellVerticalAlignment' ).value = 'g'; + describe( 'Showing the #view', () => { + beforeEach( () => { + editor.model.change( writer => { + writer.setSelection( editor.model.document.getRoot().getChild( 0 ).getChild( 0 ).getChild( 0 ), 0 ); + } ); + } ); + it( 'should create a new undoable batch for further #view cancel', () => { tableCellPropertiesButton.fire( 'execute' ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); - expect( tableCellPropertiesView ).to.include( { - borderStyle: 'a', - borderColor: 'b', - borderWidth: 'c', - padding: 'd', - backgroundColor: 'e', - horizontalAlignment: 'f', - verticalAlignment: 'g' - } ); - } ); - it( 'should use default values when command has no value', () => { - editor.commands.get( 'tableCellBorderStyle' ).value = null; - editor.commands.get( 'tableCellBorderColor' ).value = null; - editor.commands.get( 'tableCellBorderWidth' ).value = null; - editor.commands.get( 'tableCellPadding' ).value = null; - editor.commands.get( 'tableCellBackgroundColor' ).value = null; - editor.commands.get( 'tableCellHorizontalAlignment' ).value = null; - editor.commands.get( 'tableCellVerticalAlignment' ).value = null; + const firstBatch = tableCellPropertiesUI._undoStepBatch; + expect( firstBatch ).to.be.instanceOf( Batch ); + + tableCellPropertiesView.fire( 'submit' ); + expect( contextualBalloon.visibleView ).to.be.null; tableCellPropertiesButton.fire( 'execute' ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); - expect( tableCellPropertiesView ).to.include( { - borderStyle: 'none', - borderColor: '', - borderWidth: '', - padding: '', - backgroundColor: '', - horizontalAlignment: 'left', - verticalAlignment: 'middle' + const secondBatch = tableCellPropertiesUI._undoStepBatch; + expect( secondBatch ).to.be.instanceOf( Batch ); + expect( firstBatch ).to.not.equal( secondBatch ); + } ); + + describe( 'initial data', () => { + it( 'should be set from the command values', () => { + editor.commands.get( 'tableCellBorderStyle' ).value = 'a'; + editor.commands.get( 'tableCellBorderColor' ).value = 'b'; + editor.commands.get( 'tableCellBorderWidth' ).value = 'c'; + editor.commands.get( 'tableCellPadding' ).value = 'd'; + editor.commands.get( 'tableCellBackgroundColor' ).value = 'e'; + editor.commands.get( 'tableCellHorizontalAlignment' ).value = 'f'; + editor.commands.get( 'tableCellVerticalAlignment' ).value = 'g'; + + tableCellPropertiesButton.fire( 'execute' ); + + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + expect( tableCellPropertiesView ).to.include( { + borderStyle: 'a', + borderColor: 'b', + borderWidth: 'c', + padding: 'd', + backgroundColor: 'e', + horizontalAlignment: 'f', + verticalAlignment: 'g' + } ); + } ); + + it( 'should use default values when command has no value', () => { + editor.commands.get( 'tableCellBorderStyle' ).value = null; + editor.commands.get( 'tableCellBorderColor' ).value = null; + editor.commands.get( 'tableCellBorderWidth' ).value = null; + editor.commands.get( 'tableCellPadding' ).value = null; + editor.commands.get( 'tableCellBackgroundColor' ).value = null; + editor.commands.get( 'tableCellHorizontalAlignment' ).value = null; + editor.commands.get( 'tableCellVerticalAlignment' ).value = null; + + tableCellPropertiesButton.fire( 'execute' ); + + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + expect( tableCellPropertiesView ).to.include( { + borderStyle: 'none', + borderColor: '', + borderWidth: '', + padding: '', + backgroundColor: '', + horizontalAlignment: 'left', + verticalAlignment: 'middle' + } ); } ); } ); - } ); - it( 'should focus the form view', () => { - const spy = testUtils.sinon.spy( tableCellPropertiesView, 'focus' ); + it( 'should focus the form view', () => { + const spy = testUtils.sinon.spy( tableCellPropertiesView, 'focus' ); - tableCellPropertiesButton.fire( 'execute' ); + tableCellPropertiesButton.fire( 'execute' ); - sinon.assert.calledOnce( spy ); + sinon.assert.calledOnce( spy ); + } ); } ); - } ); - describe( 'Hiding the #view', () => { - beforeEach( () => { - editor.model.change( writer => { - writer.setSelection( editor.model.document.getRoot().getChild( 0 ).getChild( 0 ).getChild( 0 ), 0 ); + describe( 'Hiding the #view', () => { + beforeEach( () => { + editor.model.change( writer => { + writer.setSelection( editor.model.document.getRoot().getChild( 0 ).getChild( 0 ).getChild( 0 ), 0 ); + } ); } ); - } ); - it( 'should stop listening to EditorUI#update', () => { - const spy = testUtils.sinon.spy( tableCellPropertiesUI, 'stopListening' ); + it( 'should stop listening to EditorUI#update', () => { + const spy = testUtils.sinon.spy( tableCellPropertiesUI, 'stopListening' ); - tableCellPropertiesButton.fire( 'execute' ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); - tableCellPropertiesView.fire( 'submit' ); - expect( contextualBalloon.visibleView ).to.be.null; + tableCellPropertiesView.fire( 'submit' ); + expect( contextualBalloon.visibleView ).to.be.null; - sinon.assert.calledOnce( spy ); - sinon.assert.calledWithExactly( spy, editor.ui, 'update' ); - } ); + sinon.assert.calledOnce( spy ); + sinon.assert.calledWithExactly( spy, editor.ui, 'update' ); + } ); - it( 'should focus the editing view so the focus is not lost', () => { - const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); + it( 'should focus the editing view so the focus is not lost', () => { + const spy = testUtils.sinon.spy( editor.editing.view, 'focus' ); - tableCellPropertiesButton.fire( 'execute' ); - expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); + tableCellPropertiesButton.fire( 'execute' ); + expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView ); - tableCellPropertiesView.fire( 'submit' ); + tableCellPropertiesView.fire( 'submit' ); - sinon.assert.calledOnce( spy ); + sinon.assert.calledOnce( spy ); + } ); } ); } ); } ); diff --git a/tests/tablecellproperties/ui/tablecellpropertiesview.js b/tests/tablecellproperties/ui/tablecellpropertiesview.js index e4255ca9..34ac66c6 100644 --- a/tests/tablecellproperties/ui/tablecellpropertiesview.js +++ b/tests/tablecellproperties/ui/tablecellpropertiesview.js @@ -17,523 +17,527 @@ import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; import InputTextView from '@ckeditor/ckeditor5-ui/src/inputtext/inputtextview'; -describe( 'TableCellPropertiesView', () => { - let view, locale; +describe( 'table cell properties', () => { + describe( 'TableCellPropertiesView', () => { + let view, locale; - testUtils.createSinonSandbox(); + testUtils.createSinonSandbox(); - beforeEach( () => { - locale = { t: val => val }; - view = new TableCellPropertiesView( locale ); - view.render(); - } ); - - afterEach( () => { - view.destroy(); - } ); - - describe( 'constructor()', () => { - it( 'should set view#locale', () => { - expect( view.locale ).to.equal( locale ); + beforeEach( () => { + locale = { t: val => val }; + view = new TableCellPropertiesView( locale ); + view.render(); } ); - it( 'should create view#children collection', () => { - expect( view.children ).to.be.instanceOf( ViewCollection ); + afterEach( () => { + view.destroy(); } ); - it( 'should define the public data interface (observable properties)', () => { - expect( view ).to.include( { - borderStyle: 'none', - borderWidth: '', - borderColor: '', - padding: '', - backgroundColor: '', - horizontalAlignment: 'left', - verticalAlignment: 'middle' + describe( 'constructor()', () => { + it( 'should set view#locale', () => { + expect( view.locale ).to.equal( locale ); } ); - } ); - it( 'should create element from template', () => { - expect( view.element.classList.contains( 'ck' ) ).to.be.true; - expect( view.element.classList.contains( 'ck-form' ) ).to.be.true; - expect( view.element.classList.contains( 'ck-table-cell-properties-form' ) ).to.be.true; - expect( view.element.getAttribute( 'tabindex' ) ).to.equal( '-1' ); - } ); - - it( 'should create child views (and references)', () => { - expect( view.borderStyleDropdown ).to.be.instanceOf( LabeledView ); - expect( view.borderWidthInput ).to.be.instanceOf( LabeledView ); - expect( view.borderColorInput ).to.be.instanceOf( LabeledView ); - expect( view.backgroundInput ).to.be.instanceOf( LabeledView ); - expect( view.paddingInput ).to.be.instanceOf( LabeledView ); - expect( view.horizontalAlignmentToolbar ).to.be.instanceOf( ToolbarView ); - expect( view.verticalAlignmentToolbar ).to.be.instanceOf( ToolbarView ); - - expect( view.saveButtonView ).to.be.instanceOf( ButtonView ); - expect( view.cancelButtonView ).to.be.instanceOf( ButtonView ); - } ); - - it( 'should have a header', () => { - const header = view.element.firstChild; - - expect( header.classList.contains( 'ck' ) ).to.be.true; - expect( header.classList.contains( 'ck-form__header' ) ).to.be.true; - expect( header.textContent ).to.equal( 'Cell properties' ); - } ); + it( 'should create view#children collection', () => { + expect( view.children ).to.be.instanceOf( ViewCollection ); + } ); - describe( 'form rows', () => { - describe( 'border row', () => { - it( 'should be defined', () => { - const row = view.element.childNodes[ 1 ]; - - expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; - expect( row.classList.contains( 'ck-table-cell-properties-form__border-row' ) ).to.be.true; - expect( row.childNodes[ 0 ].textContent ).to.equal( 'Border' ); - expect( row.childNodes[ 1 ] ).to.equal( view.borderStyleDropdown.element ); - expect( row.childNodes[ 2 ] ).to.equal( view.borderColorInput.element ); - expect( row.childNodes[ 3 ] ).to.equal( view.borderWidthInput.element ); + it( 'should define the public data interface (observable properties)', () => { + expect( view ).to.include( { + borderStyle: 'none', + borderWidth: '', + borderColor: '', + padding: '', + backgroundColor: '', + horizontalAlignment: 'left', + verticalAlignment: 'middle' } ); + } ); - describe( 'border style labeled dropdown', () => { - let labeledDropdown; + it( 'should create element from template', () => { + expect( view.element.classList.contains( 'ck' ) ).to.be.true; + expect( view.element.classList.contains( 'ck-form' ) ).to.be.true; + expect( view.element.classList.contains( 'ck-table-cell-properties-form' ) ).to.be.true; + expect( view.element.getAttribute( 'tabindex' ) ).to.equal( '-1' ); + } ); - beforeEach( () => { - labeledDropdown = view.borderStyleDropdown; - } ); + it( 'should create child views (and references)', () => { + expect( view.borderStyleDropdown ).to.be.instanceOf( LabeledView ); + expect( view.borderWidthInput ).to.be.instanceOf( LabeledView ); + expect( view.borderColorInput ).to.be.instanceOf( LabeledView ); + expect( view.backgroundInput ).to.be.instanceOf( LabeledView ); + expect( view.paddingInput ).to.be.instanceOf( LabeledView ); + expect( view.horizontalAlignmentToolbar ).to.be.instanceOf( ToolbarView ); + expect( view.verticalAlignmentToolbar ).to.be.instanceOf( ToolbarView ); + + expect( view.saveButtonView ).to.be.instanceOf( ButtonView ); + expect( view.cancelButtonView ).to.be.instanceOf( ButtonView ); + } ); - it( 'should have properties set', () => { - expect( labeledDropdown.label ).to.equal( 'Style' ); - expect( labeledDropdown.class ).to.equal( 'ck-table-cell-properties-form__border-style' ); - } ); + it( 'should have a header', () => { + const header = view.element.firstChild; - it( 'should have a button with properties set', () => { - expect( labeledDropdown.view.buttonView.isOn ).to.be.false; - expect( labeledDropdown.view.buttonView.withText ).to.be.true; - expect( labeledDropdown.view.buttonView.tooltip ).to.equal( 'Style' ); - } ); + expect( header.classList.contains( 'ck' ) ).to.be.true; + expect( header.classList.contains( 'ck-form__header' ) ).to.be.true; + expect( header.textContent ).to.equal( 'Cell properties' ); + } ); - it( 'should bind button\'s label to #borderStyle property', () => { - view.borderStyle = 'dotted'; - expect( labeledDropdown.view.buttonView.label ).to.equal( 'Dotted' ); + describe( 'form rows', () => { + describe( 'border row', () => { + it( 'should be defined', () => { + const row = view.element.childNodes[ 1 ]; - view.borderStyle = 'dashed'; - expect( labeledDropdown.view.buttonView.label ).to.equal( 'Dashed' ); + expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; + expect( row.classList.contains( 'ck-table-cell-properties-form__border-row' ) ).to.be.true; + expect( row.childNodes[ 0 ].textContent ).to.equal( 'Border' ); + expect( row.childNodes[ 1 ] ).to.equal( view.borderStyleDropdown.element ); + expect( row.childNodes[ 2 ] ).to.equal( view.borderColorInput.element ); + expect( row.childNodes[ 3 ] ).to.equal( view.borderWidthInput.element ); } ); - it( 'should change #borderStyle when executed', () => { - labeledDropdown.view.listView.items.first.children.first.fire( 'execute' ); - expect( view.borderStyle ).to.equal( 'none' ); + describe( 'border style labeled dropdown', () => { + let labeledDropdown; - labeledDropdown.view.listView.items.last.children.first.fire( 'execute' ); - expect( view.borderStyle ).to.equal( 'outset' ); - } ); + beforeEach( () => { + labeledDropdown = view.borderStyleDropdown; + } ); - it( 'should come with a set of pre–defined border styles', () => { - expect( labeledDropdown.view.listView.items.map( item => item.children.first.label ) ).to.have.ordered.members( [ - 'None', 'Solid', 'Dotted', 'Dashed', 'Double', 'Groove', 'Ridge', 'Inset', 'Outset', - ] ); - } ); - } ); + it( 'should have properties set', () => { + expect( labeledDropdown.label ).to.equal( 'Style' ); + expect( labeledDropdown.class ).to.equal( 'ck-table-cell-properties-form__border-style' ); + } ); - describe( 'border width input', () => { - let labeledInput; + it( 'should have a button with properties set', () => { + expect( labeledDropdown.view.buttonView.isOn ).to.be.false; + expect( labeledDropdown.view.buttonView.withText ).to.be.true; + expect( labeledDropdown.view.buttonView.tooltip ).to.equal( 'Style' ); + } ); + + it( 'should bind button\'s label to #borderStyle property', () => { + view.borderStyle = 'dotted'; + expect( labeledDropdown.view.buttonView.label ).to.equal( 'Dotted' ); - beforeEach( () => { - labeledInput = view.borderWidthInput; - } ); + view.borderStyle = 'dashed'; + expect( labeledDropdown.view.buttonView.label ).to.equal( 'Dashed' ); + } ); - it( 'should be created', () => { - expect( labeledInput.view ).to.be.instanceOf( InputTextView ); - expect( labeledInput.label ).to.equal( 'Width' ); - expect( labeledInput.class ).to.equal( 'ck-table-cell-properties-form__border-width' ); - } ); + it( 'should change #borderStyle when executed', () => { + labeledDropdown.view.listView.items.first.children.first.fire( 'execute' ); + expect( view.borderStyle ).to.equal( 'none' ); - it( 'should reflect #borderWidth property', () => { - view.borderWidth = 'foo'; - expect( labeledInput.view.value ).to.equal( 'foo' ); + labeledDropdown.view.listView.items.last.children.first.fire( 'execute' ); + expect( view.borderStyle ).to.equal( 'outset' ); + } ); - view.borderWidth = 'bar'; - expect( labeledInput.view.value ).to.equal( 'bar' ); + it( 'should come with a set of pre–defined border styles', () => { + 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', + ] ); + } ); } ); - it( 'should be enabled only when #borderStyle is different than "none"', () => { - view.borderStyle = 'none'; - expect( labeledInput.isEnabled ).to.be.false; + describe( 'border width input', () => { + let labeledInput; - view.borderStyle = 'dotted'; - expect( labeledInput.isEnabled ).to.be.true; - } ); + beforeEach( () => { + labeledInput = view.borderWidthInput; + } ); - it( 'should update #borderWidth on DOM "input" event', () => { - labeledInput.view.element.value = 'foo'; - labeledInput.view.fire( 'input' ); - expect( view.borderWidth ).to.equal( 'foo' ); + it( 'should be created', () => { + expect( labeledInput.view ).to.be.instanceOf( InputTextView ); + expect( labeledInput.label ).to.equal( 'Width' ); + expect( labeledInput.class ).to.equal( 'ck-table-cell-properties-form__border-width' ); + } ); - labeledInput.view.element.value = 'bar'; - labeledInput.view.fire( 'input' ); - expect( view.borderWidth ).to.equal( 'bar' ); - } ); - } ); + it( 'should reflect #borderWidth property', () => { + view.borderWidth = 'foo'; + expect( labeledInput.view.value ).to.equal( 'foo' ); - describe( 'border color input', () => { - let labeledInput; + view.borderWidth = 'bar'; + expect( labeledInput.view.value ).to.equal( 'bar' ); + } ); - beforeEach( () => { - labeledInput = view.borderColorInput; - } ); + it( 'should be enabled only when #borderStyle is different than "none"', () => { + view.borderStyle = 'none'; + expect( labeledInput.isEnabled ).to.be.false; - it( 'should be created', () => { - expect( labeledInput.view ).to.be.instanceOf( InputTextView ); - expect( labeledInput.label ).to.equal( 'Color' ); - } ); + view.borderStyle = 'dotted'; + expect( labeledInput.isEnabled ).to.be.true; + } ); - it( 'should reflect #borderColor property', () => { - view.borderColor = 'foo'; - expect( labeledInput.view.value ).to.equal( 'foo' ); + it( 'should update #borderWidth on DOM "input" event', () => { + labeledInput.view.element.value = 'foo'; + labeledInput.view.fire( 'input' ); + expect( view.borderWidth ).to.equal( 'foo' ); - view.borderColor = 'bar'; - expect( labeledInput.view.value ).to.equal( 'bar' ); + labeledInput.view.element.value = 'bar'; + labeledInput.view.fire( 'input' ); + expect( view.borderWidth ).to.equal( 'bar' ); + } ); } ); - it( 'should be enabled only when #borderStyle is different than "none"', () => { - view.borderStyle = 'none'; - expect( labeledInput.isEnabled ).to.be.false; + describe( 'border color input', () => { + let labeledInput; - view.borderStyle = 'dotted'; - expect( labeledInput.isEnabled ).to.be.true; - } ); + beforeEach( () => { + labeledInput = view.borderColorInput; + } ); - it( 'should update #borderColor on DOM "input" event', () => { - labeledInput.view.element.value = 'foo'; - labeledInput.view.fire( 'input' ); - expect( view.borderColor ).to.equal( 'foo' ); + it( 'should be created', () => { + expect( labeledInput.view ).to.be.instanceOf( InputTextView ); + expect( labeledInput.label ).to.equal( 'Color' ); + } ); - labeledInput.view.element.value = 'bar'; - labeledInput.view.fire( 'input' ); - expect( view.borderColor ).to.equal( 'bar' ); - } ); - } ); - } ); - - describe( 'background and padding row', () => { - it( 'should be defined', () => { - const row = view.element.childNodes[ 2 ]; + it( 'should reflect #borderColor property', () => { + view.borderColor = 'foo'; + expect( labeledInput.view.value ).to.equal( 'foo' ); - expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; - expect( row.childNodes[ 0 ] ).to.equal( view.backgroundInput.element ); - expect( row.childNodes[ 1 ] ).to.equal( view.paddingInput.element ); - } ); - - describe( 'background color input', () => { - let labeledInput; + view.borderColor = 'bar'; + expect( labeledInput.view.value ).to.equal( 'bar' ); + } ); - beforeEach( () => { - labeledInput = view.backgroundInput; - } ); + it( 'should be enabled only when #borderStyle is different than "none"', () => { + view.borderStyle = 'none'; + expect( labeledInput.isEnabled ).to.be.false; - it( 'should be created', () => { - expect( labeledInput.view ).to.be.instanceOf( InputTextView ); - expect( labeledInput.label ).to.equal( 'Background' ); - expect( labeledInput.class ).to.equal( 'ck-table-cell-properties-form__background' ); - } ); + view.borderStyle = 'dotted'; + expect( labeledInput.isEnabled ).to.be.true; + } ); - it( 'should reflect #backgroundColor property', () => { - view.backgroundColor = 'foo'; - expect( labeledInput.view.value ).to.equal( 'foo' ); + it( 'should update #borderColor on DOM "input" event', () => { + labeledInput.view.element.value = 'foo'; + labeledInput.view.fire( 'input' ); + expect( view.borderColor ).to.equal( 'foo' ); - view.backgroundColor = 'bar'; - expect( labeledInput.view.value ).to.equal( 'bar' ); + labeledInput.view.element.value = 'bar'; + labeledInput.view.fire( 'input' ); + expect( view.borderColor ).to.equal( 'bar' ); + } ); } ); + } ); - it( 'should update #backgroundColor on DOM "input" event', () => { - labeledInput.view.element.value = 'foo'; - labeledInput.view.fire( 'input' ); - expect( view.backgroundColor ).to.equal( 'foo' ); + describe( 'background and padding row', () => { + it( 'should be defined', () => { + const row = view.element.childNodes[ 2 ]; - labeledInput.view.element.value = 'bar'; - labeledInput.view.fire( 'input' ); - expect( view.backgroundColor ).to.equal( 'bar' ); + expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; + expect( row.childNodes[ 0 ] ).to.equal( view.backgroundInput.element ); + expect( row.childNodes[ 1 ] ).to.equal( view.paddingInput.element ); } ); - } ); - describe( 'padding input', () => { - let labeledInput; + describe( 'background color input', () => { + let labeledInput; - beforeEach( () => { - labeledInput = view.paddingInput; - } ); + beforeEach( () => { + labeledInput = view.backgroundInput; + } ); - it( 'should be created', () => { - expect( labeledInput.view ).to.be.instanceOf( InputTextView ); - expect( labeledInput.label ).to.equal( 'Padding' ); - expect( labeledInput.class ).to.equal( 'ck-table-cell-properties-form__padding' ); - } ); + it( 'should be created', () => { + expect( labeledInput.view ).to.be.instanceOf( InputTextView ); + expect( labeledInput.label ).to.equal( 'Background' ); + expect( labeledInput.class ).to.equal( 'ck-table-cell-properties-form__background' ); + } ); - it( 'should reflect #padding property', () => { - view.padding = 'foo'; - expect( labeledInput.view.value ).to.equal( 'foo' ); + it( 'should reflect #backgroundColor property', () => { + view.backgroundColor = 'foo'; + expect( labeledInput.view.value ).to.equal( 'foo' ); - view.padding = 'bar'; - expect( labeledInput.view.value ).to.equal( 'bar' ); - } ); + view.backgroundColor = 'bar'; + expect( labeledInput.view.value ).to.equal( 'bar' ); + } ); - it( 'should update #padding on DOM "input" event', () => { - labeledInput.view.element.value = 'foo'; - labeledInput.view.fire( 'input' ); - expect( view.padding ).to.equal( 'foo' ); + it( 'should update #backgroundColor on DOM "input" event', () => { + labeledInput.view.element.value = 'foo'; + labeledInput.view.fire( 'input' ); + expect( view.backgroundColor ).to.equal( 'foo' ); - labeledInput.view.element.value = 'bar'; - labeledInput.view.fire( 'input' ); - expect( view.padding ).to.equal( 'bar' ); + labeledInput.view.element.value = 'bar'; + labeledInput.view.fire( 'input' ); + expect( view.backgroundColor ).to.equal( 'bar' ); + } ); } ); - } ); - } ); - describe( 'text alignment row', () => { - it( 'should be defined', () => { - const row = view.element.childNodes[ 3 ]; + describe( 'padding input', () => { + let labeledInput; - expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; - expect( row.classList.contains( 'ck-table-cell-properties-form__alignment-row' ) ).to.be.true; - expect( row.childNodes[ 0 ].textContent ).to.equal( 'Text alignment' ); - expect( row.childNodes[ 1 ] ).to.equal( view.horizontalAlignmentToolbar.element ); - expect( row.childNodes[ 2 ] ).to.equal( view.verticalAlignmentToolbar.element ); - } ); + beforeEach( () => { + labeledInput = view.paddingInput; + } ); - describe( 'horizontal text alignment toolbar', () => { - let toolbar; + it( 'should be created', () => { + expect( labeledInput.view ).to.be.instanceOf( InputTextView ); + expect( labeledInput.label ).to.equal( 'Padding' ); + expect( labeledInput.class ).to.equal( 'ck-table-cell-properties-form__padding' ); + } ); - beforeEach( () => { - toolbar = view.horizontalAlignmentToolbar; - } ); + it( 'should reflect #padding property', () => { + view.padding = 'foo'; + expect( labeledInput.view.value ).to.equal( 'foo' ); - it( 'should be defined', () => { - expect( toolbar ).to.be.instanceOf( ToolbarView ); - } ); + view.padding = 'bar'; + expect( labeledInput.view.value ).to.equal( 'bar' ); + } ); - it( 'should have an ARIA label', () => { - expect( toolbar.ariaLabel ).to.equal( 'Horizontal text alignment toolbar' ); - } ); + it( 'should update #padding on DOM "input" event', () => { + labeledInput.view.element.value = 'foo'; + labeledInput.view.fire( 'input' ); + expect( view.padding ).to.equal( 'foo' ); - it( 'should bring alignment buttons', () => { - expect( toolbar.items.map( ( { label } ) => label ) ).to.have.ordered.members( [ - 'Align cell text to the left', - 'Align cell text to the center', - 'Align cell text to the right', - 'Justify cell text', - ] ); - - expect( toolbar.items.map( ( { isOn } ) => isOn ) ).to.have.ordered.members( [ - true, false, false, false - ] ); + labeledInput.view.element.value = 'bar'; + labeledInput.view.fire( 'input' ); + expect( view.padding ).to.equal( 'bar' ); + } ); } ); + } ); - it( 'should change the #horizontalAlignment value', () => { - toolbar.items.last.fire( 'execute' ); - expect( view.horizontalAlignment ).to.equal( 'justify' ); - expect( toolbar.items.last.isOn ).to.be.true; - - toolbar.items.first.fire( 'execute' ); - expect( view.horizontalAlignment ).to.equal( 'left' ); - expect( toolbar.items.last.isOn ).to.be.false; - expect( toolbar.items.first.isOn ).to.be.true; + describe( 'text alignment row', () => { + it( 'should be defined', () => { + const row = view.element.childNodes[ 3 ]; + + expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; + expect( row.classList.contains( 'ck-table-cell-properties-form__alignment-row' ) ).to.be.true; + expect( row.childNodes[ 0 ].textContent ).to.equal( 'Text alignment' ); + expect( row.childNodes[ 1 ] ).to.equal( view.horizontalAlignmentToolbar.element ); + expect( row.childNodes[ 2 ] ).to.equal( view.verticalAlignmentToolbar.element ); + } ); + + describe( 'horizontal text alignment toolbar', () => { + let toolbar; + + beforeEach( () => { + toolbar = view.horizontalAlignmentToolbar; + } ); + + it( 'should be defined', () => { + expect( toolbar ).to.be.instanceOf( ToolbarView ); + } ); + + it( 'should have an ARIA label', () => { + expect( toolbar.ariaLabel ).to.equal( 'Horizontal text alignment toolbar' ); + } ); + + it( 'should bring alignment buttons', () => { + expect( toolbar.items.map( ( { label } ) => label ) ).to.have.ordered.members( [ + 'Align cell text to the left', + 'Align cell text to the center', + 'Align cell text to the right', + 'Justify cell text', + ] ); + + expect( toolbar.items.map( ( { isOn } ) => isOn ) ).to.have.ordered.members( [ + true, false, false, false + ] ); + } ); + + it( 'should change the #horizontalAlignment value', () => { + toolbar.items.last.fire( 'execute' ); + expect( view.horizontalAlignment ).to.equal( 'justify' ); + expect( toolbar.items.last.isOn ).to.be.true; + + toolbar.items.first.fire( 'execute' ); + expect( view.horizontalAlignment ).to.equal( 'left' ); + expect( toolbar.items.last.isOn ).to.be.false; + expect( toolbar.items.first.isOn ).to.be.true; + } ); + } ); + + describe( 'vertical text alignment toolbar', () => { + let toolbar; + + beforeEach( () => { + toolbar = view.verticalAlignmentToolbar; + } ); + + it( 'should be defined', () => { + expect( toolbar ).to.be.instanceOf( ToolbarView ); + } ); + + it( 'should have an ARIA label', () => { + expect( toolbar.ariaLabel ).to.equal( 'Vertical text alignment toolbar' ); + } ); + + it( 'should bring alignment buttons', () => { + expect( toolbar.items.map( ( { label } ) => label ) ).to.have.ordered.members( [ + 'Align cell text to the top', + 'Align cell text to the middle', + 'Align cell text to the bottom', + ] ); + + expect( toolbar.items.map( ( { isOn } ) => isOn ) ).to.have.ordered.members( [ + false, true, false + ] ); + } ); + + it( 'should change the #verticalAlignment value', () => { + toolbar.items.last.fire( 'execute' ); + expect( view.verticalAlignment ).to.equal( 'bottom' ); + expect( toolbar.items.last.isOn ).to.be.true; + + toolbar.items.first.fire( 'execute' ); + expect( view.verticalAlignment ).to.equal( 'top' ); + expect( toolbar.items.last.isOn ).to.be.false; + expect( toolbar.items.first.isOn ).to.be.true; + } ); } ); } ); - describe( 'vertical text alignment toolbar', () => { - let toolbar; + describe( 'action row', () => { + it( 'should be defined', () => { + const row = view.element.childNodes[ 4 ]; - beforeEach( () => { - toolbar = view.verticalAlignmentToolbar; + expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; + expect( row.classList.contains( 'ck-table-form__action-row' ) ).to.be.true; + expect( row.childNodes[ 0 ] ).to.equal( view.saveButtonView.element ); + expect( row.childNodes[ 1 ] ).to.equal( view.cancelButtonView.element ); } ); - it( 'should be defined', () => { - expect( toolbar ).to.be.instanceOf( ToolbarView ); - } ); + it( 'should have buttons with right properties', () => { + expect( view.saveButtonView.label ).to.equal( 'Save' ); + expect( view.saveButtonView.type ).to.equal( 'submit' ); + expect( view.saveButtonView.withText ).to.be.true; + expect( view.saveButtonView.class ).to.equal( 'ck-button-save' ); - it( 'should have an ARIA label', () => { - expect( toolbar.ariaLabel ).to.equal( 'Vertical text alignment toolbar' ); + expect( view.cancelButtonView.label ).to.equal( 'Cancel' ); + expect( view.cancelButtonView.withText ).to.be.true; + expect( view.cancelButtonView.class ).to.equal( 'ck-button-cancel' ); } ); - it( 'should bring alignment buttons', () => { - expect( toolbar.items.map( ( { label } ) => label ) ).to.have.ordered.members( [ - 'Align cell text to the top', - 'Align cell text to the middle', - 'Align cell text to the bottom', - ] ); + it( 'should make the cancel button fire the #cancel event when executed', () => { + const spy = sinon.spy(); - expect( toolbar.items.map( ( { isOn } ) => isOn ) ).to.have.ordered.members( [ - false, true, false - ] ); - } ); + view.on( 'cancel', spy ); - it( 'should change the #verticalAlignment value', () => { - toolbar.items.last.fire( 'execute' ); - expect( view.verticalAlignment ).to.equal( 'bottom' ); - expect( toolbar.items.last.isOn ).to.be.true; + view.cancelButtonView.fire( 'execute' ); - toolbar.items.first.fire( 'execute' ); - expect( view.verticalAlignment ).to.equal( 'top' ); - expect( toolbar.items.last.isOn ).to.be.false; - expect( toolbar.items.first.isOn ).to.be.true; + expect( spy.calledOnce ).to.be.true; } ); } ); } ); - describe( 'action row', () => { - it( 'should be defined', () => { - const row = view.element.childNodes[ 4 ]; - - expect( row.classList.contains( 'ck-form__row' ) ).to.be.true; - expect( row.classList.contains( 'ck-table-form__action-row' ) ).to.be.true; - expect( row.childNodes[ 0 ] ).to.equal( view.saveButtonView.element ); - expect( row.childNodes[ 1 ] ).to.equal( view.cancelButtonView.element ); - } ); - - it( 'should have buttons with right properties', () => { - expect( view.saveButtonView.label ).to.equal( 'Save' ); - expect( view.saveButtonView.type ).to.equal( 'submit' ); - expect( view.saveButtonView.withText ).to.be.true; - expect( view.saveButtonView.class ).to.equal( 'ck-button-save' ); - - expect( view.cancelButtonView.label ).to.equal( 'Cancel' ); - expect( view.cancelButtonView.withText ).to.be.true; - expect( view.cancelButtonView.class ).to.equal( 'ck-button-cancel' ); - } ); - - it( 'should make the cancel button fire the #cancel event when executed', () => { - const spy = sinon.spy(); - - view.on( 'cancel', spy ); - - view.cancelButtonView.fire( 'execute' ); - - expect( spy.calledOnce ).to.be.true; - } ); + it( 'should create #focusTracker instance', () => { + expect( view.focusTracker ).to.be.instanceOf( FocusTracker ); } ); - } ); - it( 'should create #focusTracker instance', () => { - expect( view.focusTracker ).to.be.instanceOf( FocusTracker ); - } ); - - it( 'should create #keystrokes instance', () => { - expect( view.keystrokes ).to.be.instanceOf( KeystrokeHandler ); - } ); - - it( 'should create #_focusCycler instance', () => { - expect( view._focusCycler ).to.be.instanceOf( FocusCycler ); - } ); + it( 'should create #keystrokes instance', () => { + expect( view.keystrokes ).to.be.instanceOf( KeystrokeHandler ); + } ); - it( 'should create #_focusables view collection', () => { - expect( view._focusables ).to.be.instanceOf( ViewCollection ); - } ); - } ); + it( 'should create #_focusCycler instance', () => { + expect( view._focusCycler ).to.be.instanceOf( FocusCycler ); + } ); - describe( 'render()', () => { - it( 'should register child views in #_focusables', () => { - expect( view._focusables.map( f => f ) ).to.have.members( [ - view.borderStyleDropdown, - view.borderColorInput, - view.borderWidthInput, - view.backgroundInput, - view.paddingInput, - view.horizontalAlignmentToolbar, - view.verticalAlignmentToolbar, - view.saveButtonView, - view.cancelButtonView - ] ); + it( 'should create #_focusables view collection', () => { + expect( view._focusables ).to.be.instanceOf( ViewCollection ); + } ); } ); - it( 'should register child views\' #element in #focusTracker', () => { - const spy = testUtils.sinon.spy( FocusTracker.prototype, 'add' ); - const view = new TableCellPropertiesView( { t: val => val } ); - view.render(); - - sinon.assert.calledWith( spy, view.borderStyleDropdown.element ); - sinon.assert.calledWith( spy, view.borderColorInput.element ); - sinon.assert.calledWith( spy, view.borderWidthInput.element ); - sinon.assert.calledWith( spy, view.backgroundInput.element ); - sinon.assert.calledWith( spy, view.paddingInput.element ); - sinon.assert.calledWith( spy, view.horizontalAlignmentToolbar.element ); - sinon.assert.calledWith( spy, view.verticalAlignmentToolbar.element ); - sinon.assert.calledWith( spy, view.saveButtonView.element ); - sinon.assert.calledWith( spy, view.cancelButtonView.element ); + describe( 'render()', () => { + it( 'should register child views in #_focusables', () => { + expect( view._focusables.map( f => f ) ).to.have.members( [ + view.borderStyleDropdown, + view.borderColorInput, + view.borderWidthInput, + view.backgroundInput, + view.paddingInput, + view.horizontalAlignmentToolbar, + view.verticalAlignmentToolbar, + view.saveButtonView, + view.cancelButtonView + ] ); + } ); - view.destroy(); - } ); + it( 'should register child views\' #element in #focusTracker', () => { + const spy = testUtils.sinon.spy( FocusTracker.prototype, 'add' ); + const view = new TableCellPropertiesView( { t: val => val } ); + view.render(); + + sinon.assert.calledWith( spy, view.borderStyleDropdown.element ); + sinon.assert.calledWith( spy, view.borderColorInput.element ); + sinon.assert.calledWith( spy, view.borderWidthInput.element ); + sinon.assert.calledWith( spy, view.backgroundInput.element ); + sinon.assert.calledWith( spy, view.paddingInput.element ); + sinon.assert.calledWith( spy, view.horizontalAlignmentToolbar.element ); + sinon.assert.calledWith( spy, view.verticalAlignmentToolbar.element ); + sinon.assert.calledWith( spy, view.saveButtonView.element ); + sinon.assert.calledWith( spy, view.cancelButtonView.element ); + + view.destroy(); + } ); - it( 'starts listening for #keystrokes coming from #element', () => { - const view = new TableCellPropertiesView( { t: val => val } ); - const spy = sinon.spy( view.keystrokes, 'listenTo' ); + it( 'starts listening for #keystrokes coming from #element', () => { + const view = new TableCellPropertiesView( { t: val => val } ); + const spy = sinon.spy( view.keystrokes, 'listenTo' ); - view.render(); - sinon.assert.calledOnce( spy ); - sinon.assert.calledWithExactly( spy, view.element ); - } ); + view.render(); + sinon.assert.calledOnce( spy ); + sinon.assert.calledWithExactly( spy, view.element ); + } ); - describe( 'activates keyboard navigation for the form', () => { - it( 'so "tab" focuses the next focusable item', () => { - const keyEvtData = { - keyCode: keyCodes.tab, - preventDefault: sinon.spy(), - stopPropagation: sinon.spy() - }; + describe( 'activates keyboard navigation for the form', () => { + it( 'so "tab" focuses the next focusable item', () => { + const keyEvtData = { + keyCode: keyCodes.tab, + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; - // Mock the border style dropdown button is focused. - view.focusTracker.isFocused = true; - view.focusTracker.focusedElement = view.borderStyleDropdown.element; + // Mock the border style dropdown button is focused. + view.focusTracker.isFocused = true; + view.focusTracker.focusedElement = view.borderStyleDropdown.element; - const spy = sinon.spy( view.borderColorInput, 'focus' ); + const spy = sinon.spy( view.borderColorInput, 'focus' ); - view.keystrokes.press( keyEvtData ); - sinon.assert.calledOnce( keyEvtData.preventDefault ); - sinon.assert.calledOnce( keyEvtData.stopPropagation ); - sinon.assert.calledOnce( spy ); - } ); + view.keystrokes.press( keyEvtData ); + sinon.assert.calledOnce( keyEvtData.preventDefault ); + sinon.assert.calledOnce( keyEvtData.stopPropagation ); + sinon.assert.calledOnce( spy ); + } ); - it( 'so "shift + tab" focuses the previous focusable item', () => { - const keyEvtData = { - keyCode: keyCodes.tab, - shiftKey: true, - preventDefault: sinon.spy(), - stopPropagation: sinon.spy() - }; + it( 'so "shift + tab" focuses the previous focusable item', () => { + const keyEvtData = { + keyCode: keyCodes.tab, + shiftKey: true, + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; - // Mock the border style dropdown button is focused. - view.focusTracker.isFocused = true; - view.focusTracker.focusedElement = view.borderStyleDropdown.element; + // Mock the border style dropdown button is focused. + view.focusTracker.isFocused = true; + view.focusTracker.focusedElement = view.borderStyleDropdown.element; - const spy = sinon.spy( view.cancelButtonView, 'focus' ); + const spy = sinon.spy( view.cancelButtonView, 'focus' ); - view.keystrokes.press( keyEvtData ); - sinon.assert.calledOnce( keyEvtData.preventDefault ); - sinon.assert.calledOnce( keyEvtData.stopPropagation ); - sinon.assert.calledOnce( spy ); + view.keystrokes.press( keyEvtData ); + sinon.assert.calledOnce( keyEvtData.preventDefault ); + sinon.assert.calledOnce( keyEvtData.stopPropagation ); + sinon.assert.calledOnce( spy ); + } ); } ); } ); - } ); - describe( 'DOM bindings', () => { - describe( 'submit event', () => { - it( 'should trigger submit event', () => { - const spy = sinon.spy(); + describe( 'DOM bindings', () => { + describe( 'submit event', () => { + it( 'should trigger submit event', () => { + const spy = sinon.spy(); - view.on( 'submit', spy ); - view.element.dispatchEvent( new Event( 'submit' ) ); + view.on( 'submit', spy ); + view.element.dispatchEvent( new Event( 'submit' ) ); - expect( spy.calledOnce ).to.be.true; + expect( spy.calledOnce ).to.be.true; + } ); } ); } ); - } ); - describe( 'focus()', () => { - it( 'focuses the #borderStyleDropdown', () => { - const spy = sinon.spy( view.borderStyleDropdown, 'focus' ); + describe( 'focus()', () => { + it( 'focuses the #borderStyleDropdown', () => { + const spy = sinon.spy( view.borderStyleDropdown, 'focus' ); - view.focus(); + view.focus(); - sinon.assert.calledOnce( spy ); + sinon.assert.calledOnce( spy ); + } ); } ); } ); } ); From ccc7e0c8e79708034aebfa62776faa7070e3492c Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 28 Jan 2020 14:56:29 +0100 Subject: [PATCH 29/33] Code refactoring in the TableCellPropertiesUI class. --- .../tablecellpropertiesui.js | 33 +++++++------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/src/tablecellproperties/tablecellpropertiesui.js b/src/tablecellproperties/tablecellpropertiesui.js index d14bef28..b322bcf9 100644 --- a/src/tablecellproperties/tablecellpropertiesui.js +++ b/src/tablecellproperties/tablecellpropertiesui.js @@ -184,28 +184,17 @@ export default class TableCellPropertiesUI extends Plugin { * @private */ _fillViewFormFromCommandValues() { - const editor = this.editor; - const data = {}; - - for ( const propertyName of CELL_PROPERTIES ) { - let value = editor.commands.get( propertyNameToCommandName( propertyName ) ).value; - - if ( !value ) { - if ( propertyName === 'borderStyle' ) { - value = DEFAULT_BORDER_STYLE; - } else if ( propertyName === 'horizontalAlignment' ) { - value = DEFAULT_HORIZONTAL_ALIGNMENT; - } else if ( propertyName === 'verticalAlignment' ) { - value = DEFAULT_VERTICAL_ALIGNMENT; - } else { - value = ''; - } - } - - data[ propertyName ] = value; - } - - this.view.set( data ); + const commands = this.editor.commands; + + this.view.set( { + borderStyle: commands.get( 'tableCellBorderStyle' ).value || DEFAULT_BORDER_STYLE, + borderColor: commands.get( 'tableCellBorderColor' ).value || '', + borderWidth: commands.get( 'tableCellBorderWidth' ).value || '', + padding: commands.get( 'tableCellPadding' ).value || '', + backgroundColor: commands.get( 'tableCellBackgroundColor' ).value || '', + horizontalAlignment: commands.get( 'tableCellHorizontalAlignment' ).value || DEFAULT_HORIZONTAL_ALIGNMENT, + verticalAlignment: commands.get( 'tableCellVerticalAlignment' ).value || DEFAULT_VERTICAL_ALIGNMENT, + } ); } /** From 579f7ea7af551313a9e106d71f45a8a35cbf2138 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 28 Jan 2020 15:12:14 +0100 Subject: [PATCH 30/33] Code refactoring in the TableCellPropertiesUI class. --- .../tablecellpropertiesui.js | 50 +++++++++---------- .../tablecellpropertiesui.js | 6 ++- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/tablecellproperties/tablecellpropertiesui.js b/src/tablecellproperties/tablecellpropertiesui.js index b322bcf9..37986a11 100644 --- a/src/tablecellproperties/tablecellpropertiesui.js +++ b/src/tablecellproperties/tablecellpropertiesui.js @@ -19,11 +19,6 @@ import { repositionContextualBalloon, getBalloonCellPositionData } from '../ui/u const DEFAULT_BORDER_STYLE = 'none'; const DEFAULT_HORIZONTAL_ALIGNMENT = 'left'; const DEFAULT_VERTICAL_ALIGNMENT = 'middle'; -const CELL_PROPERTIES = [ - 'borderStyle', 'borderColor', 'borderWidth', - 'padding', 'backgroundColor', - 'horizontalAlignment', 'verticalAlignment', -]; /** * The table cell properties UI plugin. It introduces the `'tableCellProperties'` button @@ -154,21 +149,16 @@ export default class TableCellPropertiesUI extends Plugin { } ); // Create the "UI -> editor data" binding. - // This listener updates the editor data (via table commands) when any observable + // 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 // visible in the editing as soon as the user types or changes fields' values. - view.on( 'change', ( evt, propertyName, newValue ) => { - // Not all observable properties of the #view must be related to the cell editing. - // For instance, they can belong to some internal logic. - if ( !CELL_PROPERTIES.includes( propertyName ) ) { - return; - } - - editor.execute( propertyNameToCommandName( propertyName ), { - value: newValue, - batch: this._undoStepBatch - } ); - } ); + 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:horizontalAlignment', this._getPropertyChangeCallback( 'tableCellHorizontalAlignment' ) ); + view.on( 'change:verticalAlignment', this._getPropertyChangeCallback( 'tableCellVerticalAlignment' ) ); return view; } @@ -268,13 +258,21 @@ export default class TableCellPropertiesUI extends Plugin { get _isViewInBalloon() { return this._balloon.hasView( this.view ); } -} -// Translates view's properties into command names. -// -// 'borderWidth' -> 'tableCellBorderWidth' -// -// @param {String} propertyName -function propertyNameToCommandName( propertyName ) { - return `tableCell${ propertyName[ 0 ].toUpperCase() }${ propertyName.slice( 1 ) }`; + /** + * Creates a callback that when executed upon {@link #view view's} property change + * executes a related editor command with the new property value. + * + * @private + * @param {String} commandName + * @returns {Function} + */ + _getPropertyChangeCallback( commandName ) { + return ( evt, propertyName, newValue ) => { + this.editor.execute( commandName, { + value: newValue, + batch: this._undoStepBatch + } ); + }; + } } diff --git a/tests/tablecellproperties/tablecellpropertiesui.js b/tests/tablecellproperties/tablecellpropertiesui.js index 3c94afe9..09568c7c 100644 --- a/tests/tablecellproperties/tablecellpropertiesui.js +++ b/tests/tablecellproperties/tablecellpropertiesui.js @@ -213,7 +213,7 @@ describe( 'table cell properties', () => { const spy = testUtils.sinon.stub( editor, 'execute' ); tableCellPropertiesUI._undoStepBatch = 'foo'; - tableCellPropertiesView.fire( 'change', 'borderStyle', 'dotted' ); + tableCellPropertiesView.borderStyle = 'dotted'; sinon.assert.calledOnce( spy ); sinon.assert.calledWithExactly( spy, 'tableCellBorderStyle', { value: 'dotted', batch: 'foo' } ); @@ -222,8 +222,10 @@ describe( 'table cell properties', () => { it( 'should not affect the editor state if internal property has changed', () => { const spy = testUtils.sinon.stub( editor, 'execute' ); + tableCellPropertiesView.set( 'internalProp', 'foo' ); + tableCellPropertiesUI._undoStepBatch = 'foo'; - tableCellPropertiesView.fire( 'change', 'internalProp', 'foo' ); + tableCellPropertiesView.internalProp = 'bar'; sinon.assert.notCalled( spy ); } ); From 08667fbda99ce2c5052f205b42b7ade60caa4809 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 28 Jan 2020 15:13:13 +0100 Subject: [PATCH 31/33] Update src/tablecellproperties/ui/tablecellpropertiesview.js Co-Authored-By: Maciej --- src/tablecellproperties/ui/tablecellpropertiesview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js index ce96814e..db4122c7 100644 --- a/src/tablecellproperties/ui/tablecellpropertiesview.js +++ b/src/tablecellproperties/ui/tablecellpropertiesview.js @@ -333,7 +333,7 @@ export default class TableCellPropertiesView extends View { this.verticalAlignmentToolbar, this.saveButtonView, this.cancelButtonView - ].forEach( v => { + ].forEach( view => { // Register the view as focusable. this._focusables.add( v ); From 1e9d558205a05ddbf44b603174ea5a56510e1c40 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 28 Jan 2020 15:13:26 +0100 Subject: [PATCH 32/33] Update src/tablecellproperties/ui/tablecellpropertiesview.js Co-Authored-By: Maciej --- src/tablecellproperties/ui/tablecellpropertiesview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js index db4122c7..b4f00d70 100644 --- a/src/tablecellproperties/ui/tablecellpropertiesview.js +++ b/src/tablecellproperties/ui/tablecellpropertiesview.js @@ -338,7 +338,7 @@ export default class TableCellPropertiesView extends View { this._focusables.add( v ); // Register the view in the focus tracker. - this.focusTracker.add( v.element ); + this.focusTracker.add( view.element ); } ); // Mainly for closing using "Esc" and navigation using "Tab". From 21f11d1f21dc6e963eaccbaa815f88234e169161 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 28 Jan 2020 15:13:36 +0100 Subject: [PATCH 33/33] Update src/tablecellproperties/ui/tablecellpropertiesview.js Co-Authored-By: Maciej --- src/tablecellproperties/ui/tablecellpropertiesview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js index b4f00d70..a76c6ddf 100644 --- a/src/tablecellproperties/ui/tablecellpropertiesview.js +++ b/src/tablecellproperties/ui/tablecellpropertiesview.js @@ -335,7 +335,7 @@ export default class TableCellPropertiesView extends View { this.cancelButtonView ].forEach( view => { // Register the view as focusable. - this._focusables.add( v ); + this._focusables.add( view ); // Register the view in the focus tracker. this.focusTracker.add( view.element );