Skip to content

Commit

Permalink
Merge pull request #12202 from ckeditor/ck/11851-keyboard-handling-fo…
Browse files Browse the repository at this point in the history 
…r-grid-components-discovery-poc

Feature (ui): Introduced the `addKeyboardHandlingForGrid()` helper to handle grid keyboard navigation. Closes #11851.

Fix (font): Fixed focus order for color grid in font color and background drop-downs. Closes #11841.

Fix (list): Unified focus handling and keyboard navigation in list properties drop-downs. Closes #11041.
  • Loading branch information
@oleq
oleq authored Aug 12, 2022
2 parents 7445513 + f8ef863 commit 5e8cdcb
Show file tree
Hide file tree
Showing 9 changed files with 614 additions and 76 deletions.
49 changes: 36 additions & 13 deletions packages/ckeditor5-font/src/ui/colortableview.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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'
}
} );

Expand All @@ -169,7 +187,7 @@ export default class ColorTableView extends View {
children: this.items
} );

this.items.add( this._removeColorButton() );
this.items.add( this._createRemoveColorButton() );
}

/**
Expand Down Expand Up @@ -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 );
}
Expand All @@ -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.
Expand All @@ -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 );
}
}

Expand All @@ -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( {
Expand All @@ -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;
}

Expand Down
81 changes: 71 additions & 10 deletions packages/ckeditor5-font/tests/ui/colortableview.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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( () => {
Expand Down
34 changes: 26 additions & 8 deletions packages/ckeditor5-list/src/listproperties/ui/listpropertiesview.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import {
FocusCycler,
SwitchButtonView,
LabeledFieldView,
createLabeledInputNumber
createLabeledInputNumber,
addKeyboardHandlingForGrid
} from 'ckeditor5/src/ui';
import {
FocusTracker,
Expand Down Expand Up @@ -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 ) {
Expand Down Expand Up @@ -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;
}

Expand Down
Loading

0 comments on commit 5e8cdcb

Please sign in to comment.