Parcourir la source

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.
Maciej il y a 5 ans
Parent
commit
a2f010f377

+ 4 - 0
packages/ckeditor5-table/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 ) {

+ 20 - 50
packages/ckeditor5-table/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.

+ 1 - 1
packages/ckeditor5-table/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;
 
 /**

+ 6 - 3
packages/ckeditor5-table/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 {

+ 5 - 4
packages/ckeditor5-table/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 );

+ 180 - 0
packages/ckeditor5-table/tests/tablecellproperties/tablecellpropertiesediting.js

@@ -267,6 +267,26 @@ describe( 'table cell properties', () => {
 					tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );
 				} );
 
+				it( 'should consume converted item borderColor attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderColor:tableCell', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'borderColor', '#f00', tableCell ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderColor:tableCell', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'borderColor', '#f00', tableCell ) );
+
+					assertTableCellStyle( editor, '' );
+				} );
+
 				it( 'should downcast borderColor attribute (same top, right, bottom, left)', () => {
 					model.change( writer => writer.setAttribute( 'borderColor', {
 						top: '#f00',
@@ -294,6 +314,26 @@ describe( 'table cell properties', () => {
 					);
 				} );
 
+				it( 'should consume converted item borderStyle attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderStyle:tableCell', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'borderStyle', 'ridge', tableCell ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderStyle:tableCell', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'borderStyle', 'ridge', tableCell ) );
+
+					assertTableCellStyle( editor, '' );
+				} );
+
 				it( 'should downcast borderStyle attribute (same top, right, bottom, left)', () => {
 					model.change( writer => writer.setAttribute( 'borderStyle', {
 						top: 'solid',
@@ -316,6 +356,26 @@ describe( 'table cell properties', () => {
 					assertTableCellStyle( editor, 'border-bottom:dotted;border-left:dashed;border-right:ridge;border-top:solid;' );
 				} );
 
+				it( 'should consume converted item borderWidth attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderWidth:tableCell', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'borderWidth', '2px', tableCell ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderWidth:tableCell', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'borderWidth', '2px', tableCell ) );
+
+					assertTableCellStyle( editor, '' );
+				} );
+
 				it( 'should downcast borderWidth attribute (same top, right, bottom, left)', () => {
 					model.change( writer => writer.setAttribute( 'borderWidth', {
 						top: '42px',
@@ -559,6 +619,26 @@ describe( 'table cell properties', () => {
 					tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );
 				} );
 
+				it( 'should consume converted item backgroundColor attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:backgroundColor:tableCell', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'backgroundColor', '#f00', tableCell ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:backgroundColor:tableCell', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'backgroundColor', '#f00', tableCell ) );
+
+					assertTableCellStyle( editor, '' );
+				} );
+
 				it( 'should downcast backgroundColor', () => {
 					model.change( writer => writer.setAttribute( 'backgroundColor', '#f00', tableCell ) );
 
@@ -637,6 +717,26 @@ describe( 'table cell properties', () => {
 					tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );
 				} );
 
+				it( 'should consume converted item horizontalAlignment attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:horizontalAlignment:tableCell', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'horizontalAlignment', 'right', tableCell ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:horizontalAlignment:tableCell', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'horizontalAlignment', 'right', tableCell ) );
+
+					assertTableCellStyle( editor, '' );
+				} );
+
 				it( 'should downcast horizontalAlignment=left', () => {
 					model.change( writer => writer.setAttribute( 'horizontalAlignment', 'left', tableCell ) );
 
@@ -694,6 +794,26 @@ describe( 'table cell properties', () => {
 					tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );
 				} );
 
+				it( 'should consume converted item verticalAlignment attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:verticalAlignment:tableCell', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'verticalAlignment', 'top', tableCell ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:verticalAlignment:tableCell', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'verticalAlignment', 'top', tableCell ) );
+
+					assertTableCellStyle( editor, '' );
+				} );
+
 				it( 'should downcast verticalAlignment', () => {
 					model.change( writer => writer.setAttribute( 'verticalAlignment', 'middle', tableCell ) );
 
@@ -733,6 +853,26 @@ describe( 'table cell properties', () => {
 					tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );
 				} );
 
+				it( 'should consume converted item borderColor attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:padding:tableCell', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'padding', '1px', tableCell ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:padding:tableCell', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'padding', '1px', tableCell ) );
+
+					assertTableCellStyle( editor, '' );
+				} );
+
 				it( 'should downcast padding (same top, right, bottom, left)', () => {
 					model.change( writer => writer.setAttribute( 'padding', {
 						top: '2px',
@@ -807,6 +947,26 @@ describe( 'table cell properties', () => {
 					tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );
 				} );
 
+				it( 'should consume converted item width attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:width:tableCell', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'width', '40px', tableCell ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:width:tableCell', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'width', '40px', tableCell ) );
+
+					assertTableCellStyle( editor, '' );
+				} );
+
 				it( 'should downcast width attribute', () => {
 					model.change( writer => writer.setAttribute( 'width', '20px', tableCell ) );
 
@@ -868,6 +1028,26 @@ describe( 'table cell properties', () => {
 					tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );
 				} );
 
+				it( 'should consume converted item height attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:height:tableCell', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'height', '40px', tableCell ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:height:tableCell', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'height', '40px', tableCell ) );
+
+					assertTableCellStyle( editor, '' );
+				} );
+
 				it( 'should downcast height attribute', () => {
 					model.change( writer => writer.setAttribute( 'height', '20px', tableCell ) );
 

+ 4 - 4
packages/ckeditor5-table/tests/tableproperties/commands/tablealignmentcommand.js

@@ -102,7 +102,7 @@ describe( 'table properties', () => {
 
 						command.execute( { value: 'right' } );
 
-						assertTableStyle( editor, 'margin-left:auto;margin-right:0;' );
+						assertTableStyle( editor, null, 'float:right;' );
 					} );
 
 					it( 'should change selected table alignment to a passed value', () => {
@@ -110,7 +110,7 @@ describe( 'table properties', () => {
 
 						command.execute( { value: 'right' } );
 
-						assertTableStyle( editor, 'margin-left:auto;margin-right:0;' );
+						assertTableStyle( editor, null, 'float:right;' );
 					} );
 
 					it( 'should remove alignment from a selected table if no value is passed', () => {
@@ -128,7 +128,7 @@ describe( 'table properties', () => {
 
 						command.execute( { value: 'right' } );
 
-						assertTableStyle( editor, 'margin-left:auto;margin-right:0;' );
+						assertTableStyle( editor, null, 'float:right;' );
 					} );
 
 					it( 'should change selected table alignment to a passed value', () => {
@@ -136,7 +136,7 @@ describe( 'table properties', () => {
 
 						command.execute( { value: 'right' } );
 
-						assertTableStyle( editor, 'margin-left:auto;margin-right:0;' );
+						assertTableStyle( editor, null, 'float:right;' );
 					} );
 
 					it( 'should remove alignment from a selected table if no value is passed', () => {

+ 63 - 0
packages/ckeditor5-table/tests/tableproperties/tablepropertiesediting-integration.js

@@ -0,0 +1,63 @@
+/**
+ * @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 VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+
+import TableEditing from '../../src/tableediting';
+import TablePropertiesEditing from '../../src/tableproperties/tablepropertiesediting';
+
+import AlignmentEditing from '@ckeditor/ckeditor5-alignment/src/alignmentediting';
+import { assertTableStyle } from '../_utils/utils';
+
+describe( 'table properties', () => {
+	describe( 'TablePropertiesEditing integration', () => {
+		let editor, model;
+
+		afterEach( async () => {
+			await editor.destroy();
+		} );
+
+		describe( 'Alignment', () => {
+			let table;
+
+			beforeEach( async () => {
+				editor = await createEditorWithAdditionalPlugins( [ AlignmentEditing ] );
+
+				model = editor.model;
+
+				table = createEmptyTable();
+			} );
+
+			it( 'should properly downcast table with Alignment plugin enabled', () => {
+				model.change( writer => writer.setAttribute( 'alignment', 'right', table ) );
+
+				assertTableStyle( editor, null, 'float:right;' );
+			} );
+		} );
+
+		function createEmptyTable() {
+			setModelData(
+				model,
+				'<table headingRows="0" headingColumns="0">' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph>foo</paragraph>' +
+						'</tableCell>' +
+					'</tableRow>' +
+				'</table>'
+			);
+
+			return model.document.getRoot().getNodeByPath( [ 0 ] );
+		}
+	} );
+
+	function createEditorWithAdditionalPlugins( plugins ) {
+		return VirtualTestEditor.create( {
+			plugins: [ ...plugins, TablePropertiesEditing, Paragraph, TableEditing ]
+		} );
+	}
+} );

+ 179 - 47
packages/ckeditor5-table/tests/tableproperties/tablepropertiesediting.js

@@ -219,6 +219,26 @@ describe( 'table properties', () => {
 					table = createEmptyTable();
 				} );
 
+				it( 'should consume converted item borderColor attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderColor:table', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'borderColor', '#f00', table ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderColor:table', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'borderColor', '#f00', table ) );
+
+					assertTableStyle( editor, '' );
+				} );
+
 				it( 'should downcast borderColor attribute (same top, right, bottom, left)', () => {
 					model.change( writer => writer.setAttribute( 'borderColor', {
 						top: '#f00',
@@ -246,6 +266,26 @@ describe( 'table properties', () => {
 					);
 				} );
 
+				it( 'should consume converted item borderStyle attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderStyle:table', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'borderStyle', 'solid', table ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderStyle:table', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'borderStyle', 'solid', table ) );
+
+					assertTableStyle( editor, '' );
+				} );
+
 				it( 'should downcast borderStyle attribute (same top, right, bottom, left)', () => {
 					model.change( writer => writer.setAttribute( 'borderStyle', {
 						top: 'solid',
@@ -268,6 +308,26 @@ describe( 'table properties', () => {
 					assertTableStyle( editor, 'border-bottom:dotted;border-left:dashed;border-right:ridge;border-top:solid;' );
 				} );
 
+				it( 'should consume converted item borderWidth attribute', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderWidth:table', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'borderWidth', '2px', table ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:borderWidth:table', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'borderWidth', '2px', table ) );
+
+					assertTableStyle( editor, '' );
+				} );
+
 				it( 'should downcast borderWidth attribute (same top, right, bottom, left)', () => {
 					model.change( writer => writer.setAttribute( 'borderWidth', {
 						top: '42px',
@@ -508,6 +568,26 @@ describe( 'table properties', () => {
 					table = createEmptyTable();
 				} );
 
+				it( 'should consume converted item', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:backgroundColor:table', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'backgroundColor', '#f00', table ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:backgroundColor:table', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'backgroundColor', '#f00', table ) );
+
+					assertTableStyle( editor, '' );
+				} );
+
 				it( 'should downcast backgroundColor', () => {
 					model.change( writer => writer.setAttribute( 'backgroundColor', '#f00', table ) );
 
@@ -562,6 +642,26 @@ describe( 'table properties', () => {
 					table = createEmptyTable();
 				} );
 
+				it( 'should consume converted item', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:width:table', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'width', '400px', table ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:width:table', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'width', '400px', table ) );
+
+					assertTableStyle( editor, '' );
+				} );
+
 				it( 'should downcast width', () => {
 					model.change( writer => writer.setAttribute( 'width', '1337px', table ) );
 
@@ -639,6 +739,26 @@ describe( 'table properties', () => {
 
 					assertTableStyle( editor, null, 'height:1410em;' );
 				} );
+
+				it( 'should consume converted item', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:height:table', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
+					model.change( writer => writer.setAttribute( 'height', '400px', table ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:height:table', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'high' } ) );
+
+					model.change( writer => writer.setAttribute( 'height', '400px', table ) );
+
+					assertTableStyle( editor, '' );
+				} );
 			} );
 		} );
 
@@ -648,48 +768,20 @@ describe( 'table properties', () => {
 			} );
 
 			describe( 'upcast conversion', () => {
-				it( 'should upcast style="margin-left:auto;margin-right:0" to right value', () => {
-					editor.setData( '<table style="margin-left:auto;margin-right:0"><tr><td>foo</td></tr></table>' );
+				it( 'should upcast style="float:right" to right value', () => {
+					editor.setData( '<table style="float:right"><tr><td>foo</td></tr></table>' );
 					const table = model.document.getRoot().getNodeByPath( [ 0 ] );
 
 					expect( table.getAttribute( 'alignment' ) ).to.equal( 'right' );
 				} );
 
-				it( 'should upcast style="margin-left:0;margin-right:auto" to left value', () => {
-					editor.setData( '<table style="margin-left:0;margin-right:auto"><tr><td>foo</td></tr></table>' );
+				it( 'should upcast style="float:left;" to left value', () => {
+					editor.setData( '<table style="float:left;"><tr><td>foo</td></tr></table>' );
 					const table = model.document.getRoot().getNodeByPath( [ 0 ] );
 
 					expect( table.getAttribute( 'alignment' ) ).to.equal( 'left' );
 				} );
 
-				it( 'should upcast style="margin-left:auto;margin-right:auto" to center value', () => {
-					editor.setData( '<table style="margin-left:auto;margin-right:auto"><tr><td>foo</td></tr></table>' );
-					const table = model.document.getRoot().getNodeByPath( [ 0 ] );
-
-					expect( table.getAttribute( 'alignment' ) ).to.equal( 'center' );
-				} );
-
-				it( 'should upcast style="margin-left:auto;margin-right:0pt" to right value', () => {
-					editor.setData( '<table style="margin-left:auto;margin-right:0pt"><tr><td>foo</td></tr></table>' );
-					const table = model.document.getRoot().getNodeByPath( [ 0 ] );
-
-					expect( table.getAttribute( 'alignment' ) ).to.equal( 'right' );
-				} );
-
-				it( 'should upcast style="margin-left:auto;margin-right:0%" to right value', () => {
-					editor.setData( '<table style="margin-left:auto;margin-right:0%"><tr><td>foo</td></tr></table>' );
-					const table = model.document.getRoot().getNodeByPath( [ 0 ] );
-
-					expect( table.getAttribute( 'alignment' ) ).to.equal( 'right' );
-				} );
-
-				it( 'should not upcast style="margin-left:auto;margin-right:0.23pt" to right value', () => {
-					editor.setData( '<table style="margin-left:auto;margin-right:0.23pt"><tr><td>foo</td></tr></table>' );
-					const table = model.document.getRoot().getNodeByPath( [ 0 ] );
-
-					expect( table.hasAttribute( 'alignment' ) ).to.be.false;
-				} );
-
 				it( 'should upcast align=right attribute', () => {
 					editor.setData( '<table align="right"><tr><td>foo</td></tr></table>' );
 					const table = model.document.getRoot().getNodeByPath( [ 0 ] );
@@ -704,11 +796,11 @@ describe( 'table properties', () => {
 					expect( table.getAttribute( 'alignment' ) ).to.equal( 'left' );
 				} );
 
-				it( 'should upcast align=center attribute', () => {
+				it( 'should discard align=center attribute', () => {
 					editor.setData( '<table align="center"><tr><td>foo</td></tr></table>' );
 					const table = model.document.getRoot().getNodeByPath( [ 0 ] );
 
-					expect( table.getAttribute( 'alignment' ) ).to.equal( 'center' );
+					expect( table.getAttribute( 'alignment' ) ).to.be.undefined;
 				} );
 
 				it( 'should discard align=justify attribute', () => {
@@ -726,38 +818,78 @@ describe( 'table properties', () => {
 					table = createEmptyTable();
 				} );
 
-				it( 'should downcast right alignment', () => {
+				it( 'should consume converted item', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:alignment:table', ( evt, data, conversionApi ) => {
+							expect( conversionApi.consumable.consume( data.item, evt.name ) ).to.be.false;
+						} ) );
+
 					model.change( writer => writer.setAttribute( 'alignment', 'right', table ) );
+				} );
+
+				it( 'should be overridable', () => {
+					editor.conversion.for( 'downcast' )
+						.add( dispatcher => dispatcher.on( 'attribute:alignment:table', ( evt, data, conversionApi ) => {
+							conversionApi.consumable.consume( data.item, evt.name );
+						}, { priority: 'highest' } ) );
 
-					assertTableStyle( editor, 'margin-left:auto;margin-right:0;' );
+					model.change( writer => writer.setAttribute( 'alignment', 'right', table ) );
+
+					assertTableStyle( editor, '' );
 				} );
 
-				it( 'should downcast left alignment', () => {
+				it( 'should downcast "right" alignment', () => {
+					model.change( writer => writer.setAttribute( 'alignment', 'right', table ) );
+
+					assertTableStyle( editor, null, 'float:right;' );
+				} );
+
+				it( 'should downcast "left" alignment', () => {
 					model.change( writer => writer.setAttribute( 'alignment', 'left', table ) );
 
-					assertTableStyle( editor, 'margin-left:0;margin-right:auto;' );
+					assertTableStyle( editor, null, 'float:left;' );
 				} );
 
-				it( 'should downcast centered alignment', () => {
+				it( 'should not downcast "center" alignment', () => {
 					model.change( writer => writer.setAttribute( 'alignment', 'center', table ) );
 
-					assertTableStyle( editor, 'margin-left:auto;margin-right:auto;' );
+					assertTableStyle( editor, null, null );
 				} );
 
-				it( 'should downcast changed alignment', () => {
-					model.change( writer => writer.setAttribute( 'alignment', 'center', table ) );
+				it( 'should downcast changed alignment (left -> right)', () => {
+					model.change( writer => writer.setAttribute( 'alignment', 'left', table ) );
 
-					assertTableStyle( editor, 'margin-left:auto;margin-right:auto;' );
+					assertTableStyle( editor, null, 'float:left;' );
 
 					model.change( writer => writer.setAttribute( 'alignment', 'right', table ) );
 
-					assertTableStyle( editor, 'margin-left:auto;margin-right:0;' );
+					assertTableStyle( editor, null, 'float:right;' );
 				} );
 
-				it( 'should downcast removed alignment', () => {
-					model.change( writer => writer.setAttribute( 'alignment', 'center', table ) );
+				it( 'should downcast changed alignment (right -> left)', () => {
+					model.change( writer => writer.setAttribute( 'alignment', 'right', table ) );
+
+					assertTableStyle( editor, null, 'float:right;' );
+
+					model.change( writer => writer.setAttribute( 'alignment', 'left', table ) );
+
+					assertTableStyle( editor, null, 'float:left;' );
+				} );
+
+				it( 'should downcast removed alignment (from left)', () => {
+					model.change( writer => writer.setAttribute( 'alignment', 'left', table ) );
+
+					assertTableStyle( editor, null, 'float:left;' );
+
+					model.change( writer => writer.removeAttribute( 'alignment', table ) );
+
+					assertTableStyle( editor );
+				} );
+
+				it( 'should downcast removed alignment (from right)', () => {
+					model.change( writer => writer.setAttribute( 'alignment', 'right', table ) );
 
-					assertTableStyle( editor, 'margin-left:auto;margin-right:auto;' );
+					assertTableStyle( editor, null, 'float:right;' );
 
 					model.change( writer => writer.removeAttribute( 'alignment', table ) );
 

+ 1 - 1
packages/ckeditor5-table/tests/tableproperties/tablepropertiesui.js

@@ -512,7 +512,7 @@ describe( 'table properties', () => {
 						backgroundColor: '',
 						width: '',
 						height: '',
-						alignment: 'center'
+						alignment: ''
 					} );
 				} );
 			} );

+ 1 - 1
packages/ckeditor5-table/tests/tableproperties/ui/tablepropertiesview.js

@@ -50,7 +50,7 @@ describe( 'table properties', () => {
 					backgroundColor: '',
 					width: '',
 					height: '',
-					alignment: 'center'
+					alignment: ''
 				} );
 			} );
 

+ 28 - 9
packages/ckeditor5-table/tests/ui/utils.js

@@ -328,7 +328,7 @@ describe( 'UI Utils', () => {
 				false,
 				false,
 				true,
-				false,
+				false
 			] );
 		} );
 	} );
@@ -338,12 +338,14 @@ describe( 'UI Utils', () => {
 
 		const labels = {
 			first: 'Do something',
-			second: 'Do something else'
+			second: 'Do something else',
+			third: 'Be default'
 		};
 
 		const icons = {
-			first: '<svg viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg"><path /></svg>',
-			second: '<svg viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg"><path /></svg>'
+			first: '<svg viewBox="0 0 21 21" xmlns="http://www.w3.org/2000/svg"><path /></svg>',
+			second: '<svg viewBox="0 0 22 22" xmlns="http://www.w3.org/2000/svg"><path /></svg>',
+			third: '<svg viewBox="0 0 23 23" xmlns="http://www.w3.org/2000/svg"><path /></svg>'
 		};
 
 		beforeEach( () => {
@@ -354,7 +356,8 @@ describe( 'UI Utils', () => {
 
 			fillToolbar( {
 				view, toolbar, icons, labels,
-				propertyName: 'someProperty'
+				propertyName: 'someProperty',
+				nameToValue: name => name === 'third' ? '' : name
 			} );
 		} );
 
@@ -363,33 +366,45 @@ describe( 'UI Utils', () => {
 		} );
 
 		it( 'should create buttons', () => {
-			expect( toolbar.items ).to.have.length( 2 );
+			expect( toolbar.items ).to.have.length( 3 );
 			expect( toolbar.items.first ).to.be.instanceOf( ButtonView );
+			expect( toolbar.items.get( 1 ) ).to.be.instanceOf( ButtonView );
 			expect( toolbar.items.last ).to.be.instanceOf( ButtonView );
 		} );
 
 		it( 'should set button labels', () => {
 			expect( toolbar.items.first.label ).to.equal( 'Do something' );
-			expect( toolbar.items.last.label ).to.equal( 'Do something else' );
+			expect( toolbar.items.get( 1 ).label ).to.equal( 'Do something else' );
+			expect( toolbar.items.last.label ).to.equal( 'Be default' );
 		} );
 
 		it( 'should set button icons', () => {
 			expect( toolbar.items.first.icon ).to.equal( icons.first );
-			expect( toolbar.items.last.icon ).to.equal( icons.second );
+			expect( toolbar.items.get( 1 ).icon ).to.equal( icons.second );
+			expect( toolbar.items.last.icon ).to.equal( icons.third );
 		} );
 
 		it( 'should bind button #isOn to an observable property', () => {
 			expect( toolbar.items.first.isOn ).to.be.false;
+			expect( toolbar.items.get( 1 ).isOn ).to.be.false;
 			expect( toolbar.items.last.isOn ).to.be.false;
 
 			view.someProperty = 'first';
 
 			expect( toolbar.items.first.isOn ).to.be.true;
+			expect( toolbar.items.get( 1 ).isOn ).to.be.false;
 			expect( toolbar.items.last.isOn ).to.be.false;
 
 			view.someProperty = 'second';
 
 			expect( toolbar.items.first.isOn ).to.be.false;
+			expect( toolbar.items.get( 1 ).isOn ).to.be.true;
+			expect( toolbar.items.last.isOn ).to.be.false;
+
+			view.someProperty = '';
+
+			expect( toolbar.items.first.isOn ).to.be.false;
+			expect( toolbar.items.get( 1 ).isOn ).to.be.false;
 			expect( toolbar.items.last.isOn ).to.be.true;
 		} );
 
@@ -398,9 +413,13 @@ describe( 'UI Utils', () => {
 
 			expect( view.someProperty ).to.equal( 'first' );
 
-			toolbar.items.last.fire( 'execute' );
+			toolbar.items.get( 1 ).fire( 'execute' );
 
 			expect( view.someProperty ).to.equal( 'second' );
+
+			toolbar.items.last.fire( 'execute' );
+
+			expect( view.someProperty ).to.equal( '' );
 		} );
 	} );
 } );