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

I/6197: Proper convertion of the table alignment property. #234

Merged
merged 18 commits into from
Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/converters/tableproperties.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ export function downcastTableAttribute( conversion, modelAttribute, styleName )
const { item, attributeNewValue } = data;
const { mapper, writer } = conversionApi;

if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
return;
}

const table = [ ...mapper.toViewElement( item ).getChildren() ].find( child => child.is( 'table' ) );

if ( attributeNewValue ) {
Expand Down
70 changes: 20 additions & 50 deletions src/tableproperties/tablepropertiesediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ import TableWidthCommand from './commands/tablewidthcommand';
import TableHeightCommand from './commands/tableheightcommand';
import TableAlignmentCommand from './commands/tablealignmentcommand';

// RegExp used for matching margin style in converters.
const MARGIN_REG_EXP = /^(auto|0(%|[a-z]{2,4})?)$/;
const ALIGN_VALUES_REG_EXP = /^(left|right|center)$/;
const ALIGN_VALUES_REG_EXP = /^(left|right)$/;

/**
* The table properties editing feature.
Expand Down Expand Up @@ -116,31 +114,32 @@ function enableAlignmentProperty( schema, conversion ) {
schema.extend( 'table', {
allowAttributes: [ 'alignment' ]
} );
conversion.for( 'upcast' )

conversion
.attributeToAttribute( {
view: {
styles: {
'margin-right': MARGIN_REG_EXP,
'margin-left': MARGIN_REG_EXP
}
},
model: {
name: 'table',
key: 'alignment',
value: viewElement => {
// At this point we only have auto or 0 value (with a unit).
if ( viewElement.getStyle( 'margin-right' ) != 'auto' ) {
return 'right';
values: [ 'left', 'right' ]
},
view: {
left: {
key: 'style',
value: {
float: 'left'
}

if ( viewElement.getStyle( 'margin-left' ) != 'auto' ) {
return 'left';
},
right: {
key: 'style',
value: {
float: 'right'
}

return 'center';
}
}
} )
},
converterPriority: 'high'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it still necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

} );

conversion.for( 'upcast' )
// Support for backwards compatibility and pasting from other sources.
.attributeToAttribute( {
view: {
Expand All @@ -154,35 +153,6 @@ function enableAlignmentProperty( schema, conversion ) {
value: viewElement => viewElement.getAttribute( 'align' )
}
} );

conversion.for( 'downcast' ).add( dispatcher => dispatcher.on( 'attribute:alignment:table', ( evt, data, conversionApi ) => {
const { item, attributeNewValue } = data;
const { mapper, writer } = conversionApi;

const table = [ ...mapper.toViewElement( item ).getChildren() ].find( child => child.is( 'table' ) );

if ( !attributeNewValue ) {
writer.removeStyle( 'margin-left', table );
writer.removeStyle( 'margin-right', table );

return;
}

const styles = {
'margin-right': 'auto',
'margin-left': 'auto'
};

if ( attributeNewValue == 'left' ) {
styles[ 'margin-left' ] = '0';
}

if ( attributeNewValue == 'right' ) {
styles[ 'margin-right' ] = '0';
}

writer.setStyle( styles, table );
} ) );
}

// Enables conversion for an attribute for simple view-model mappings.
Expand Down
2 changes: 1 addition & 1 deletion src/tableproperties/tablepropertiesui.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
import { debounce } from 'lodash-es';

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

/**
Expand Down
9 changes: 6 additions & 3 deletions src/tableproperties/ui/tablepropertiesview.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,10 @@ export default class TablePropertiesView extends View {
* The value of the table alignment style.
*
* @observable
* @default 'center'
* @default ''
* @member #alignment
*/
alignment: 'center',
alignment: ''
} );

/**
Expand Down Expand Up @@ -574,7 +574,10 @@ export default class TablePropertiesView extends View {
icons: ALIGNMENT_ICONS,
toolbar: alignmentToolbar,
labels: this._alignmentLabels,
propertyName: 'alignment'
propertyName: 'alignment',
nameToValue: name => {
return name === 'center' ? '' : name;
}
} );

return {
Expand Down
9 changes: 5 additions & 4 deletions src/ui/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,22 +212,23 @@ export function getBorderStyleDefinitions( view ) {
* @param {module:ui/toolbar/toolbarview~ToolbarView} options.toolbar
* @param {Object.<String,String>} labels
* @param {String} propertyName
* @param {Function} [nameToValue] Optional function that maps button name to value. By default names are the same as values.
*/
export function fillToolbar( { view, icons, toolbar, labels, propertyName } ) {
export function fillToolbar( { view, icons, toolbar, labels, propertyName, nameToValue = name => name } ) {
for ( const name in labels ) {
const button = new ButtonView( view.locale );

button.set( {
label: labels[ name ],
icon: icons[ name ],
icon: icons[ name ]
} );

button.bind( 'isOn' ).to( view, propertyName, value => {
return value === name;
return value === nameToValue( name );
} );

button.on( 'execute', () => {
view[ propertyName ] = name;
view[ propertyName ] = nameToValue( name );
} );

toolbar.items.add( button );
Expand Down
Loading