Merge pull request #218 from ckeditor/t/200c

Fix: Column insertion and cell merging buttons should work correctly when the editor content is right–to–left (RTL). Closes #200.

Author: jodator

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' )
 					}
 				},

tests/manual/rtl.html

@@ -0,0 +1,30 @@
+<style>
+	body {
+		font-family: Helvetica, Arial, sans-serif;
+		font-size: 14px;
+	}
+</style>
+
+<div id="editor">
+	<figure class="table">
+		<table>
+			<tbody>
+				<tr>
+					<td>1</td>
+					<td>2</td>
+					<td>3</td>
+				</tr>
+				<tr>
+					<td>4</td>
+					<td>5</td>
+					<td>6</td>
+				</tr>
+				<tr>
+					<td>7</td>
+					<td>8</td>
+					<td>9</td>
+				</tr>
+			</tbody>
+		</table>
+	</figure>
+</div>

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 );
+	} );

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.

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