Merge pull request #155 from ckeditor/t/134

Fix: Keyboard navigation should work inside to-do lists in RTL content (see ckeditor/ckeditor5-list#134).

src/todolistediting.js

@@ -120,7 +120,11 @@ export default class TodoListEditing extends Plugin {
 		//
 		// <blockquote><p>Foo{}</p></blockquote>
 		// <ul><li><checkbox/>Bar</li></ul>
-		editor.keystrokes.set( 'arrowleft', ( evt, stop ) => jumpOverCheckmarkOnLeftArrowKeyPress( stop, model ) );
+		//
+		// Note: When content language direction is RTL, the behaviour is mirrored.
+		const localizedJumpOverCheckmarkKey = editor.locale.contentLanguageDirection === 'ltr' ? 'arrowleft' : 'arrowright';
+
+		editor.keystrokes.set( localizedJumpOverCheckmarkKey, ( evt, stop ) => jumpOverCheckmarkOnSideArrowKeyPress( stop, model ) );
 
 		// Toggle check state of selected to-do list items on keystroke.
 		editor.keystrokes.set( 'Ctrl+space', () => editor.execute( 'todoListCheck' ) );
@@ -249,13 +253,14 @@ function moveSelectionAfterCheckmark( writer, selection ) {
 	return false;
 }
 
-// Handles the left arrow key and moves the selection at the end of the previous block element if the selection is just after
-// the checkbox element. In other words, it jumps over the checkbox element when moving the selection to the left.
+// Handles the left/right (LTR/RTL content) arrow key and moves the selection at the end of the previous block element
+// if the selection is just after the checkbox element. In other words, it jumps over the checkbox element when
+// moving the selection to the left/right (LTR/RTL).
 //
 // @private
 // @param {Function} stopKeyEvent
 // @param {module:engine/model/model~Model} model
-function jumpOverCheckmarkOnLeftArrowKeyPress( stopKeyEvent, model ) {
+function jumpOverCheckmarkOnSideArrowKeyPress( stopKeyEvent, model ) {
 	const schema = model.schema;
 	const selection = model.document.selection;
 

tests/manual/todo-list-rtl.html

@@ -0,0 +1,24 @@
+<div id="editor">
+	<p>مرحبا</p>
+	<ul>
+		<li><input type="checkbox">مرحبا</li>
+		<li><input type="checkbox" checked="checked">مرحبا</li>
+		<li><input type="checkbox">مرحبا</li>
+		<li><input type="checkbox" checked="checked">مرحبا</li>
+		<li><input type="checkbox">مرحبا</li>
+		<li><input type="checkbox">مرحبا</li>
+		<li><input type="checkbox" checked="checked">مرحبا</li>
+		<li><input type="checkbox">مرحبا</li>
+	</ul>
+	<p>مرحبا.</p>
+	<p>مرحبا</p>
+	<ol>
+		<li>مرحبا</li>
+	</ol>
+	<ul>
+		<li><input type="checkbox">مرحبا</li>
+	</ul>
+	<ul>
+		<li>مرحبا</li>
+	</ul>
+</div>

tests/manual/todo-list-rtl.js

@@ -0,0 +1,41 @@
+/**
+ * @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 Enter from '@ckeditor/ckeditor5-enter/src/enter';
+import Typing from '@ckeditor/ckeditor5-typing/src/typing';
+import Heading from '@ckeditor/ckeditor5-heading/src/heading';
+import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
+import Highlight from '@ckeditor/ckeditor5-highlight/src/highlight';
+import Table from '@ckeditor/ckeditor5-table/src/table';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+import Undo from '@ckeditor/ckeditor5-undo/src/undo';
+import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard';
+import List from '../../src/list';
+import TodoList from '../../src/todolist';
+
+ClassicEditor
+	.create( document.querySelector( '#editor' ), {
+		plugins: [ Enter, Typing, Heading, Highlight, Table, Bold, Paragraph, Undo, List, TodoList, Clipboard ],
+		language: 'ar',
+		toolbar: [
+			'heading', '|', 'bulletedList', 'numberedList', 'todoList', '|', 'bold', 'highlight', 'insertTable', '|', 'undo', 'redo'
+		],
+		table: {
+			contentToolbar: [
+				'tableColumn',
+				'tableRow',
+				'mergeTableCells'
+			]
+		}
+	} )
+	.then( editor => {
+		window.editor = editor;
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );

tests/manual/todo-list-rtl.md

@@ -0,0 +1,19 @@
+## To–do list and RTL (right–to–left content)
+
+### UI
+
+1. Make sure the list is displayed correctly.
+	1. Check boxes should be on the left side of the text.
+	2. There should be some space between the text and the check mark.
+	3. List should be indented from the right side.
+
+### Keyboard navigation
+
+1. Put a selection at the beginning of the document.
+2. Use the **right arrow** key to navigate forwards.
+3. Go through the to–do list.
+4. The navigation should
+	* be uninterrupted,
+	* go always forward.
+5. Reach the end of the document.
+6. Go backwards using the **left arrow** key and repeat the entire scenario.

tests/todolistediting.js

@@ -790,69 +790,142 @@ describe( 'TodoListEditing', () => {
 		} );
 	} );
 
-	describe( 'leftArrow key handling', () => {
-		let domEvtDataStub;
+	describe( 'arrow key handling', () => {
+		let editor, model, view, viewDoc, domEvtDataStub;
+
+		describe( 'left arrow in a LTR (left–to–right) content', () => {
+			beforeEach( () => {
+				return VirtualTestEditor
+					.create( {
+						language: 'en',
+						plugins: [ TodoListEditing, Typing, BoldEditing, BlockQuoteEditing ]
+					} )
+					.then( newEditor => {
+						editor = newEditor;
+
+						model = editor.model;
+						view = editor.editing.view;
+						viewDoc = view.document;
+
+						model.schema.register( 'foo', {
+							allowWhere: '$block',
+							allowAttributes: [ 'listIndent', 'listType' ],
+							isBlock: true,
+							isObject: true
+						} );
+
+						domEvtDataStub = {
+							keyCode: getCode( 'arrowLeft' ),
+							preventDefault: sinon.spy(),
+							stopPropagation: sinon.spy(),
+							domTarget: {
+								ownerDocument: {
+									defaultView: {
+										getSelection: () => ( { rangeCount: 0 } )
+									}
+								}
+							}
+						};
+					} );
+			} );
 
-		beforeEach( () => {
-			domEvtDataStub = {
-				keyCode: getCode( 'arrowLeft' ),
-				preventDefault: sinon.spy(),
-				stopPropagation: sinon.spy(),
-				domTarget: {
-					ownerDocument: {
-						defaultView: {
-							getSelection: () => ( { rangeCount: 0 } )
-						}
-					}
-				}
-			};
-		} );
+			afterEach( () => {
+				editor.destroy();
+			} );
 
-		it( 'should jump at the end of the previous node when selection is after checkmark element', () => {
-			setModelData( model,
-				'<blockQuote><paragraph>foo</paragraph></blockQuote>' +
-				'<listItem listIndent="0" listType="todo">[]bar</listItem>'
-			);
+			testArrowKey();
+		} );
 
-			viewDoc.fire( 'keydown', domEvtDataStub );
+		describe( 'right arrow in a RTL (left–to–right) content', () => {
+			beforeEach( () => {
+				return VirtualTestEditor
+					.create( {
+						language: 'ar',
+						plugins: [ TodoListEditing, Typing, BoldEditing, BlockQuoteEditing ]
+					} )
+					.then( newEditor => {
+						editor = newEditor;
+
+						model = editor.model;
+						view = editor.editing.view;
+						viewDoc = view.document;
+
+						model.schema.register( 'foo', {
+							allowWhere: '$block',
+							allowAttributes: [ 'listIndent', 'listType' ],
+							isBlock: true,
+							isObject: true
+						} );
+
+						domEvtDataStub = {
+							keyCode: getCode( 'arrowRight' ),
+							preventDefault: sinon.spy(),
+							stopPropagation: sinon.spy(),
+							domTarget: {
+								ownerDocument: {
+									defaultView: {
+										getSelection: () => ( { rangeCount: 0 } )
+									}
+								}
+							}
+						};
+					} );
+			} );
 
-			expect( getModelData( model ) ).to.equal(
-				'<blockQuote><paragraph>foo[]</paragraph></blockQuote>' +
-				'<listItem listIndent="0" listType="todo">bar</listItem>'
-			);
+			afterEach( () => {
+				editor.destroy();
+			} );
 
-			sinon.assert.calledOnce( domEvtDataStub.preventDefault );
-			sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
+			testArrowKey();
 		} );
 
-		it( 'should do nothing when list item is a first block element in the root', () => {
-			setModelData( model, '<listItem listIndent="0" listType="todo">[]bar</listItem>' );
+		function testArrowKey() {
+			it( 'should jump at the end of the previous node when selection is after checkmark element', () => {
+				setModelData( model,
+					'<blockQuote><paragraph>foo</paragraph></blockQuote>' +
+					'<listItem listIndent="0" listType="todo">[]bar</listItem>'
+				);
 
-			viewDoc.fire( 'keydown', domEvtDataStub );
+				viewDoc.fire( 'keydown', domEvtDataStub );
 
-			sinon.assert.notCalled( domEvtDataStub.preventDefault );
-			sinon.assert.notCalled( domEvtDataStub.stopPropagation );
+				expect( getModelData( model ) ).to.equal(
+					'<blockQuote><paragraph>foo[]</paragraph></blockQuote>' +
+					'<listItem listIndent="0" listType="todo">bar</listItem>'
+				);
 
-			expect( getModelData( model ) ).to.equal( '<listItem listIndent="0" listType="todo">[]bar</listItem>' );
-		} );
+				sinon.assert.calledOnce( domEvtDataStub.preventDefault );
+				sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
+			} );
 
-		it( 'should do nothing when selection is not collapsed', () => {
-			setModelData( model, '<listItem listIndent="0" listType="todo">[bar]</listItem>' );
+			it( 'should do nothing when list item is a first block element in the root', () => {
+				setModelData( model, '<listItem listIndent="0" listType="todo">[]bar</listItem>' );
 
-			viewDoc.fire( 'keydown', domEvtDataStub );
+				viewDoc.fire( 'keydown', domEvtDataStub );
 
-			sinon.assert.notCalled( domEvtDataStub.preventDefault );
-			sinon.assert.notCalled( domEvtDataStub.stopPropagation );
-		} );
+				sinon.assert.notCalled( domEvtDataStub.preventDefault );
+				sinon.assert.notCalled( domEvtDataStub.stopPropagation );
 
-		it( 'should do nothing when selection is not at the beginning list item', () => {
-			setModelData( model, '<listItem listIndent="0" listType="todo">b[]ar</listItem>' );
+				expect( getModelData( model ) ).to.equal( '<listItem listIndent="0" listType="todo">[]bar</listItem>' );
+			} );
 
-			viewDoc.fire( 'keydown', domEvtDataStub );
+			it( 'should do nothing when selection is not collapsed', () => {
+				setModelData( model, '<listItem listIndent="0" listType="todo">[bar]</listItem>' );
 
-			sinon.assert.notCalled( domEvtDataStub.preventDefault );
-			sinon.assert.notCalled( domEvtDataStub.stopPropagation );
-		} );
+				viewDoc.fire( 'keydown', domEvtDataStub );
+
+				sinon.assert.notCalled( domEvtDataStub.preventDefault );
+				sinon.assert.notCalled( domEvtDataStub.stopPropagation );
+			} );
+
+			it( 'should do nothing when selection is not at the beginning list item', () => {
+				setModelData( model, '<listItem listIndent="0" listType="todo">b[]ar</listItem>' );
+
+				viewDoc.fire( 'keydown', domEvtDataStub );
+
+				sinon.assert.notCalled( domEvtDataStub.preventDefault );
+				sinon.assert.notCalled( domEvtDataStub.stopPropagation );
+			} );
+		}
 	} );
 
 	describe( 'Ctrl+space keystroke handling', () => {