Merge pull request #9692 from ckeditor/i/5585

Fix: Fixed arrow handling with toolbar focused in case RTL language UI. Closes #5585.
ckeditor · May 20, 2021 · 2d2e34f · 2d2e34f
2 parents eb331b1 + d1215a1
commit 2d2e34f
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 6 deletions.
diff --git a/packages/ckeditor5-ui/src/toolbar/toolbarview.js b/packages/ckeditor5-ui/src/toolbar/toolbarview.js
@@ -171,16 +171,19 @@ export default class ToolbarView extends View {
 		 * @protected
 		 * @member {module:ui/focuscycler~FocusCycler}
 		 */
+
+		const isRtl = locale.uiLanguageDirection === 'rtl';
+
 		this._focusCycler = new FocusCycler( {
 			focusables: this.focusables,
 			focusTracker: this.focusTracker,
 			keystrokeHandler: this.keystrokes,
 			actions: {
 				// Navigate toolbar items backwards using the arrow[left,up] keys.
-				focusPrevious: [ 'arrowleft', 'arrowup' ],
+				focusPrevious: [ isRtl ? 'arrowright' : 'arrowleft', 'arrowup' ],
 
 				// Navigate toolbar items forwards using the arrow[right,down] keys.
-				focusNext: [ 'arrowright', 'arrowdown' ]
+				focusNext: [ isRtl ? 'arrowleft' : 'arrowright', 'arrowdown' ]
 			}
 		} );
 

diff --git a/packages/ckeditor5-ui/tests/manual/toolbar/grouping.html b/packages/ckeditor5-ui/tests/manual/toolbar/grouping.html
@@ -4,6 +4,13 @@ <h2>Editor with LTR UI</h2>
 	<p>Editor content</p>
 </div>
 
+<h2>Editor with RTL content, LTR UI</h2>
+
+<div id="editor-rtl-mixed">
+	<p>Editor content</p>
+</div>
+
+
 <h2>Editor with RTL UI</h2>
 
 <div id="editor-rtl">

diff --git a/packages/ckeditor5-ui/tests/manual/toolbar/grouping.js b/packages/ckeditor5-ui/tests/manual/toolbar/grouping.js
@@ -8,10 +8,11 @@
 import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
 import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
 
-createEditor( '#editor-ltr', 'en' );
-createEditor( '#editor-rtl', 'ar' );
+createEditor( '#editor-ltr', 'en', 'en' );
+createEditor( '#editor-rtl-mixed', 'ar', 'en' );
+createEditor( '#editor-rtl', 'ar', 'ar' );
 
-function createEditor( selector, language ) {
+function createEditor( selector, language, uiLanguageCode ) {
 	ClassicEditor
 		.create( document.querySelector( selector ), {
 			plugins: [ ArticlePluginSet ],
@@ -41,7 +42,10 @@ function createEditor( selector, language ) {
 					'mergeTableCells'
 				]
 			},
-			language
+			language: {
+				ui: uiLanguageCode,
+				content: language
+			}
 		} )
 		.then( editor => {
 			window.editor = editor;

diff --git a/packages/ckeditor5-ui/tests/manual/toolbar/grouping.md b/packages/ckeditor5-ui/tests/manual/toolbar/grouping.md
@@ -29,6 +29,12 @@
 6. Enter the group button, navigate across grouped items, go back (<kbd>Esc</kbd>).
 7. There should be no interruptions or glitches in the navigation.
 
+## RTL content with LTR UI
+
+1. Perform the same scenarios as for the initial editor.
+2. There should be no visual or behavioral difference between the first editor and this one except that the content is mirrored.
+3. The button aggregating grouped toolbar items should be displayed on the right-hand side.
+
 ## RTL UI support
 
 1. Perform the same scenarios in the editor with RTL (right–to–left) UI.

diff --git a/packages/ckeditor5-ui/tests/toolbar/toolbarview.js b/packages/ckeditor5-ui/tests/toolbar/toolbarview.js
@@ -361,6 +361,48 @@ describe( 'ToolbarView', () => {
 			} );
 		} );
 
+		describe( 'activates keyboard navigation for the RTL toolbar', () => {
+			beforeEach( () => {
+				view.destroy();
+				locale = new Locale( { uiLanguage: 'ar' } );
+
+				view = new ToolbarView( locale );
+				view.render();
+			} );
+
+			it( 'so "arrowleft" focuses next focusable item', () => {
+				const keyEvtData = getArrowKeyData( 'arrowleft' );
+
+				view.items.add( focusable() );
+				view.items.add( nonFocusable() );
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+
+				// Mock the first item is focused.
+				view.focusTracker.isFocused = true;
+				view.focusTracker.focusedElement = view.items.get( 0 ).element;
+
+				view.keystrokes.press( keyEvtData );
+				sinon.assert.calledOnce( view.items.get( 2 ).focus );
+			} );
+
+			it( 'so "arrowright" focuses previous focusable item', () => {
+				const keyEvtData = getArrowKeyData( 'arrowright' );
+
+				view.items.add( focusable() );
+				view.items.add( nonFocusable() );
+				view.items.add( focusable() );
+				view.items.add( focusable() );
+
+				// Mock the last item is focused.
+				view.focusTracker.isFocused = true;
+				view.focusTracker.focusedElement = view.items.get( 0 ).element;
+
+				view.keystrokes.press( keyEvtData );
+				sinon.assert.calledOnce( view.items.get( 3 ).focus );
+			} );
+		} );
+
 		it( 'calls _behavior#render()', () => {
 			const view = new ToolbarView( locale );
 			sinon.spy( view._behavior, 'render' );