From bc1ffcb01410c8eff0fb23c683f9ec298f6665e2 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 23 Mar 2020 19:41:53 +0100 Subject: [PATCH 1/3] Fixed horizontal alignment of table cell in RTL content. --- .../tablecellpropertiesediting.js | 32 ++--- .../tablecellpropertiesediting.js | 111 ++++++++++++++++++ 2 files changed, 124 insertions(+), 19 deletions(-) diff --git a/src/tablecellproperties/tablecellpropertiesediting.js b/src/tablecellproperties/tablecellpropertiesediting.js index 6f0344b3..a6fffcf7 100644 --- a/src/tablecellproperties/tablecellpropertiesediting.js +++ b/src/tablecellproperties/tablecellpropertiesediting.js @@ -69,6 +69,7 @@ export default class TableCellPropertiesEditing extends Plugin { const editor = this.editor; const schema = editor.model.schema; const conversion = editor.conversion; + const locale = editor.locale; editor.data.addStyleProcessorRules( addBorderRules ); enableBorderProperties( schema, conversion ); @@ -76,7 +77,7 @@ export default class TableCellPropertiesEditing extends Plugin { editor.commands.add( 'tableCellBorderColor', new TableCellBorderColorCommand( editor ) ); editor.commands.add( 'tableCellBorderWidth', new TableCellBorderWidthCommand( editor ) ); - enableHorizontalAlignmentProperty( schema, conversion ); + enableHorizontalAlignmentProperty( schema, conversion, locale ); editor.commands.add( 'tableCellHorizontalAlignment', new TableCellHorizontalAlignmentCommand( editor ) ); enableProperty( schema, conversion, 'width', 'width' ); @@ -117,37 +118,30 @@ function enableBorderProperties( schema, conversion ) { // // @param {module:engine/model/schema~Schema} schema // @param {module:engine/conversion/conversion~Conversion} conversion -function enableHorizontalAlignmentProperty( schema, conversion ) { +// @param {module:utils/locale~Locale} locale The {@link module:core/editor/editor~Editor#locale} instance. +function enableHorizontalAlignmentProperty( schema, conversion, locale ) { schema.extend( 'tableCell', { allowAttributes: [ 'horizontalAlignment' ] } ); + const defaultOption = locale.contentLanguageDirection == 'rtl' ? 'right' : 'left'; + const options = [ 'left', 'right', 'center', 'justify' ].filter( option => option != defaultOption ); + conversion.attributeToAttribute( { model: { name: 'tableCell', key: 'horizontalAlignment', - values: [ 'right', 'center', 'justify' ] + values: options }, - view: { - right: { - key: 'style', - value: { - 'text-align': 'right' - } - }, - center: { + view: options.reduce( ( result, option ) => ( { + ...result, + [ option ]: { key: 'style', value: { - 'text-align': 'center' - } - }, - justify: { - key: 'style', - value: { - 'text-align': 'justify' + 'text-align': option } } - } + } ), {} ) } ); } diff --git a/tests/tablecellproperties/tablecellpropertiesediting.js b/tests/tablecellproperties/tablecellpropertiesediting.js index 3c33f108..7ed85377 100644 --- a/tests/tablecellproperties/tablecellpropertiesediting.js +++ b/tests/tablecellproperties/tablecellpropertiesediting.js @@ -1126,4 +1126,115 @@ describe( 'table cell properties', () => { } ); } ); } ); + + describe( 'TableCellPropertiesEditing for RTL language', () => { + let editor, model; + + beforeEach( async () => { + editor = await VirtualTestEditor.create( { + plugins: [ TableCellPropertiesEditing, Paragraph, TableEditing ], + language: 'ar' + } ); + + model = editor.model; + } ); + + afterEach( async () => { + await editor.destroy(); + } ); + + describe( 'horizontal alignment', () => { + describe( 'upcast conversion', () => { + it( 'should not upcast text-align:right style', () => { + editor.setData( '
foo
' ); + const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); + + expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.be.undefined; + } ); + + it( 'should upcast text-align:left style', () => { + editor.setData( '
foo
' ); + const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); + + expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.equal( 'left' ); + } ); + + it( 'should upcast text-align:center style', () => { + editor.setData( '
foo
' ); + const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); + + expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.equal( 'center' ); + } ); + + it( 'should upcast text-align:justify style', () => { + editor.setData( '
foo
' ); + const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); + + expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.equal( 'justify' ); + } ); + } ); + + describe( 'downcast conversion', () => { + let tableCell; + + beforeEach( () => { + setModelData( + model, + '' + + '' + + '' + + 'foo' + + '' + + '' + + '
' + ); + 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', 'center', 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', 'center', tableCell ) ); + + assertTableCellStyle( editor, '' ); + } ); + + it( 'should not downcast horizontalAlignment=right', () => { + model.change( writer => writer.setAttribute( 'horizontalAlignment', 'right', tableCell ) ); + + assertTableCellStyle( editor ); + } ); + + it( 'should downcast horizontalAlignment=left', () => { + model.change( writer => writer.setAttribute( 'horizontalAlignment', 'left', tableCell ) ); + + assertTableCellStyle( editor, 'text-align:left;' ); + } ); + + it( 'should downcast horizontalAlignment=center', () => { + model.change( writer => writer.setAttribute( 'horizontalAlignment', 'center', tableCell ) ); + + assertTableCellStyle( editor, 'text-align:center;' ); + } ); + + it( 'should downcast horizontalAlignment=justify', () => { + model.change( writer => writer.setAttribute( 'horizontalAlignment', 'justify', tableCell ) ); + + assertTableCellStyle( editor, 'text-align:justify;' ); + } ); + } ); + } ); + } ); } ); From 1429d55eb5d34c5fd161b4afcf7dadcb6e6063fb Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 26 Mar 2020 13:55:24 +0100 Subject: [PATCH 2/3] Code refactoring: Moved RTL content TableCellPropertiesEditing horizontal alignment tests to the right category. --- .../tablecellpropertiesediting.js | 232 +++++++++--------- 1 file changed, 120 insertions(+), 112 deletions(-) diff --git a/tests/tablecellproperties/tablecellpropertiesediting.js b/tests/tablecellproperties/tablecellpropertiesediting.js index 7ed85377..93fab54c 100644 --- a/tests/tablecellproperties/tablecellpropertiesediting.js +++ b/tests/tablecellproperties/tablecellpropertiesediting.js @@ -59,7 +59,7 @@ describe( 'table cell properties', () => { expect( editor.commands.get( 'tableCellHorizontalAlignment' ) ).to.be.instanceOf( TableCellHorizontalAlignmentCommand ); } ); - it( 'adds tableCellAlignment command', () => { + it( 'adds tableCellVerticalAlignment command', () => { expect( editor.commands.get( 'tableCellVerticalAlignment' ) ).to.be.instanceOf( TableCellVerticalAlignmentCommand ); } ); @@ -712,6 +712,51 @@ describe( 'table cell properties', () => { expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.equal( 'justify' ); } ); + + describe( 'for RTL content language', () => { + let editor, model; + + beforeEach( async () => { + editor = await VirtualTestEditor.create( { + plugins: [ TableCellPropertiesEditing, Paragraph, TableEditing ], + language: 'ar' + } ); + + model = editor.model; + } ); + + afterEach( async () => { + await editor.destroy(); + } ); + + it( 'should not upcast text-align:right style', () => { + editor.setData( '
foo
' ); + const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); + + expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.be.undefined; + } ); + + it( 'should upcast text-align:left style', () => { + editor.setData( '
foo
' ); + const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); + + expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.equal( 'left' ); + } ); + + it( 'should upcast text-align:center style', () => { + editor.setData( '
foo
' ); + const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); + + expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.equal( 'center' ); + } ); + + it( 'should upcast text-align:justify style', () => { + editor.setData( '
foo
' ); + const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); + + expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.equal( 'justify' ); + } ); + } ); } ); describe( 'downcast conversion', () => { @@ -774,6 +819,80 @@ describe( 'table cell properties', () => { assertTableCellStyle( editor, 'text-align:justify;' ); } ); + + describe( 'for RTL content language', () => { + let editor, model; + + beforeEach( async () => { + editor = await VirtualTestEditor.create( { + plugins: [ TableCellPropertiesEditing, Paragraph, TableEditing ], + language: 'ar' + } ); + + model = editor.model; + + setModelData( + model, + '' + + '' + + '' + + 'foo' + + '' + + '' + + '
' + ); + + tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); + } ); + + afterEach( async () => { + await editor.destroy(); + } ); + + it( 'should consume converted item\'s 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', 'center', 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', 'center', tableCell ) ); + + assertTableCellStyle( editor, '' ); + } ); + + it( 'should not downcast horizontalAlignment=right', () => { + model.change( writer => writer.setAttribute( 'horizontalAlignment', 'right', tableCell ) ); + + assertTableCellStyle( editor ); + } ); + + it( 'should downcast horizontalAlignment=left', () => { + model.change( writer => writer.setAttribute( 'horizontalAlignment', 'left', tableCell ) ); + + assertTableCellStyle( editor, 'text-align:left;' ); + } ); + + it( 'should downcast horizontalAlignment=center', () => { + model.change( writer => writer.setAttribute( 'horizontalAlignment', 'center', tableCell ) ); + + assertTableCellStyle( editor, 'text-align:center;' ); + } ); + + it( 'should downcast horizontalAlignment=justify', () => { + model.change( writer => writer.setAttribute( 'horizontalAlignment', 'justify', tableCell ) ); + + assertTableCellStyle( editor, 'text-align:justify;' ); + } ); + } ); } ); } ); @@ -1126,115 +1245,4 @@ describe( 'table cell properties', () => { } ); } ); } ); - - describe( 'TableCellPropertiesEditing for RTL language', () => { - let editor, model; - - beforeEach( async () => { - editor = await VirtualTestEditor.create( { - plugins: [ TableCellPropertiesEditing, Paragraph, TableEditing ], - language: 'ar' - } ); - - model = editor.model; - } ); - - afterEach( async () => { - await editor.destroy(); - } ); - - describe( 'horizontal alignment', () => { - describe( 'upcast conversion', () => { - it( 'should not upcast text-align:right style', () => { - editor.setData( '
foo
' ); - const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); - - expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.be.undefined; - } ); - - it( 'should upcast text-align:left style', () => { - editor.setData( '
foo
' ); - const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); - - expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.equal( 'left' ); - } ); - - it( 'should upcast text-align:center style', () => { - editor.setData( '
foo
' ); - const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); - - expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.equal( 'center' ); - } ); - - it( 'should upcast text-align:justify style', () => { - editor.setData( '
foo
' ); - const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] ); - - expect( tableCell.getAttribute( 'horizontalAlignment' ) ).to.equal( 'justify' ); - } ); - } ); - - describe( 'downcast conversion', () => { - let tableCell; - - beforeEach( () => { - setModelData( - model, - '' + - '' + - '' + - 'foo' + - '' + - '' + - '
' - ); - 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', 'center', 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', 'center', tableCell ) ); - - assertTableCellStyle( editor, '' ); - } ); - - it( 'should not downcast horizontalAlignment=right', () => { - model.change( writer => writer.setAttribute( 'horizontalAlignment', 'right', tableCell ) ); - - assertTableCellStyle( editor ); - } ); - - it( 'should downcast horizontalAlignment=left', () => { - model.change( writer => writer.setAttribute( 'horizontalAlignment', 'left', tableCell ) ); - - assertTableCellStyle( editor, 'text-align:left;' ); - } ); - - it( 'should downcast horizontalAlignment=center', () => { - model.change( writer => writer.setAttribute( 'horizontalAlignment', 'center', tableCell ) ); - - assertTableCellStyle( editor, 'text-align:center;' ); - } ); - - it( 'should downcast horizontalAlignment=justify', () => { - model.change( writer => writer.setAttribute( 'horizontalAlignment', 'justify', tableCell ) ); - - assertTableCellStyle( editor, 'text-align:justify;' ); - } ); - } ); - } ); - } ); } ); From 956eb3ee2456bacef7845e3be52da6c4f31ce84b Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 26 Mar 2020 13:58:50 +0100 Subject: [PATCH 3/3] Code refactoring: Minor code simplification in the horizontal alignment conversion code. --- src/tablecellproperties/tablecellpropertiesediting.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tablecellproperties/tablecellpropertiesediting.js b/src/tablecellproperties/tablecellpropertiesediting.js index a6fffcf7..3e1d6ac9 100644 --- a/src/tablecellproperties/tablecellpropertiesediting.js +++ b/src/tablecellproperties/tablecellpropertiesediting.js @@ -124,8 +124,7 @@ function enableHorizontalAlignmentProperty( schema, conversion, locale ) { allowAttributes: [ 'horizontalAlignment' ] } ); - const defaultOption = locale.contentLanguageDirection == 'rtl' ? 'right' : 'left'; - const options = [ 'left', 'right', 'center', 'justify' ].filter( option => option != defaultOption ); + const options = [ locale.contentLanguageDirection == 'rtl' ? 'left' : 'right', 'center', 'justify' ]; conversion.attributeToAttribute( { model: {