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

Handling tab and shift+tab keys in document lists and an integration with other plugins #11244

Merged
merged 36 commits into from
Feb 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
8f76321
Merge branch 'ck/11083-multicommand-with-priorities' into ck/10880-ta…
CatStrategist Feb 1, 2022
f51c225
First implementation of tab integration and tab, tab+shift handling i…
CatStrategist Feb 3, 2022
9e86bd8
Added event context for tab key handling while table widget is selected.
niegowski Feb 3, 2022
53599b0
Merge branch 'ck/10812-document-list-editing' into ck/10880-tab-key
CatStrategist Feb 7, 2022
114bbe0
Merge branch 'ck/10880-tab-key' of github.com:ckeditor/ckeditor5 into…
CatStrategist Feb 9, 2022
57bb1dc
Fix tests of features that started to listen to tabobserver instead o…
CatStrategist Feb 9, 2022
5b3c3ca
Merge branch 'ck/11083-multicommand-with-priorities' into ck/10880-ta…
CatStrategist Feb 11, 2022
d1d203c
Documents list tab observers integration tests
CatStrategist Feb 16, 2022
256cbdd
Merge branch 'ck/10812-document-list-editing' into ck/10880-tab-key
Reinmar Feb 16, 2022
34de71d
Simplify setting up listeners in tablekeyboard plugin and update docs…
CatStrategist Feb 16, 2022
147bc1e
Delete a code that was commented out and register the indentCodeBlock…
CatStrategist Feb 16, 2022
c3be58c
Update the tabobserver and fix passing wrong object to the DomEventData.
CatStrategist Feb 16, 2022
9805616
Add priorities to the indentList and the outdentList commands when re…
CatStrategist Feb 16, 2022
18aba9f
Add more tests to integration tests of the DocumentLists plugin.
CatStrategist Feb 16, 2022
851b5b3
Add tests to the TabObserver.
CatStrategist Feb 16, 2022
8bca270
Add minor adjustments to accessing variables.
CatStrategist Feb 16, 2022
65e5886
Add missing createSinonSandbox() calls that were omitted during creat…
CatStrategist Feb 17, 2022
4de0417
Add missing mention of default ArrowKeysObserver in View docs.
CatStrategist Feb 18, 2022
aeaa38d
Delete redundant operation in TabObserver and add documentation.
CatStrategist Feb 18, 2022
cd9506f
Improve API docs for enterobserver.
CatStrategist Feb 18, 2022
c57084e
Imports reorder.
niegowski Feb 18, 2022
8088168
Add comments, fix and organize imports, fix API docs.
CatStrategist Feb 21, 2022
cf2cc86
Add missing ArrowKeysObserver in view tests.
CatStrategist Feb 21, 2022
29277b5
Add integration tests for outdenting block in a list.
CatStrategist Feb 21, 2022
ff4848b
Add more tests.
CatStrategist Feb 22, 2022
6742d7e
Merge branch 'ck/10880-tab-key' of github.com:ckeditor/ckeditor5 into…
CatStrategist Feb 22, 2022
f38eda7
Remove unused describes in tests.
CatStrategist Feb 22, 2022
861a5fe
Add more tests.
CatStrategist Feb 22, 2022
acd4f77
Fix TabObserver test.
CatStrategist Feb 23, 2022
fd2f117
Fix tests formatting.
CatStrategist Feb 23, 2022
a4ccde1
Fix comment describing reasoning behind command execution priority.
CatStrategist Feb 23, 2022
68c6a65
Fix function api doc description.
CatStrategist Feb 23, 2022
49c8095
Fix variable names.
CatStrategist Feb 23, 2022
a84b730
Change Tab listener for old lists and add tests.
CatStrategist Feb 24, 2022
8c71916
Add checkbox in documentlist manual test to display borders.
CatStrategist Feb 24, 2022
83101fa
Apply review comment.
niegowski Feb 24, 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
27 changes: 15 additions & 12 deletions packages/ckeditor5-code-block/src/codeblockediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,18 @@ export default class CodeBlockEditing extends Plugin {
editor.commands.add( 'indentCodeBlock', new IndentCodeBlockCommand( editor ) );
editor.commands.add( 'outdentCodeBlock', new OutdentCodeBlockCommand( editor ) );

const getCommandExecuter = commandName => {
return ( data, cancel ) => {
const command = this.editor.commands.get( commandName );
this.listenTo( view.document, 'tab', ( evt, data ) => {
const commandName = data.shiftKey ? 'outdentCodeBlock' : 'indentCodeBlock';
const command = editor.commands.get( commandName );

if ( command.isEnabled ) {
this.editor.execute( commandName );
cancel();
}
};
};
if ( command.isEnabled ) {
editor.execute( commandName );

editor.keystrokes.set( 'Tab', getCommandExecuter( 'indentCodeBlock' ) );
editor.keystrokes.set( 'Shift+Tab', getCommandExecuter( 'outdentCodeBlock' ) );
data.stopPropagation();
data.preventDefault();
evt.stop();
}
}, { context: 'pre' } );

schema.register( 'codeBlock', {
allowWhere: '$block',
Expand Down Expand Up @@ -220,7 +219,11 @@ export default class CodeBlockEditing extends Plugin {
const outdent = commands.get( 'outdent' );

if ( indent ) {
indent.registerChildCommand( commands.get( 'indentCodeBlock' ) );
// Priority is highest due to integration with `IndentList` command of `List` plugin.
// If selection is in a code block we give priority to it. This way list item cannot be indented
// but if we would give priority to indenting list item then user would have to indent list item
// as much as possible and only then he could indent code block.
indent.registerChildCommand( commands.get( 'indentCodeBlock' ), { priority: 'highest' } );
}

if ( outdent ) {
Expand Down
136 changes: 135 additions & 1 deletion packages/ckeditor5-code-block/tests/codeblockediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ describe( 'CodeBlockEditing', () => {
} );

it( 'should execute outdentCodeBlock command on Shift+Tab keystroke', () => {
domEvtDataStub.keyCode += getCode( 'Shift' );
niegowski marked this conversation as resolved.
Show resolved Hide resolved
domEvtDataStub.shiftKey = true;

setModelData( model, '<codeBlock language="plaintext">[]foo</codeBlock>' );

Expand Down Expand Up @@ -223,6 +223,140 @@ describe( 'CodeBlockEditing', () => {
sinon.assert.notCalled( domEvtDataStub.preventDefault );
sinon.assert.notCalled( domEvtDataStub.stopPropagation );
} );

it( 'should not call indent block command when outside `pre` context', () => {
const indentBlockCommand = editor.commands.get( 'indentCodeBlock' );
const indentBlockCommandSpy = sinon.spy( indentBlockCommand, 'execute' );

setModelData( model,
'<paragraph>[]foo</paragraph>',
'<codeBlock language="plaintext">bar</codeBlock>'
);

editor.editing.view.document.fire( 'keydown', domEvtDataStub );

sinon.assert.notCalled( indentBlockCommandSpy );
sinon.assert.notCalled( domEvtDataStub.preventDefault );
sinon.assert.notCalled( domEvtDataStub.stopPropagation );
} );

it( 'should not call outdent block command when outside `pre` context', () => {
const outdentBlockCommand = editor.commands.get( 'outdentCodeBlock' );
const outdentBlockCommandSpy = sinon.spy( outdentBlockCommand, 'execute' );

domEvtDataStub.shiftKey = true;

setModelData( model,
'<paragraph>[]foo</paragraph>',
'<codeBlock language="plaintext">bar</codeBlock>'
);

editor.editing.view.document.fire( 'keydown', domEvtDataStub );

sinon.assert.notCalled( outdentBlockCommandSpy );
sinon.assert.notCalled( domEvtDataStub.preventDefault );
sinon.assert.notCalled( domEvtDataStub.stopPropagation );
} );

it( 'should not indent on tab key when tab event was captured by listener with higher priority', () => {
setModelData( model, '<codeBlock language="plaintext">[]foo</codeBlock>' );

const onTabPress = ( bubblingEventInfo, domEventData ) => {
domEventData.preventDefault();
domEventData.stopPropagation();
bubblingEventInfo.stop();
};

const onTabPressSpy = sinon.spy( onTabPress );

editor.editing.view.document.on( 'tab', onTabPressSpy, { context: 'pre', priority: 'highest' } );

editor.editing.view.document.fire( 'keydown', domEvtDataStub );

sinon.assert.notCalled( editor.execute );
sinon.assert.calledOnce( domEvtDataStub.preventDefault );
sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
sinon.assert.calledOnce( onTabPressSpy );
} );

it( 'should not be stopped by a listener with lower priority', () => {
setModelData( model, '<codeBlock language="plaintext">[]foo</codeBlock>' );

const onTabPress = ( bubblingEventInfo, domEventData ) => {
domEventData.preventDefault();
domEventData.stopPropagation();
bubblingEventInfo.stop();
};

const onTabPressSpy = sinon.spy( onTabPress );

editor.editing.view.document.on( 'tab', onTabPressSpy, { context: 'pre', priority: 'low' } );

editor.editing.view.document.fire( 'keydown', domEvtDataStub );

sinon.assert.calledOnce( editor.execute );
sinon.assert.calledWithExactly( editor.execute, 'indentCodeBlock' );
sinon.assert.calledOnce( domEvtDataStub.preventDefault );
sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
sinon.assert.notCalled( onTabPressSpy );
} );

it( 'should not outdent on tab key when tab event was captured by listener with higher priority', () => {
setModelData( model, '<codeBlock language="plaintext">[]foo</codeBlock>' );

model.change( writer => {
// <codeBlock language="plaintext"> foo[]</codeBlock>
writer.insertText( ' ', model.document.getRoot().getChild( 0 ), 0 );
} );

domEvtDataStub.shiftKey = true;

const onTabPress = ( bubblingEventInfo, domEventData ) => {
domEventData.preventDefault();
domEventData.stopPropagation();
bubblingEventInfo.stop();
};

const onTabPressSpy = sinon.spy( onTabPress );

editor.editing.view.document.on( 'tab', onTabPressSpy, { context: 'pre', priority: 'highest' } );

editor.editing.view.document.fire( 'keydown', domEvtDataStub );

sinon.assert.notCalled( editor.execute );
sinon.assert.calledOnce( domEvtDataStub.preventDefault );
sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
sinon.assert.calledOnce( onTabPressSpy );
} );

it( 'outdent should not be stopped by a listener with lower priority', () => {
setModelData( model, '<codeBlock language="plaintext">[]foo</codeBlock>' );

model.change( writer => {
// <codeBlock language="plaintext"> []foo</codeBlock>
writer.insertText( ' ', model.document.getRoot().getChild( 0 ) );
} );

domEvtDataStub.shiftKey = true;

const onTabPress = ( bubblingEventInfo, domEventData ) => {
domEventData.preventDefault();
domEventData.stopPropagation();
bubblingEventInfo.stop();
};

const onTabPressSpy = sinon.spy( onTabPress );

editor.editing.view.document.on( 'tab', onTabPressSpy, { context: 'pre', priority: 'lowest' } );

editor.editing.view.document.fire( 'keydown', domEvtDataStub );

sinon.assert.calledOnce( editor.execute );
sinon.assert.calledWithExactly( editor.execute, 'outdentCodeBlock' );
sinon.assert.calledOnce( domEvtDataStub.preventDefault );
sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
sinon.assert.notCalled( onTabPressSpy );
} );
} );

describe( 'enter key handling', () => {
Expand Down
6 changes: 5 additions & 1 deletion packages/ckeditor5-core/tests/multicommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@
*/

import MultiCommand from '../src/multicommand';
import ModelTestEditor from './_utils/modeltesteditor';
import Command from '../src/command';

import ModelTestEditor from './_utils/modeltesteditor';
import testUtils from './_utils/utils';

describe( 'MultiCommand', () => {
let editor, multiCommand;

testUtils.createSinonSandbox();

beforeEach( () => {
return ModelTestEditor
.create()
Expand Down
68 changes: 68 additions & 0 deletions packages/ckeditor5-engine/src/view/observer/tabobserver.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* @license Copyright (c) 2003-2022, CKSource Holding sp. z o.o. All rights reserved.
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/**
* @module engine/view/observer/tabobserver
*/

import Observer from './observer';
import BubblingEventInfo from './bubblingeventinfo';

import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';

/**
* Tab observer introduces the {@link module:engine/view/document~Document#event:tab `Document#tab`} event.
*
* Note that because {@link module:engine/view/observer/tabobserver~TabObserver} is attached by the
* {@link module:engine/view/view~View} this event is available by default.
*
* @extends module:engine/view/observer/observer~Observer
*/
export default class TabObserver extends Observer {
/**
* @inheritDoc
*/
constructor( view ) {
super( view );

const doc = this.document;

doc.on( 'keydown', ( evt, data ) => {
if (
!this.isEnabled ||
data.keyCode != keyCodes.tab ||
data.ctrlKey
) {
return;
}

const event = new BubblingEventInfo( doc, 'tab', doc.selection.getFirstRange() );

doc.fire( event, data );

if ( event.stop.called ) {
evt.stop();
}
} );
}

/**
* @inheritDoc
*/
observe() {}
}

/**
* Event fired when the user presses a tab key.
*
* Introduced by {@link module:engine/view/observer/tabobserver~TabObserver}.
*
* Note that because {@link module:engine/view/observer/tabobserver~TabObserver} is attached by the
* {@link module:engine/view/view~View} this event is available by default.
*
* @event module:engine/view/document~Document#event:tab
*
* @param {module:engine/view/observer/domeventdata~DomEventData} data
*/
4 changes: 4 additions & 0 deletions packages/ckeditor5-engine/src/view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import FocusObserver from './observer/focusobserver';
import CompositionObserver from './observer/compositionobserver';
import InputObserver from './observer/inputobserver';
import ArrowKeysObserver from './observer/arrowkeysobserver';
import TabObserver from './observer/tabobserver';

import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
import mix from '@ckeditor/ckeditor5-utils/src/mix';
Expand Down Expand Up @@ -54,6 +55,8 @@ import env from '@ckeditor/ckeditor5-utils/src/env';
* * {@link module:engine/view/observer/keyobserver~KeyObserver},
* * {@link module:engine/view/observer/fakeselectionobserver~FakeSelectionObserver}.
* * {@link module:engine/view/observer/compositionobserver~CompositionObserver}.
* * {@link module:engine/view/observer/arrowkeysobserver~ArrowKeysObserver}.
* * {@link module:engine/view/observer/tabobserver~TabObserver}.
*
* This class also {@link module:engine/view/view~View#attachDomRoot binds the DOM and the view elements}.
*
Expand Down Expand Up @@ -186,6 +189,7 @@ export default class View {
this.addObserver( FakeSelectionObserver );
this.addObserver( CompositionObserver );
this.addObserver( ArrowKeysObserver );
this.addObserver( TabObserver );

if ( env.isAndroid ) {
this.addObserver( InputObserver );
Expand Down
Loading