Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keyboard handling for grid components in font color and list style #12202

Merged
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
dd4e5e6
poc for colors and list properties
mmotyczynska Jul 14, 2022
bdf35cb
poc for tables
mmotyczynska Jul 18, 2022
7159662
Special characters arrow navigation
mmotyczynska Jul 19, 2022
f30aafd
Change addKeyboardHandlingForGrid to pass reference to children colle…
mmotyczynska Jul 19, 2022
04bb929
Create parent view for special characters
mmotyczynska Jul 19, 2022
b0c8e4d
Finish handling of arrowup in grid navigation.
mmotyczynska Jul 28, 2022
4f019d3
Fix naviagation for insert table grid (arrowright, arrowleft).
mmotyczynska Jul 28, 2022
387c6e1
Merge remote-tracking branch 'origin/master' into ck/11851-keyboard-h…
mmotyczynska Jul 28, 2022
cf0ece8
Fix navigation with tab in special characters
mmotyczynska Jul 28, 2022
28f2acd
Merge remote-tracking branch 'origin/master' into ck/11851-keyboard-h…
mmotyczynska Jul 28, 2022
62a604c
Some code adjusting.
mmotyczynska Aug 1, 2022
1d0dbdc
Move addKeyboardHandlingForGrid to ui/bindings.
mmotyczynska Aug 2, 2022
ce0a42f
Add arrow navigation to bulleted list.
mmotyczynska Aug 2, 2022
5690290
Improve tab navigation in list styles (tab no longer moves focus betw…
mmotyczynska Aug 2, 2022
ccb2792
Fix 'start at' in list properties by moving keystroke handlers for li…
mmotyczynska Aug 2, 2022
516eb51
Move keyboard navigation handlers to the ColorGridView - this adds na…
mmotyczynska Aug 2, 2022
f500ad0
Fix navigation with tab for font color dropdowns.
mmotyczynska Aug 2, 2022
9fd48cc
Remove focusCycler form ColorGridView because now the keystrokes (arr…
mmotyczynska Aug 2, 2022
111782a
Get focused element using FocusTracker instead of document.activeElem…
mmotyczynska Aug 2, 2022
3c94673
Change the function parameter from view to keystrokes.
mmotyczynska Aug 3, 2022
a12ec03
Change grid view collection listeners from 'add/remove' to 'change'.
mmotyczynska Aug 3, 2022
36065c3
Fix inserting tables after removing view parameter from the helper.
mmotyczynska Aug 3, 2022
5497d76
Improve focusTracker for InsertTableView.
mmotyczynska Aug 3, 2022
8e5a42f
Merge remote-tracking branch 'origin/master' into ck/11851-keyboard-h…
mmotyczynska Aug 3, 2022
9f28dc2
Add keyboard navigation only when there is styles grid.
mmotyczynska Aug 4, 2022
d607ff4
Remove test for focus cycler in ColorGridView.
mmotyczynska Aug 4, 2022
e735c34
Ensure every grid has its own focusTrackers and pass it to helper fun…
mmotyczynska Aug 4, 2022
6d19dca
Add document colors grid to focus tracker in a right place. Change or…
mmotyczynska Aug 5, 2022
7f6b334
Update tests of listpropertiesview after moving grid elements to the …
mmotyczynska Aug 5, 2022
c09629f
Modify tests for list properties for tab, shift+tab and arrowright ke…
mmotyczynska Aug 8, 2022
5746bd0
Update tests for special characters after grouping navigation, grid a…
mmotyczynska Aug 8, 2022
2caba6f
Merge remote-tracking branch 'origin/master' into ck/11851-keyboard-h…
mmotyczynska Aug 8, 2022
0f04cbf
Unit test for focus() in charactergridview.
mmotyczynska Aug 8, 2022
087b28c
Remove some unnecessary comments from specialcharactersview.js.
mmotyczynska Aug 8, 2022
ecf7c09
Add test for focus() in specialcharactersview.js.
mmotyczynska Aug 8, 2022
131bd3e
Improve cc for inserttableview.js.
mmotyczynska Aug 8, 2022
e6528b4
Destroy view in afterEach in inserttableview.js tests.
mmotyczynska Aug 9, 2022
86a29a1
Add test when nothing is focused in inserttableview.js.
mmotyczynska Aug 9, 2022
027b0a1
Add unit tests for addKeyboardHandlingForGrid helper.
mmotyczynska Aug 9, 2022
b4550a9
Merge remote-tracking branch 'origin/master' into ck/11851-keyboard-h…
mmotyczynska Aug 9, 2022
4fa70e7
Docs added. The name of function changed: getFocusedElementIndex.
mmotyczynska Aug 10, 2022
a94610e
Refactor of addKeyboardHandlingForGrid helper.
mmotyczynska Aug 10, 2022
beae189
Do not convert collection to an array.
mmotyczynska Aug 10, 2022
b925457
Fix typo.
mmotyczynska Aug 10, 2022
8a5af96
Use number of columns from the configuration.
mmotyczynska Aug 10, 2022
c278ac8
Refactor tests for addKeyboardHandlingForGrid.js.
mmotyczynska Aug 10, 2022
2c74741
Move TestView class definition into describe scope.
mmotyczynska Aug 10, 2022
4cfa57a
Move stylesView.keystrokes initialization to _createStylesView.
mmotyczynska Aug 10, 2022
2568b6a
Code refactoring fro getGridItemFocuser (use collection api).
mmotyczynska Aug 10, 2022
8eeea65
Revert special characters keyboard navigation to handle it in a diffe…
mmotyczynska Aug 10, 2022
d61f7f0
Revert insert table keyboard navigation to handle it in a different b…
mmotyczynska Aug 10, 2022
9c925db
Set default number of columns in a color grid.
mmotyczynska Aug 11, 2022
861f4cd
Move stylesView keystroke initialization to _createStylesView.
mmotyczynska Aug 11, 2022
d3bde2d
Merge branch 'master' into ck/11851-keyboard-handling-for-grid-compon…
oleq Aug 12, 2022
e90ff17
Refactored the arguments of addKeyboardHandlingForGrid() into an obje…
oleq Aug 12, 2022
4bf588d
Rename addKeyboardHandlingForGrid.js to addkeyboardhandlingforgrid.js
oleq Aug 12, 2022
f13f149
Rename addKeyboardHandlingForGrid.js to addkeyboardhandlingforgrid.js
oleq Aug 12, 2022
8e67abd
Tests: Code refactoring in ListPropertyView tests.
oleq Aug 12, 2022
7f842d4
Tests: Additional tests for the ColorTableView.
oleq Aug 12, 2022
915f7a6
Fixed import paths in several files to use lower case.
oleq Aug 12, 2022
f8ef863
Tests: Additional tests for the keyboard navigation in the ColorGridV…
oleq Aug 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions packages/ckeditor5-font/src/ui/colortableview.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,11 @@ export default class ColorTableView extends View {
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 Down Expand Up @@ -256,6 +256,7 @@ export default class ColorTableView extends View {
this.staticColorsGrid = this._createStaticColorsGrid();

this.items.add( this.staticColorsGrid );
this.focusTracker.add( this.staticColorsGrid.element );

if ( this.documentColorsCount ) {
// Create a label for document colors.
Expand All @@ -274,6 +275,8 @@ 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 );
}
}

Expand Down
6 changes: 4 additions & 2 deletions packages/ckeditor5-font/tests/ui/colortableview.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@ describe( 'ColorTableView', () => {
documentColorsLabel: 'Document colors',
documentColorsCount: 4
} );
colorTableView.appendGrids();
// Grids rendering is deferred (#6192) therefeore render happens before appending grids.
colorTableView.render();
colorTableView.appendGrids();
} );

afterEach( () => {
Expand Down Expand Up @@ -382,8 +383,9 @@ describe( 'ColorTableView', () => {
removeButtonLabel: 'Remove color',
documentColorsCount: 0
} );
colorTableView.appendGrids();
// Grids rendering is deferred (#6192) therefeore render happens before appending grids.
mmotyczynska marked this conversation as resolved.
Show resolved Hide resolved
colorTableView.render();
colorTableView.appendGrids();
} );

afterEach( () => {
Expand Down
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,23 @@ 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 );
}

this.stylesView.keystrokes = new KeystrokeHandler();
this.stylesView.keystrokes.listenTo( this.stylesView.element );
mmotyczynska marked this conversation as resolved.
Show resolved Hide resolved

addKeyboardHandlingForGrid( this.stylesView.keystrokes, this.stylesView.focusTracker, this.stylesView.children, 4 );
}

if ( this.startIndexFieldView ) {
Expand Down Expand Up @@ -276,6 +281,12 @@ export default class ListPropertiesView extends View {

stylesView.children.delegate( 'execute' ).to( this );

stylesView.focus = function() {
this.children.first.focus();
};

stylesView.focusTracker = new FocusTracker();

return stylesView;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,14 +308,20 @@ describe( 'ListPropertiesView', () => {
describe( 'when styles and all numbered list properties are enabled', () => {
it( 'should register child views in #focusables', () => {
expect( view.focusables.map( f => f ) ).to.have.members( [
view.stylesView.children.first,
view.stylesView.children.last,
view.children.first,
view.children.last.buttonView,
view.startIndexFieldView,
view.reversedSwitchButtonView
] );
} );

it( 'should register child views of stylesView in its focusTracker', () => {
expect( [ ...view.stylesView.focusTracker._elements ] ).to.have.members( [
view.stylesView.children.first.element,
view.stylesView.children.last.element
] );
} );

it( 'should register child views\' #element in #focusTracker', () => {
const view = new ListPropertiesView( locale, {
enabledProperties: {
Expand All @@ -330,15 +336,18 @@ describe( 'ListPropertiesView', () => {
styleGridAriaLabel: 'Foo'
} );

const spy = sinon.spy( view.focusTracker, 'add' );
const spyView = sinon.spy( view.focusTracker, 'add' );
const spyStylesView = sinon.spy( view.stylesView.focusTracker, 'add' );

view.render();

sinon.assert.calledWithExactly( spy.getCall( 0 ), view.stylesView.children.first.element );
sinon.assert.calledWithExactly( spy.getCall( 1 ), view.stylesView.children.last.element );
sinon.assert.calledWithExactly( spy.getCall( 2 ), view.children.last.buttonView.element );
sinon.assert.calledWithExactly( spy.getCall( 3 ), view.startIndexFieldView.element );
sinon.assert.calledWithExactly( spy.getCall( 4 ), view.reversedSwitchButtonView.element );
sinon.assert.calledWithExactly( spyStylesView.getCall( 0 ), view.stylesView.children.first.element );
sinon.assert.calledWithExactly( spyStylesView.getCall( 1 ), view.stylesView.children.last.element );

sinon.assert.calledWithExactly( spyView.getCall( 0 ), view.children.first.element );
sinon.assert.calledWithExactly( spyView.getCall( 1 ), view.children.last.buttonView.element );
sinon.assert.calledWithExactly( spyView.getCall( 2 ), view.startIndexFieldView.element );
sinon.assert.calledWithExactly( spyView.getCall( 3 ), view.reversedSwitchButtonView.element );

view.destroy();
} );
Expand Down Expand Up @@ -423,11 +432,12 @@ describe( 'ListPropertiesView', () => {
stopPropagation: sinon.spy()
};

// Mock the first style button is focused.
// Mock the styles view is focused.
view.focusTracker.isFocused = true;
view.focusTracker.focusedElement = view.stylesView.children.first.element;
view.focusTracker.focusedElement = view.children.first.element;

const spy = sinon.spy( view.stylesView.children.last, 'focus' );
// Spy the next view which in this case is the ListProperties button
const spy = sinon.spy( view.children.last.buttonView, 'focus' );

view.keystrokes.press( keyEvtData );
sinon.assert.calledOnce( keyEvtData.preventDefault );
Expand All @@ -443,18 +453,38 @@ describe( 'ListPropertiesView', () => {
stopPropagation: sinon.spy()
};

// Mock the first style button is focused.
// Mock the styles view is focused.
view.focusTracker.isFocused = true;
view.focusTracker.focusedElement = view.stylesView.children.first.element;
view.focusTracker.focusedElement = view.children.first.element;
view.children.last.isCollapsed = false;

// Spy the previous view which in this case is the Reversed order switch button
const spy = sinon.spy( view.reversedSwitchButtonView, 'focus' );

view.keystrokes.press( keyEvtData );
sinon.assert.calledOnce( keyEvtData.preventDefault );
sinon.assert.calledOnce( keyEvtData.stopPropagation );
sinon.assert.calledOnce( spy );
} );

it( 'so "arrow right" focuses the next focusable item', () => {
const keyEvtData = {
keyCode: keyCodes.arrowright,
preventDefault: sinon.spy(),
stopPropagation: sinon.spy()
};

// Mock the first style button is focused.
view.stylesView.focusTracker.isFocused = true;
view.stylesView.focusTracker.focusedElement = view.stylesView.children.first.element;

const spy = sinon.spy( view.stylesView.children.last, 'focus' );

view.stylesView.keystrokes.press( keyEvtData );
sinon.assert.calledOnce( keyEvtData.preventDefault );
sinon.assert.calledOnce( keyEvtData.stopPropagation );
sinon.assert.calledOnce( spy );
} );
} );

it( 'intercepts the arrow* events and overrides the default (parent) toolbar behavior', () => {
Expand Down
12 changes: 9 additions & 3 deletions packages/ckeditor5-special-characters/src/specialcharacters.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { CKEditorError } from 'ckeditor5/src/utils';
import SpecialCharactersNavigationView from './ui/specialcharactersnavigationview';
import CharacterGridView from './ui/charactergridview';
import CharacterInfoView from './ui/characterinfoview';
import SpecialCharactersView from './ui/specialcharactersview';

import specialCharactersIcon from '../theme/icons/specialcharacters.svg';
import '../theme/specialcharacters.css';
Expand Down Expand Up @@ -97,9 +98,14 @@ export default class SpecialCharacters extends Plugin {
if ( !dropdownPanelContent ) {
dropdownPanelContent = this._createDropdownPanelContent( locale, dropdownView );

dropdownView.panelView.children.add( dropdownPanelContent.navigationView );
dropdownView.panelView.children.add( dropdownPanelContent.gridView );
dropdownView.panelView.children.add( dropdownPanelContent.infoView );
const specialCharactersView = new SpecialCharactersView(
locale,
dropdownPanelContent.navigationView,
dropdownPanelContent.gridView,
dropdownPanelContent.infoView
);

dropdownView.panelView.children.add( specialCharactersView );
}

dropdownPanelContent.infoView.set( {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
* @module special-characters/ui/charactergridview
*/

import { View, ButtonView } from 'ckeditor5/src/ui';
import { View, ButtonView, addKeyboardHandlingForGrid } from 'ckeditor5/src/ui';

import { KeystrokeHandler, FocusTracker } from 'ckeditor5/src/utils';

import '../../theme/charactergrid.css';

Expand Down Expand Up @@ -56,6 +58,18 @@ export default class CharacterGridView extends View {
}
} );

/**
* Tracks information about the DOM focus in the grid.
*
* @readonly
* @member {module:utils/focustracker~FocusTracker}
*/
this.focusTracker = new FocusTracker();

this.keystrokes = new KeystrokeHandler();

addKeyboardHandlingForGrid( this.keystrokes, this.focusTracker, this.tiles, 10 );
mmotyczynska marked this conversation as resolved.
Show resolved Hide resolved

/**
* Fired when any of {@link #tiles grid tiles} is clicked.
*
Expand Down Expand Up @@ -113,4 +127,40 @@ export default class CharacterGridView extends View {

return tile;
}

render() {
super.render();

for ( const item of this.tiles ) {
this.focusTracker.add( item.element );
}

this.tiles.on( 'change', ( eventInfo, { added, removed } ) => {
if ( added.length > 0 ) {
for ( const item of added ) {
this.focusTracker.add( item.element );
}
}
if ( removed.length > 0 ) {
for ( const item of removed ) {
this.focusTracker.remove( item.element );
}
}
} );

this.keystrokes.listenTo( this.element );
}

/**
* @inheritDoc
*/
destroy() {
super.destroy();

this.keystrokes.destroy();
}

focus() {
this.tiles.get( 0 ).focus();
}
}
Loading