Merge pull request #234 from ckeditor/i/6197

Other: Convert `alignment` table property to a `<figure>` element. Closes ckeditor/ckeditor5#6197. Closes ckeditor/ckeditor5#6179.

Author: jodator

src/converters/tableproperties.js

@@ -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 ) {

src/tableproperties/tablepropertiesediting.js

@@ -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.
@@ -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'
+		} );
+
+	conversion.for( 'upcast' )
 		// Support for backwards compatibility and pasting from other sources.
 		.attributeToAttribute( {
 			view: {
@@ -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.

src/tableproperties/tablepropertiesui.js

@@ -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;
 
 /**

src/tableproperties/ui/tablepropertiesview.js

@@ -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: ''
 		} );
 
 		/**
@@ -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 {

src/ui/utils.js

@@ -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 );