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

Commit

Permalink
Merge pull request #245 from ckeditor/i/6232
Browse files Browse the repository at this point in the history 
Internal: Make default value empty in the model. Closes ckeditor/ckeditor5#6232.
  • Loading branch information
@Reinmar
Reinmar committed Feb 13, 2020
2 parents 97da473 + c3a5fa8 commit b0c0591
Show file tree
Hide file tree
Showing 12 changed files with 169 additions and 86 deletions.
3 changes: 2 additions & 1 deletion src/tablecellproperties/commands/tablecellverticalalignmentcommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ import TableCellPropertyCommand from './tablecellpropertycommand';
*
* * `'top'`
* * `'bottom'`
* * `'middle'`
*
* The `'middle'` value is default one so there's no need to set this value.
*
* @extends module:table/tablecellproperties/commands/tablecellpropertycommand
*/
Expand Down
60 changes: 51 additions & 9 deletions src/tablecellproperties/tablecellpropertiesediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import TableCellBorderStyleCommand from './commands/tablecellborderstylecommand'
import TableCellBorderColorCommand from './commands/tablecellbordercolorcommand';
import TableCellBorderWidthCommand from './commands/tablecellborderwidthcommand';

const VALIGN_VALUES_REG_EXP = /^(top|bottom)$/;

/**
* The table cell properties editing feature.
*
Expand Down Expand Up @@ -92,7 +94,7 @@ export default class TableCellPropertiesEditing extends Plugin {
enableProperty( schema, conversion, 'backgroundColor', 'background-color' );
editor.commands.add( 'tableCellBackgroundColor', new TableCellBackgroundColorCommand( editor ) );

enableProperty( schema, conversion, 'verticalAlignment', 'vertical-align' );
enableVerticalAlignmentProperty( schema, conversion );
editor.commands.add( 'tableCellVerticalAlignment', new TableCellVerticalAlignmentCommand( editor ) );
}
}
Expand Down Expand Up @@ -125,16 +127,9 @@ function enableHorizontalAlignmentProperty( schema, conversion ) {
model: {
name: 'tableCell',
key: 'horizontalAlignment',
values: [ 'left', 'right', 'center', 'justify' ]
values: [ 'right', 'center', 'justify' ]
},
view: {
// TODO: controversial one but I don't know if we want "default".
left: {
key: 'style',
value: {
'text-align': 'left'
}
},
right: {
key: 'style',
value: {
Expand All @@ -157,6 +152,53 @@ function enableHorizontalAlignmentProperty( schema, conversion ) {
} );
}

// Enables the `'verticalAlignment'` attribute for table cells.
//
// @param {module:engine/model/schema~Schema} schema
// @param {module:engine/conversion/conversion~Conversion} conversion
function enableVerticalAlignmentProperty( schema, conversion ) {
schema.extend( 'tableCell', {
allowAttributes: [ 'verticalAlignment' ]
} );

conversion.attributeToAttribute( {
model: {
name: 'tableCell',
key: 'verticalAlignment',
values: [ 'top', 'bottom' ]
},
view: {
top: {
key: 'style',
value: {
'vertical-align': 'top'
}
},
bottom: {
key: 'style',
value: {
'vertical-align': 'bottom'
}
}
}
} );

conversion.for( 'upcast' )
// Support for backwards compatibility and pasting from other sources.
.attributeToAttribute( {
view: {
attributes: {
valign: VALIGN_VALUES_REG_EXP
}
},
model: {
name: 'tableCell',
key: 'verticalAlignment',
value: viewElement => viewElement.getAttribute( 'valign' )
}
} );
}

// Enables conversion for an attribute for simple view-model mappings.
//
// @param {String} modelAttribute
Expand Down
15 changes: 6 additions & 9 deletions src/tablecellproperties/tablecellpropertiesui.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,16 @@ 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,
colorFieldValidator,
getBalloonCellPositionData,
getLocalizedColorErrorText,
getLocalizedLengthErrorText,
colorFieldValidator,
lengthFieldValidator,
lineWidthFieldValidator
lineWidthFieldValidator,
repositionContextualBalloon
} from '../ui/utils';
import { debounce } from 'lodash-es';

const DEFAULT_BORDER_STYLE = 'none';
const DEFAULT_HORIZONTAL_ALIGNMENT = 'left';
const DEFAULT_VERTICAL_ALIGNMENT = 'middle';
const ERROR_TEXT_TIMEOUT = 500;

/**
Expand Down Expand Up @@ -235,15 +232,15 @@ export default class TableCellPropertiesUI extends Plugin {
const commands = this.editor.commands;

this.view.set( {
borderStyle: commands.get( 'tableCellBorderStyle' ).value || DEFAULT_BORDER_STYLE,
borderStyle: commands.get( 'tableCellBorderStyle' ).value || '',
borderColor: commands.get( 'tableCellBorderColor' ).value || '',
borderWidth: commands.get( 'tableCellBorderWidth' ).value || '',
width: commands.get( 'tableCellWidth' ).value || '',
height: commands.get( 'tableCellHeight' ).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,
horizontalAlignment: commands.get( 'tableCellHorizontalAlignment' ).value || '',
verticalAlignment: commands.get( 'tableCellVerticalAlignment' ).value || ''
} );
}

Expand Down
40 changes: 23 additions & 17 deletions src/tablecellproperties/ui/tablecellpropertiesview.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ export default class TableCellPropertiesView extends View {
* The value of the cell border style.
*
* @observable
* @default 'none'
* @default ''
* @member #borderStyle
*/
borderStyle: 'none',
borderStyle: '',

/**
* The value of the cell border width style.
Expand Down Expand Up @@ -130,19 +130,19 @@ export default class TableCellPropertiesView extends View {
* The value of the horizontal text alignment style.
*
* @observable
* @default 'left'
* @default ''
* @member #horizontalAlignment
*/
horizontalAlignment: 'left',
horizontalAlignment: '',

/**
* The value of the vertical text alignment style.
*
* @observable
* @default 'middle'
* @default ''
* @member #verticalAlignment
*/
verticalAlignment: 'middle'
verticalAlignment: ''
} );

const { borderStyleDropdown, borderWidthInput, borderColorInput, borderRowLabel } = this._createBorderFields();
Expand Down Expand Up @@ -455,7 +455,7 @@ export default class TableCellPropertiesView extends View {
} );

borderStyleDropdown.view.buttonView.bind( 'label' ).to( this, 'borderStyle', value => {
return styleLabels[ value ];
return styleLabels[ value ? value : 'none' ];
} );

borderStyleDropdown.view.on( 'execute', evt => {
Expand All @@ -470,13 +470,11 @@ export default class TableCellPropertiesView extends View {

borderWidthInput.set( {
label: t( 'Width' ),
class: 'ck-table-form__border-width',
class: 'ck-table-form__border-width'
} );

borderWidthInput.view.bind( 'value' ).to( this, 'borderWidth' );
borderWidthInput.bind( 'isEnabled' ).to( this, 'borderStyle', value => {
return value !== 'none';
} );
borderWidthInput.bind( 'isEnabled' ).to( this, 'borderStyle', isBorderStyleSet );
borderWidthInput.view.on( 'input', () => {
this.borderWidth = borderWidthInput.view.element.value;
} );
Expand All @@ -486,9 +484,7 @@ export default class TableCellPropertiesView extends View {
const borderColorInput = new LabeledView( locale, createLabeledInputText );
borderColorInput.label = t( 'Color' );
borderColorInput.view.bind( 'value' ).to( this, 'borderColor' );
borderColorInput.bind( 'isEnabled' ).to( this, 'borderStyle', value => {
return value !== 'none';
} );
borderColorInput.bind( 'isEnabled' ).to( this, 'borderStyle', isBorderStyleSet );

borderColorInput.view.on( 'input', () => {
this.borderColor = borderColorInput.view.element.value;
Expand All @@ -497,7 +493,7 @@ export default class TableCellPropertiesView extends View {
// Reset the border color and width fields when style is "none".
// https://github.com/ckeditor/ckeditor5/issues/6227
this.on( 'change:borderStyle', ( evt, name, value ) => {
if ( value === 'none' ) {
if ( !isBorderStyleSet( value ) ) {
this.borderColor = '';
this.borderWidth = '';
}
Expand Down Expand Up @@ -665,7 +661,10 @@ export default class TableCellPropertiesView extends View {
icons: ALIGNMENT_ICONS,
toolbar: horizontalAlignmentToolbar,
labels: this._horizontalAlignmentLabels,
propertyName: 'horizontalAlignment'
propertyName: 'horizontalAlignment',
nameToValue: name => {
return name === 'left' ? '' : name;
}
} );

// -- Vertical -----------------------------------------------------
Expand All @@ -682,7 +681,10 @@ export default class TableCellPropertiesView extends View {
icons: ALIGNMENT_ICONS,
toolbar: verticalAlignmentToolbar,
labels: this._verticalAlignmentLabels,
propertyName: 'verticalAlignment'
propertyName: 'verticalAlignment',
nameToValue: name => {
return name === 'middle' ? '' : name;
}
} );

return {
Expand Down Expand Up @@ -773,3 +775,7 @@ export default class TableCellPropertiesView extends View {
};
}
}

function isBorderStyleSet( value ) {
return !!value;
}
12 changes: 5 additions & 7 deletions src/tableproperties/tablepropertiesui.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,16 @@ import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextu
import TablePropertiesView from './ui/tablepropertiesview';
import tableProperties from './../../theme/icons/table-properties.svg';
import {
repositionContextualBalloon,
colorFieldValidator,
getBalloonTablePositionData,
getLocalizedColorErrorText,
getLocalizedLengthErrorText,
colorFieldValidator,
lengthFieldValidator,
lineWidthFieldValidator
lineWidthFieldValidator,
repositionContextualBalloon
} from '../ui/utils';
import { debounce } from 'lodash-es';

const DEFAULT_BORDER_STYLE = 'none';
const DEFAULT_ALIGNMENT = '';
const ERROR_TEXT_TIMEOUT = 500;

/**
Expand Down Expand Up @@ -226,13 +224,13 @@ export default class TablePropertiesUI extends Plugin {
const commands = this.editor.commands;

this.view.set( {
borderStyle: commands.get( 'tableBorderStyle' ).value || DEFAULT_BORDER_STYLE,
borderStyle: commands.get( 'tableBorderStyle' ).value || '',
borderColor: commands.get( 'tableBorderColor' ).value || '',
borderWidth: commands.get( 'tableBorderWidth' ).value || '',
backgroundColor: commands.get( 'tableBackgroundColor' ).value || '',
width: commands.get( 'tableWidth' ).value || '',
height: commands.get( 'tableHeight' ).value || '',
alignment: commands.get( 'tableAlignment' ).value || DEFAULT_ALIGNMENT,
alignment: commands.get( 'tableAlignment' ).value || '',
} );
}

Expand Down
22 changes: 11 additions & 11 deletions src/tableproperties/ui/tablepropertiesview.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ export default class TablePropertiesView extends View {
* The value of the border style.
*
* @observable
* @default 'none'
* @default ''
* @member #borderStyle
*/
borderStyle: 'none',
borderStyle: '',

/**
* The value of the border width style.
Expand Down Expand Up @@ -401,7 +401,7 @@ export default class TablePropertiesView extends View {
} );

borderStyleDropdown.view.buttonView.bind( 'label' ).to( this, 'borderStyle', value => {
return styleLabels[ value ];
return styleLabels[ value ? value : 'none' ];
} );

borderStyleDropdown.view.on( 'execute', evt => {
Expand All @@ -416,13 +416,11 @@ export default class TablePropertiesView extends View {

borderWidthInput.set( {
label: t( 'Width' ),
class: 'ck-table-form__border-width',
class: 'ck-table-form__border-width'
} );

borderWidthInput.view.bind( 'value' ).to( this, 'borderWidth' );
borderWidthInput.bind( 'isEnabled' ).to( this, 'borderStyle', value => {
return value !== 'none';
} );
borderWidthInput.bind( 'isEnabled' ).to( this, 'borderStyle', isBorderStyleSet );
borderWidthInput.view.on( 'input', () => {
this.borderWidth = borderWidthInput.view.element.value;
} );
Expand All @@ -432,9 +430,7 @@ export default class TablePropertiesView extends View {
const borderColorInput = new LabeledView( locale, createLabeledInputText );
borderColorInput.label = t( 'Color' );
borderColorInput.view.bind( 'value' ).to( this, 'borderColor' );
borderColorInput.bind( 'isEnabled' ).to( this, 'borderStyle', value => {
return value !== 'none';
} );
borderColorInput.bind( 'isEnabled' ).to( this, 'borderStyle', isBorderStyleSet );

borderColorInput.view.on( 'input', () => {
this.borderColor = borderColorInput.view.element.value;
Expand All @@ -443,7 +439,7 @@ export default class TablePropertiesView extends View {
// Reset the border color and width fields when style is "none".
// https://github.com/ckeditor/ckeditor5/issues/6227
this.on( 'change:borderStyle', ( evt, name, value ) => {
if ( value === 'none' ) {
if ( !isBorderStyleSet( value ) ) {
this.borderColor = '';
this.borderWidth = '';
}
Expand Down Expand Up @@ -661,3 +657,7 @@ export default class TablePropertiesView extends View {
};
}
}

function isBorderStyleSet( value ) {
return !!value;
}
Loading

0 comments on commit b0c0591

Please sign in to comment.