Keyboard handling for grid components in font color and list style

packages/ckeditor5-font/src/ui/colortableview.js

@@ -8,7 +8,16 @@
  */
 
 import { icons } from 'ckeditor5/src/core';
-import { ButtonView, ColorGridView, ColorTileView, FocusCycler, LabelView, Template, View } from 'ckeditor5/src/ui';
+import {
+	ButtonView,
+	ColorGridView,
+	ColorTileView,
+	FocusCycler,
+	LabelView,
+	Template,
+	View,
+	ViewCollection
+} from 'ckeditor5/src/ui';
 import { FocusTracker, KeystrokeHandler } from 'ckeditor5/src/utils';
 
 import DocumentColorCollection from '../documentcolorcollection';
@@ -109,6 +118,15 @@ export default class ColorTableView extends View {
 		 */
 		this.documentColorsCount = documentColorsCount;
 
+		/**
+		 * A collection of views that can be focused in the view.
+		 *
+		 * @readonly
+		 * @protected
+		 * @member {module:ui/viewcollection~ViewCollection}
+		 */
+		this._focusables = new ViewCollection();
+
 		/**
 		 * Preserves the reference to {@link module:ui/colorgrid/colorgrid~ColorGridView} used to create
 		 * the default (static) color set.
@@ -137,15 +155,15 @@ export default class ColorTableView extends View {
 		 * @member {module:ui/focuscycler~FocusCycler}
 		 */
 		this._focusCycler = new FocusCycler( {
-			focusables: this.items,
+			focusables: this._focusables,
 			focusTracker: this.focusTracker,
 			keystrokeHandler: this.keystrokes,
 			actions: {
-				// Navigate list items backwards using the Arrow Up key.
-				focusPrevious: 'arrowup',
+				// Navigate list items backwards using the <kbd>Shift</kbd> + <kbd>Tab</kbd> keystroke.
+				focusPrevious: 'shift + tab',
 
-				// Navigate list items forwards using the Arrow Down key.
-				focusNext: 'arrowdown'
+				// Navigate list items forwards using the <kbd>Tab</kbd> key.
+				focusNext: 'tab'
 			}
 		} );
 
@@ -169,7 +187,7 @@ export default class ColorTableView extends View {
 			children: this.items
 		} );
 
-		this.items.add( this._removeColorButton() );
+		this.items.add( this._createRemoveColorButton() );
 	}
 
 	/**
@@ -226,11 +244,6 @@ export default class ColorTableView extends View {
 	render() {
 		super.render();
 
-		// Items added before rendering should be known to the #focusTracker.
-		for ( const item of this.items ) {
-			this.focusTracker.add( item.element );
-		}
-
 		// Start listening for the keystrokes coming from #element.
 		this.keystrokes.listenTo( this.element );
 	}
@@ -256,6 +269,8 @@ export default class ColorTableView extends View {
 		this.staticColorsGrid = this._createStaticColorsGrid();
 
 		this.items.add( this.staticColorsGrid );
+		this.focusTracker.add( this.staticColorsGrid.element );
+		this._focusables.add( this.staticColorsGrid );
 
 		if ( this.documentColorsCount ) {
 			// Create a label for document colors.
@@ -273,7 +288,10 @@ export default class ColorTableView extends View {
 			} );
 			this.items.add( label );
 			this.documentColorsGrid = this._createDocumentColorsGrid();
+
 			this.items.add( this.documentColorsGrid );
+			this.focusTracker.add( this.documentColorsGrid.element );
+			this._focusables.add( this.documentColorsGrid );
 		}
 	}
 
@@ -297,7 +315,7 @@ export default class ColorTableView extends View {
 	 * @private
 	 * @returns {module:ui/button/buttonview~ButtonView}
 	 */
-	_removeColorButton() {
+	_createRemoveColorButton() {
 		const buttonView = new ButtonView();
 
 		buttonView.set( {
@@ -312,6 +330,11 @@ export default class ColorTableView extends View {
 			this.fire( 'execute', { value: null } );
 		} );
 
+		buttonView.render();
+
+		this.focusTracker.add( buttonView.element );
+		this._focusables.add( buttonView );
+
 		return buttonView;
 	}
 

packages/ckeditor5-font/tests/ui/colortableview.js

@@ -5,20 +5,24 @@
 
 /* globals document,Event */
 
-import TestColorPlugin from '../_utils/testcolorplugin';
+import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import ColorTableView from './../../src/ui/colortableview';
+import ColorTileView from '@ckeditor/ckeditor5-ui/src/colorgrid/colortileview';
+
 import Collection from '@ckeditor/ckeditor5-utils/src/collection';
 import ViewCollection from '@ckeditor/ckeditor5-ui/src/viewcollection';
 import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker';
 import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler';
 import FocusCycler from '@ckeditor/ckeditor5-ui/src/focuscycler';
-import removeButtonIcon from '@ckeditor/ckeditor5-core/theme/icons/eraser.svg';
+import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
+import global from '@ckeditor/ckeditor5-utils/src/dom/global';
+
+import TestColorPlugin from '../_utils/testcolorplugin';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
-import ColorTileView from '@ckeditor/ckeditor5-ui/src/colorgrid/colortileview';
-import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
 import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
-import global from '@ckeditor/ckeditor5-utils/src/dom/global';
+
+import removeButtonIcon from '@ckeditor/ckeditor5-core/theme/icons/eraser.svg';
 
 describe( 'ColorTableView', () => {
 	let locale, colorTableView;
@@ -79,12 +83,16 @@ describe( 'ColorTableView', () => {
 			documentColorsLabel: 'Document colors',
 			documentColorsCount: 4
 		} );
-		colorTableView.appendGrids();
+		// Grids rendering is deferred (#6192) therefore render happens before appending grids.
 		colorTableView.render();
+		colorTableView.appendGrids();
+
+		document.body.appendChild( colorTableView.element );
 	} );
 
 	afterEach( () => {
 		colorTableView.destroy();
+		colorTableView.element.remove();
 	} );
 
 	testUtils.createSinonSandbox();
@@ -163,22 +171,74 @@ describe( 'ColorTableView', () => {
 		} );
 	} );
 
-	describe( 'focus tracker', () => {
-		it( 'should focus first child of colorTableView in DOM', () => {
+	describe( 'focus tracking', () => {
+		it( 'should focus first child of colorTableView in DOM on focus()', () => {
 			const spy = sinon.spy( colorTableView._focusCycler, 'focusFirst' );
 
 			colorTableView.focus();
 
 			sinon.assert.calledOnce( spy );
 		} );
 
-		it( 'should focuses the last child of colorTableView in DOM', () => {
+		it( 'should focus the last child of colorTableView in DOM on focusLast()', () => {
 			const spy = sinon.spy( colorTableView._focusCycler, 'focusLast' );
 
 			colorTableView.focusLast();
 
 			sinon.assert.calledOnce( spy );
 		} );
+
+		describe( 'navigation across table controls using Tab and Shift+Tab keys', () => {
+			beforeEach( () => {
+				// Needed for the document colors grid to show up in the view.
+				colorTableView.documentColors.add( {
+					color: '#000000',
+					label: 'Black',
+					options: {
+						hasBorder: false
+					}
+				} );
+			} );
+
+			it( 'should navigate forwards using the Tab key', () => {
+				const keyEvtData = {
+					keyCode: keyCodes.tab,
+					preventDefault: sinon.spy(),
+					stopPropagation: sinon.spy()
+				};
+
+				// Mock the remove color button is focused.
+				colorTableView.focusTracker.isFocused = true;
+				colorTableView.focusTracker.focusedElement = colorTableView.items.get( 0 ).element;
+
+				const spy = sinon.spy( colorTableView.staticColorsGrid, 'focus' );
+
+				colorTableView.keystrokes.press( keyEvtData );
+				sinon.assert.calledOnce( keyEvtData.preventDefault );
+				sinon.assert.calledOnce( keyEvtData.stopPropagation );
+				sinon.assert.calledOnce( spy );
+			} );
+
+			it( 'should navigate backwards using the Shift+Tab key', () => {
+				const keyEvtData = {
+					keyCode: keyCodes.tab,
+					shiftKey: true,
+					preventDefault: sinon.spy(),
+					stopPropagation: sinon.spy()
+				};
+
+				// Mock the remove color button is focused.
+				colorTableView.focusTracker.isFocused = true;
+				colorTableView.focusTracker.focusedElement = colorTableView.items.get( 0 ).element;
+
+				const spy = sinon.spy( colorTableView.documentColorsGrid, 'focus' );
+
+				colorTableView.keystrokes.press( keyEvtData );
+				sinon.assert.calledOnce( keyEvtData.preventDefault );
+				sinon.assert.calledOnce( keyEvtData.stopPropagation );
+				sinon.assert.calledOnce( spy );
+			} );
+		} );
 	} );
 
 	describe( 'remove color button', () => {
@@ -382,8 +442,9 @@ describe( 'ColorTableView', () => {
 					removeButtonLabel: 'Remove color',
 					documentColorsCount: 0
 				} );
-				colorTableView.appendGrids();
+				// Grids rendering is deferred (#6192) therefore render happens before appending grids.
 				colorTableView.render();
+				colorTableView.appendGrids();
 			} );
 
 			afterEach( () => {

packages/ckeditor5-list/src/listproperties/ui/listpropertiesview.js

@@ -13,7 +13,8 @@ import {
 	FocusCycler,
 	SwitchButtonView,
 	LabeledFieldView,
-	createLabeledInputNumber
+	createLabeledInputNumber,
+	addKeyboardHandlingForGrid
 } from 'ckeditor5/src/ui';
 import {
 	FocusTracker,
@@ -178,19 +179,25 @@ export default class ListPropertiesView extends View {
 		super.render();
 
 		if ( this.stylesView ) {
-			for ( const styleButtonView of this.stylesView.children ) {
-				// Register the view as focusable.
-				this.focusables.add( styleButtonView );
-
-				// Register the view in the focus tracker.
-				this.focusTracker.add( styleButtonView.element );
-			}
+			this.focusables.add( this.stylesView );
+			this.focusTracker.add( this.stylesView.element );
 
 			// Register the collapsible toggle button to the focus system.
 			if ( this.startIndexFieldView || this.reversedSwitchButtonView ) {
 				this.focusables.add( this.children.last.buttonView );
 				this.focusTracker.add( this.children.last.buttonView.element );
 			}
+
+			for ( const item of this.stylesView.children ) {
+				this.stylesView.focusTracker.add( item.element );
+			}
+
+			addKeyboardHandlingForGrid( {
+				keystrokeHandler: this.stylesView.keystrokes,
+				focusTracker: this.stylesView.focusTracker,
+				gridItems: this.stylesView.children,
+				numberOfColumns: 4
+			} );
 		}
 
 		if ( this.startIndexFieldView ) {
@@ -276,6 +283,17 @@ export default class ListPropertiesView extends View {
 
 		stylesView.children.delegate( 'execute' ).to( this );
 
+		stylesView.focus = function() {
+			this.children.first.focus();
+		};
+
+		stylesView.focusTracker = new FocusTracker();
+		stylesView.keystrokes = new KeystrokeHandler();
+
+		stylesView.render();
+
+		stylesView.keystrokes.listenTo( stylesView.element );
+
 		return stylesView;
 	}