From 571d358078bd7a1bf34142b0055bac5e5e12bd68 Mon Sep 17 00:00:00 2001 From: panr Date: Mon, 2 Mar 2020 10:51:19 +0100 Subject: [PATCH 1/8] Set a proper order of horizontal alignment labels for both LTR and RTL modes --- .../ui/tablecellpropertiesview.js | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js index 222281cc..011ad669 100644 --- a/src/tablecellproperties/ui/tablecellpropertiesview.js +++ b/src/tablecellproperties/ui/tablecellpropertiesview.js @@ -695,7 +695,8 @@ export default class TableCellPropertiesView extends View { labels: this._horizontalAlignmentLabels, propertyName: 'horizontalAlignment', nameToValue: name => { - return name === 'left' ? '' : name; + const isRTLContent = this.locale.contentLanguageDirection === 'rtl'; + return name === ( isRTLContent ? 'right' : 'left' ) ? '' : name; } } ); @@ -783,12 +784,29 @@ export default class TableCellPropertiesView extends View { get _horizontalAlignmentLabels() { const t = this.t; - return { + const labels = { left: t( 'Align cell text to the left' ), center: t( 'Align cell text to the center' ), right: t( 'Align cell text to the right' ), justify: t( 'Justify cell text' ) }; + + // Set of labels which can be reversed to match proper {@link #uiLanguage editor UI language}. + const ltr = [ 'left', 'center', 'right' ]; + const labelsDirection = this.locale.uiLanguageDirection === 'rtl' ? ltr.reverse() : ltr; + + // Creates object with a proper order of labeles. + const orderedLabels = labelsDirection.reduce( ( acc, curr ) => { + return { + ...acc, + [ curr ]: labels[ curr ] + }; + }, {} ); + + // Appends other labels. + orderedLabels.justify = labels.justify; + + return orderedLabels; } /** From e4584c2b1d26347b9de405594720d56754645374 Mon Sep 17 00:00:00 2001 From: panr Date: Mon, 2 Mar 2020 10:53:03 +0100 Subject: [PATCH 2/8] Set a proper position for the colorinput dropdown for both LTR and RTL modes --- src/ui/colorinputview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/colorinputview.js b/src/ui/colorinputview.js index 66d52b25..ace6b754 100644 --- a/src/ui/colorinputview.js +++ b/src/ui/colorinputview.js @@ -175,7 +175,7 @@ export default class ColorInputView extends View { dropdown.buttonView.children.add( colorPreview ); - dropdown.panelPosition = 'sw'; + dropdown.panelPosition = locale.uiLanguageDirection === 'rtl' ? 'se' : 'sw'; dropdown.panelView.children.add( removeColorButton ); dropdown.panelView.children.add( colorGrid ); dropdown.bind( 'isEnabled' ).to( this, 'isReadOnly', value => !value ); From e9810125216ad903c975bbb2f2bf70513d51ccd9 Mon Sep 17 00:00:00 2001 From: panr Date: Mon, 2 Mar 2020 10:54:15 +0100 Subject: [PATCH 3/8] Add tableProperties and tableCellProperties plugins to table rtl.js test --- tests/manual/rtl.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/manual/rtl.js b/tests/manual/rtl.js index 7c91d944..3e10676c 100644 --- a/tests/manual/rtl.js +++ b/tests/manual/rtl.js @@ -8,18 +8,21 @@ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset'; +import TableProperties from '../../src/tableproperties'; +import TableCellProperties from '../../src/tablecellproperties'; + ClassicEditor .create( document.querySelector( '#editor' ), { - plugins: [ ArticlePluginSet ], + plugins: [ ArticlePluginSet, TableProperties, TableCellProperties ], language: { - ui: 'en', - content: 'ar' + ui: 'ar', + content: 'en' }, toolbar: [ 'heading', '|', 'insertTable', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' ], table: { - contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells' ], + contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells', 'tableProperties', 'tableCellProperties' ], tableToolbar: [ 'bold', 'italic' ] } } ) From 04e10e6d9c77e997f974ca3ed537366bf15a4910 Mon Sep 17 00:00:00 2001 From: panr Date: Mon, 2 Mar 2020 10:55:26 +0100 Subject: [PATCH 4/8] Add tests for checking if horizontal alignment lables are set in a proper order for both LTR and RTL modes --- .../ui/tablecellpropertiesview.js | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/tablecellproperties/ui/tablecellpropertiesview.js b/tests/tablecellproperties/ui/tablecellpropertiesview.js index 36004c31..17801898 100644 --- a/tests/tablecellproperties/ui/tablecellpropertiesview.js +++ b/tests/tablecellproperties/ui/tablecellpropertiesview.js @@ -474,7 +474,7 @@ describe( 'table cell properties', () => { expect( toolbar.ariaLabel ).to.equal( 'Horizontal text alignment toolbar' ); } ); - it( 'should bring alignment buttons', () => { + it( 'should bring alignment buttons in the right order in LTR (default)', () => { expect( toolbar.items.map( ( { label } ) => label ) ).to.have.ordered.members( [ 'Align cell text to the left', 'Align cell text to the center', @@ -487,6 +487,28 @@ describe( 'table cell properties', () => { ] ); } ); + it( 'should bring alignment buttons in the right order in RTL', () => { + // Creates its own local instances of locale, view and toolbar. + const locale = { + t: val => val, + uiLanguageDirection: 'rtl', + contentLanguageDirection: 'rtl' + }; + const view = new TableCellPropertiesView( locale, VIEW_OPTIONS ); + const toolbar = view.horizontalAlignmentToolbar; + + expect( toolbar.items.map( ( { label } ) => label ) ).to.have.ordered.members( [ + 'Align cell text to the right', + 'Align cell text to the center', + 'Align cell text to the left', + 'Justify cell text' + ] ); + + expect( toolbar.items.map( ( { isOn } ) => isOn ) ).to.have.ordered.members( [ + true, false, false, false + ] ); + } ); + it( 'should change the #horizontalAlignment value', () => { toolbar.items.last.fire( 'execute' ); expect( view.horizontalAlignment ).to.equal( 'justify' ); From ea36b27e07307dbfb58fdf01e223b1aa594eca54 Mon Sep 17 00:00:00 2001 From: panr Date: Mon, 2 Mar 2020 10:57:24 +0100 Subject: [PATCH 5/8] Add tests checking if colorinput dropdown position is properly set in both LTR and RTL modes --- tests/ui/colorinputview.js | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/ui/colorinputview.js b/tests/ui/colorinputview.js index 57dd928e..599949ae 100644 --- a/tests/ui/colorinputview.js +++ b/tests/ui/colorinputview.js @@ -73,7 +73,6 @@ describe( 'ColorInputView', () => { it( 'should be created', () => { expect( view._dropdownView ).to.be.instanceOf( DropdownView ); expect( view._dropdownView.buttonView.element.classList.contains( 'ck-input-color__button' ) ).to.be.true; - expect( view._dropdownView.panelPosition ).to.equal( 'sw' ); } ); it( 'should bind #isEnabled to the view\'s #isReadOnly', () => { @@ -107,6 +106,30 @@ describe( 'ColorInputView', () => { it( 'should have the remove color button', () => { expect( view._dropdownView.panelView.children.first ).to.be.instanceOf( ButtonView ); } ); + + describe( 'position', () => { + it( 'should be SouthWest in LTR', () => { + locale.uiLanguageDirection = 'ltr'; + view = new ColorInputView( locale, { + colorDefinitions: DEFAULT_COLORS, + columns: 5 + } ); + view.render(); + + expect( view._dropdownView.panelPosition ).to.equal( 'sw' ); + } ); + + it( 'should be SouthEast in RTL', () => { + locale.uiLanguageDirection = 'rtl'; + view = new ColorInputView( locale, { + colorDefinitions: DEFAULT_COLORS, + columns: 5 + } ); + view.render(); + + expect( view._dropdownView.panelPosition ).to.equal( 'se' ); + } ); + } ); } ); describe( 'color grid', () => { From 5300d4a1c928f796328fd1af4e1e7b19ecbddc29 Mon Sep 17 00:00:00 2001 From: panr Date: Fri, 6 Mar 2020 15:42:59 +0100 Subject: [PATCH 6/8] Simplify code --- .../ui/tablecellpropertiesview.js | 37 +++++++------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js index 011ad669..cdebd30b 100644 --- a/src/tablecellproperties/ui/tablecellpropertiesview.js +++ b/src/tablecellproperties/ui/tablecellpropertiesview.js @@ -682,6 +682,7 @@ export default class TableCellPropertiesView extends View { // -- Horizontal --------------------------------------------------- const horizontalAlignmentToolbar = new ToolbarView( locale ); + const isRTLContent = this.locale.contentLanguageDirection === 'rtl'; horizontalAlignmentToolbar.set( { isCompact: true, @@ -695,7 +696,6 @@ export default class TableCellPropertiesView extends View { labels: this._horizontalAlignmentLabels, propertyName: 'horizontalAlignment', nameToValue: name => { - const isRTLContent = this.locale.contentLanguageDirection === 'rtl'; return name === ( isRTLContent ? 'right' : 'left' ) ? '' : name; } } ); @@ -782,31 +782,20 @@ export default class TableCellPropertiesView extends View { * @type {Object.} */ get _horizontalAlignmentLabels() { + const locale = this.locale; const t = this.t; - const labels = { - left: t( 'Align cell text to the left' ), - center: t( 'Align cell text to the center' ), - right: t( 'Align cell text to the right' ), - justify: t( 'Justify cell text' ) - }; - - // Set of labels which can be reversed to match proper {@link #uiLanguage editor UI language}. - const ltr = [ 'left', 'center', 'right' ]; - const labelsDirection = this.locale.uiLanguageDirection === 'rtl' ? ltr.reverse() : ltr; - - // Creates object with a proper order of labeles. - const orderedLabels = labelsDirection.reduce( ( acc, curr ) => { - return { - ...acc, - [ curr ]: labels[ curr ] - }; - }, {} ); - - // Appends other labels. - orderedLabels.justify = labels.justify; - - return orderedLabels; + const left = t( 'Align cell text to the left' ); + const center = t( 'Align cell text to the center' ); + const right = t( 'Align cell text to the right' ); + const justify = t( 'Justify cell text' ); + + // Returns object with a proper order of labeles. + if ( locale.uiLanguageDirection === 'rtl' ) { + return { right, center, left, justify }; + } else { + return { left, center, right, justify }; + } } /** From 7fc6a8b212da34e4b6d7183e4612f3a88152daeb Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 9 Mar 2020 16:48:17 +0100 Subject: [PATCH 7/8] Mirrored the order of TableProperties alignment buttons when the UI is RTL. --- .../ui/tablecellpropertiesview.js | 2 +- src/tableproperties/ui/tablepropertiesview.js | 16 ++++++++---- .../ui/tablecellpropertiesview.js | 6 +++-- .../tableproperties/ui/tablepropertiesview.js | 25 ++++++++++++++++++- 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js index cdebd30b..c4463c1e 100644 --- a/src/tablecellproperties/ui/tablecellpropertiesview.js +++ b/src/tablecellproperties/ui/tablecellpropertiesview.js @@ -790,7 +790,7 @@ export default class TableCellPropertiesView extends View { const right = t( 'Align cell text to the right' ); const justify = t( 'Justify cell text' ); - // Returns object with a proper order of labeles. + // Returns object with a proper order of labels. if ( locale.uiLanguageDirection === 'rtl' ) { return { right, center, left, justify }; } else { diff --git a/src/tableproperties/ui/tablepropertiesview.js b/src/tableproperties/ui/tablepropertiesview.js index fb412cba..20642640 100644 --- a/src/tableproperties/ui/tablepropertiesview.js +++ b/src/tableproperties/ui/tablepropertiesview.js @@ -681,13 +681,19 @@ export default class TablePropertiesView extends View { * @type {Object.} */ get _alignmentLabels() { + const locale = this.locale; const t = this.t; - return { - left: t( 'Align table to the left' ), - center: t( 'Center table' ), - right: t( 'Align table to the right' ) - }; + const left = t( 'Align table to the left' ); + const center = t( 'Center table' ); + const right = t( 'Align table to the right' ); + + // Returns object with a proper order of labels. + if ( locale.uiLanguageDirection === 'rtl' ) { + return { right, center, left }; + } else { + return { left, center, right }; + } } } diff --git a/tests/tablecellproperties/ui/tablecellpropertiesview.js b/tests/tablecellproperties/ui/tablecellpropertiesview.js index 17801898..e5ef71c9 100644 --- a/tests/tablecellproperties/ui/tablecellpropertiesview.js +++ b/tests/tablecellproperties/ui/tablecellpropertiesview.js @@ -474,7 +474,7 @@ describe( 'table cell properties', () => { expect( toolbar.ariaLabel ).to.equal( 'Horizontal text alignment toolbar' ); } ); - it( 'should bring alignment buttons in the right order in LTR (default)', () => { + it( 'should bring alignment buttons in the right order (left-to-right UI)', () => { expect( toolbar.items.map( ( { label } ) => label ) ).to.have.ordered.members( [ 'Align cell text to the left', 'Align cell text to the center', @@ -487,7 +487,7 @@ describe( 'table cell properties', () => { ] ); } ); - it( 'should bring alignment buttons in the right order in RTL', () => { + it( 'should bring alignment buttons in the right order (right-to-left UI)', () => { // Creates its own local instances of locale, view and toolbar. const locale = { t: val => val, @@ -507,6 +507,8 @@ describe( 'table cell properties', () => { expect( toolbar.items.map( ( { isOn } ) => isOn ) ).to.have.ordered.members( [ true, false, false, false ] ); + + view.destroy(); } ); it( 'should change the #horizontalAlignment value', () => { diff --git a/tests/tableproperties/ui/tablepropertiesview.js b/tests/tableproperties/ui/tablepropertiesview.js index 64a107cf..9c99aaba 100644 --- a/tests/tableproperties/ui/tablepropertiesview.js +++ b/tests/tableproperties/ui/tablepropertiesview.js @@ -439,7 +439,7 @@ describe( 'table properties', () => { expect( toolbar.ariaLabel ).to.equal( 'Table alignment toolbar' ); } ); - it( 'should bring alignment buttons', () => { + it( 'should bring alignment buttons in the right order (left-to-right UI)', () => { expect( toolbar.items.map( ( { label } ) => label ) ).to.have.ordered.members( [ 'Align table to the left', 'Center table', @@ -451,6 +451,29 @@ describe( 'table properties', () => { ] ); } ); + it( 'should bring alignment buttons in the right order (right-to-left UI)', () => { + // Creates its own local instances of locale, view and toolbar. + const locale = { + t: val => val, + uiLanguageDirection: 'rtl', + contentLanguageDirection: 'rtl' + }; + const view = new TablePropertiesView( locale, VIEW_OPTIONS ); + const toolbar = view.alignmentToolbar; + + expect( toolbar.items.map( ( { label } ) => label ) ).to.have.ordered.members( [ + 'Align table to the right', + 'Center table', + 'Align table to the left' + ] ); + + expect( toolbar.items.map( ( { isOn } ) => isOn ) ).to.have.ordered.members( [ + false, true, false + ] ); + + view.destroy(); + } ); + it( 'should change the #horizontalAlignment value', () => { toolbar.items.last.fire( 'execute' ); expect( view.alignment ).to.equal( 'right' ); From b17302992919c07b04f2a4b243dc99036b202765 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 9 Mar 2020 16:49:43 +0100 Subject: [PATCH 8/8] Code refactoring. --- src/tablecellproperties/ui/tablecellpropertiesview.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tablecellproperties/ui/tablecellpropertiesview.js b/src/tablecellproperties/ui/tablecellpropertiesview.js index c4463c1e..6444f716 100644 --- a/src/tablecellproperties/ui/tablecellpropertiesview.js +++ b/src/tablecellproperties/ui/tablecellpropertiesview.js @@ -682,7 +682,7 @@ export default class TableCellPropertiesView extends View { // -- Horizontal --------------------------------------------------- const horizontalAlignmentToolbar = new ToolbarView( locale ); - const isRTLContent = this.locale.contentLanguageDirection === 'rtl'; + const isContentRTL = this.locale.contentLanguageDirection === 'rtl'; horizontalAlignmentToolbar.set( { isCompact: true, @@ -696,7 +696,7 @@ export default class TableCellPropertiesView extends View { labels: this._horizontalAlignmentLabels, propertyName: 'horizontalAlignment', nameToValue: name => { - return name === ( isRTLContent ? 'right' : 'left' ) ? '' : name; + return name === ( isContentRTL ? 'right' : 'left' ) ? '' : name; } } );