From f41b964c2787c1f7704c8e53ebb47b08678ed705 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 19 Sep 2019 17:02:31 +0200 Subject: [PATCH 1/2] Tests: Added a manual test for table in an RTL content. --- tests/manual/rtl.html | 30 ++++++++++++++++++++++++++++++ tests/manual/rtl.js | 31 +++++++++++++++++++++++++++++++ tests/manual/rtl.md | 21 +++++++++++++++++++++ 3 files changed, 82 insertions(+) create mode 100644 tests/manual/rtl.html create mode 100644 tests/manual/rtl.js create mode 100644 tests/manual/rtl.md diff --git a/tests/manual/rtl.html b/tests/manual/rtl.html new file mode 100644 index 00000000..0c8435e1 --- /dev/null +++ b/tests/manual/rtl.html @@ -0,0 +1,30 @@ + + +
+
+ + + + + + + + + + + + + + + + + + +
123
456
789
+
+
diff --git a/tests/manual/rtl.js b/tests/manual/rtl.js new file mode 100644 index 00000000..a7141a43 --- /dev/null +++ b/tests/manual/rtl.js @@ -0,0 +1,31 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* globals console, window, document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; +import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset'; + +ClassicEditor + .create( document.querySelector( '#editor' ), { + plugins: [ ArticlePluginSet ], + language: { + ui: 'en', + content: 'ar' + }, + toolbar: [ + 'heading', '|', 'insertTable', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' + ], + table: { + contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells' ], + tableToolbar: [ 'bold', 'italic' ] + } + } ) + .then( editor => { + window.editor = editor; + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/tests/manual/rtl.md b/tests/manual/rtl.md new file mode 100644 index 00000000..0a219633 --- /dev/null +++ b/tests/manual/rtl.md @@ -0,0 +1,21 @@ +# Tables and RTL (right–to–left) content + +The editor has English UI and Arabic content. In Arabic, the table is mirrored so some features of the editor must work differently to provide the right UX. + +## Adding columns + +1. Focus the "5" cell. +2. Using the toolbar menu, add column to the right. +3. The column should visually appear on the right–hand side of the "5" cell. +4. Focus the "5" cell. +5. Using the toolbar menu, add column to the left. +3. The column should visually appear on the left–hand side of the "5" cell. + +## Merging cells + +1. Focus the "5" cell. +2. Using the toolbar menu, merge cell to the right. +3. "5" and "4" cells should be merged. +4. Focus the "5" cell. +5. Using the toolbar menu, merge cell to the left. +6. "5" and "6" cells should be merged. From 2638a0c80dc7ba081ce6dd9743c5e9f3ec397fdb Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 19 Sep 2019 17:16:34 +0200 Subject: [PATCH 2/2] =?UTF-8?q?Fix:=20Column=20insertion=20and=20cell=20me?= =?UTF-8?q?rging=20buttons=20should=20work=20correctly=20when=20the=20edit?= =?UTF-8?q?or=20content=20is=20right=E2=80=93to=E2=80=93left=20(RTL).=20Cl?= =?UTF-8?q?oses=20#200.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/tableui.js | 10 ++++---- tests/tableui.js | 62 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/src/tableui.js b/src/tableui.js index 20ba85da..537d947b 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -38,6 +38,8 @@ export default class TableUI extends Plugin { init() { const editor = this.editor; const t = this.editor.t; + const contentLanguageDirection = editor.locale.contentLanguageDirection; + const isContentLtr = contentLanguageDirection === 'ltr'; editor.ui.componentFactory.add( 'insertTable', locale => { const command = editor.commands.get( 'insertTable' ); @@ -86,14 +88,14 @@ export default class TableUI extends Plugin { { type: 'button', model: { - commandName: 'insertTableColumnLeft', + commandName: isContentLtr ? 'insertTableColumnLeft' : 'insertTableColumnRight', label: t( 'Insert column left' ) } }, { type: 'button', model: { - commandName: 'insertTableColumnRight', + commandName: isContentLtr ? 'insertTableColumnRight' : 'insertTableColumnLeft', label: t( 'Insert column right' ) } }, @@ -158,7 +160,7 @@ export default class TableUI extends Plugin { { type: 'button', model: { - commandName: 'mergeTableCellRight', + commandName: isContentLtr ? 'mergeTableCellRight' : 'mergeTableCellLeft', label: t( 'Merge cell right' ) } }, @@ -172,7 +174,7 @@ export default class TableUI extends Plugin { { type: 'button', model: { - commandName: 'mergeTableCellLeft', + commandName: isContentLtr ? 'mergeTableCellLeft' : 'mergeTableCellRight', label: t( 'Merge cell left' ) } }, diff --git a/tests/tableui.js b/tests/tableui.js index 6b91ca4f..4cb2b0c5 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -229,7 +229,7 @@ describe( 'TableUI', () => { expect( labels ).to.deep.equal( [ 'Header column', '|', 'Insert column left', 'Insert column right', 'Delete column' ] ); } ); - it( 'should bind items in panel to proper commands', () => { + it( 'should bind items in panel to proper commands (LTR content)', () => { const items = dropdown.listView.items; const setColumnHeaderCommand = editor.commands.get( 'setTableColumnHeader' ); @@ -266,6 +266,35 @@ describe( 'TableUI', () => { expect( dropdown.buttonView.isEnabled ).to.be.false; } ); + it( 'should bind items in panel to proper commands (RTL content)', () => { + const element = document.createElement( 'div' ); + + document.body.appendChild( element ); + + return ClassicTestEditor + .create( element, { + language: { + ui: 'en', + content: 'ar' + }, + plugins: [ TableEditing, TableUI ] + } ) + .then( editor => { + const dropdown = editor.ui.componentFactory.create( 'tableColumn' ); + const items = dropdown.listView.items; + + expect( items.get( 2 ).children.first.label ).to.equal( 'Insert column left' ); + expect( items.get( 2 ).children.first.commandName ).to.equal( 'insertTableColumnRight' ); + + expect( items.get( 3 ).children.first.label ).to.equal( 'Insert column right' ); + expect( items.get( 3 ).children.first.commandName ).to.equal( 'insertTableColumnLeft' ); + + element.remove(); + + return editor.destroy(); + } ); + } ); + it( 'should focus view after command execution', () => { const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' ); @@ -336,7 +365,7 @@ describe( 'TableUI', () => { ] ); } ); - it( 'should bind items in panel to proper commands', () => { + it( 'should bind items in panel to proper commands (LTR content)', () => { const items = dropdown.listView.items; const mergeCellUpCommand = editor.commands.get( 'mergeTableCellUp' ); @@ -386,6 +415,35 @@ describe( 'TableUI', () => { expect( dropdown.buttonView.isEnabled ).to.be.false; } ); + it( 'should bind items in panel to proper commands (RTL content)', () => { + const element = document.createElement( 'div' ); + + document.body.appendChild( element ); + + return ClassicTestEditor + .create( element, { + language: { + ui: 'en', + content: 'ar' + }, + plugins: [ TableEditing, TableUI ] + } ) + .then( editor => { + const dropdown = editor.ui.componentFactory.create( 'mergeTableCells' ); + const items = dropdown.listView.items; + + expect( items.get( 1 ).children.first.label ).to.equal( 'Merge cell right' ); + expect( items.get( 1 ).children.first.commandName ).to.equal( 'mergeTableCellLeft' ); + + expect( items.get( 3 ).children.first.label ).to.equal( 'Merge cell left' ); + expect( items.get( 3 ).children.first.commandName ).to.equal( 'mergeTableCellRight' ); + + element.remove(); + + return editor.destroy(); + } ); + } ); + it( 'should focus view after command execution', () => { const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' );