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 2 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
25 changes: 12 additions & 13 deletions packages/ckeditor5-list/src/list/listediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,19 +171,18 @@ export default class ListEditing extends Plugin {
evt.stop();
}, { context: 'li' } );

const getCommandExecuter = commandName => {
return ( data, cancel ) => {
const command = this.editor.commands.get( commandName );

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

editor.keystrokes.set( 'Tab', getCommandExecuter( 'indentList' ) );
editor.keystrokes.set( 'Shift+Tab', getCommandExecuter( 'outdentList' ) );
this.listenTo( editor.editing.view.document, 'tab', ( evt, data ) => {
const commandName = data.shiftKey ? 'outdentList' : 'indentList';
const command = this.editor.commands.get( commandName );

if ( command.isEnabled ) {
editor.execute( commandName );

data.stopPropagation();
data.preventDefault();
evt.stop();
}
}, { context: 'li' } );
}

/**
Expand Down
103 changes: 101 additions & 2 deletions packages/ckeditor5-list/tests/list/listediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ import IndentEditing from '@ckeditor/ckeditor5-indent/src/indentediting';

import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard';
import TableEditing from '@ckeditor/ckeditor5-table/src/tableediting';
import TableKeyboard from '@ckeditor/ckeditor5-table/src/tablekeyboard';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import { modelTable } from '@ckeditor/ckeditor5-table/tests/_utils/utils';

describe( 'ListEditing', () => {
let editor, model, modelDoc, modelRoot, view, viewDoc, viewRoot;
Expand All @@ -34,7 +36,7 @@ describe( 'ListEditing', () => {
return VirtualTestEditor
.create( {
plugins: [ Paragraph, IndentEditing, ClipboardPipeline, BoldEditing, ListEditing, UndoEditing, BlockQuoteEditing,
TableEditing ]
TableEditing, TableKeyboard ]
} )
.then( newEditor => {
editor = newEditor;
Expand Down Expand Up @@ -375,7 +377,7 @@ describe( 'ListEditing', () => {
} );

it( 'should execute outdentList command on Shift+Tab keystroke', () => {
domEvtDataStub.keyCode += getCode( 'Shift' );
domEvtDataStub.shiftKey = true;

setModelData(
model,
Expand Down Expand Up @@ -416,6 +418,103 @@ describe( 'ListEditing', () => {
sinon.assert.notCalled( domEvtDataStub.preventDefault );
sinon.assert.notCalled( domEvtDataStub.stopPropagation );
} );

it( 'should execute list indent command when in a li context and nested in an element that also listens to Tab', () => {
const listInputModel = '<listItem listIndent="0" listType="bulleted">foo</listItem>' +
'<listItem listIndent="0" listType="bulleted">[]bar</listItem>';

const listOutputModel = '<listItem listIndent="0" listType="bulleted">foo</listItem>' +
'<listItem listIndent="1" listType="bulleted">[]bar</listItem>';

const input = modelTable( [
[ listInputModel, 'bar' ]
] );

const output = modelTable( [
[ listOutputModel, 'bar' ]
] );

setModelData( model, input );

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

sinon.assert.calledWithExactly( editor.execute, 'indentList' );
sinon.assert.calledOnce( domEvtDataStub.preventDefault );
sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
expect( getModelData( model ) ).to.equalMarkup( output );
} );

it( 'should execute list outdent command when in a li context and nested in an element that also listens to Tab', () => {
const listInputModel = '<listItem listIndent="0" listType="bulleted">foo</listItem>' +
'<listItem listIndent="1" listType="bulleted">[]bar</listItem>';

const listOutputModel = '<listItem listIndent="0" listType="bulleted">foo</listItem>' +
'<listItem listIndent="0" listType="bulleted">[]bar</listItem>';

const input = modelTable( [
[ listInputModel, 'bar' ]
] );

const output = modelTable( [
[ listOutputModel, 'bar' ]
] );

setModelData( model, input );

domEvtDataStub.shiftKey = true;

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

sinon.assert.calledWithExactly( editor.execute, 'outdentList' );
sinon.assert.calledOnce( domEvtDataStub.preventDefault );
sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
expect( getModelData( model ) ).to.equalMarkup( output );
} );

it( 'should not capture event when list cannot be indented and allow other listeners to capture it', () => {
const listInputModel = '<listItem listIndent="0" listType="bulleted">bar[]</listItem>';
const listOutputModel = '<listItem listIndent="0" listType="bulleted">bar</listItem>';

const input = modelTable( [
[ 'foo', listInputModel ]
] );

const output = modelTable( [
[ 'foo', listOutputModel ],
[ '[]', '' ]
] );

setModelData( model, input );

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

sinon.assert.neverCalledWith( editor.execute, 'indentList' );
sinon.assert.neverCalledWith( editor.execute, 'outdentList' );
sinon.assert.calledOnce( domEvtDataStub.preventDefault );
sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
expect( getModelData( model ) ).to.equalMarkup( output );
} );

it( 'should not capture event when not in a list and should allow other listeners to capture it', () => {
const input = modelTable( [
[ 'foo', 'bar[]' ]
] );

const output = modelTable( [
[ 'foo', 'bar' ],
[ '[]', '' ]
] );

setModelData( model, input );

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

sinon.assert.neverCalledWith( editor.execute, 'indentList' );
sinon.assert.neverCalledWith( editor.execute, 'outdentList' );
sinon.assert.calledOnce( domEvtDataStub.preventDefault );
sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
expect( getModelData( model ) ).to.equalMarkup( output );
} );
} );

describe( 'flat lists', () => {
Expand Down
13 changes: 11 additions & 2 deletions packages/ckeditor5-list/tests/manual/documentlist.html
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
<div class="options-container">
<input id="chbx-show-borders" type="checkbox" />
<label for="chbx-show-borders">Show borders</label>
</div>

<div id="editor">
a
<ul>
Expand Down Expand Up @@ -100,16 +105,20 @@ <h2>Heading 1</h2>
</div>

<style>
ul, ol {
.show-borders ul, ol {
niegowski marked this conversation as resolved.
Show resolved Hide resolved
border: 2px dashed blue;
padding-top: 5px;
padding-bottom: 5px;
margin: 5px;
}

li {
.show-borders li {
border: 2px dotted red;
padding: 5px;
margin: 5px;
}

.options-container {
margin-bottom: 16px;
}
</style>
6 changes: 6 additions & 0 deletions packages/ckeditor5-list/tests/manual/documentlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,9 @@ ClassicEditor
.catch( err => {
console.error( err.stack );
} );

const onShowBordersChange = () => {
document.body.classList.toggle( 'show-borders' );
};

document.getElementById( 'chbx-show-borders' ).addEventListener( 'change', onShowBordersChange );
2 changes: 2 additions & 0 deletions packages/ckeditor5-list/tests/manual/documentlist.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
## Document List

TODO

Border colors: Blue for `ul` and `ol`, red for `li`.