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

Implemented backspace and delete handling in and around document lists #11201

Merged
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
4471565
WiP.
niegowski Dec 30, 2021
d2cded2
Prototype of DocumentListMergeCommand.
niegowski Jan 5, 2022
2bc35ac
Merge branch 'ck/10812-document-list-editing' into ck/10878-document-…
niegowski Jan 10, 2022
906dcc3
Cleaning.
niegowski Jan 10, 2022
cdcb9e9
Refactor.
niegowski Jan 11, 2022
a9f9754
Finish is enabled for mergelistcommand
CatStrategist Jan 12, 2022
4f542fe
Merge branch 'ck/10812-document-list-editing' into ck/10878-document-…
CatStrategist Jan 13, 2022
76a5e18
Merge branch 'ck/10812-document-list-editing' into ck/10878-document-…
CatStrategist Jan 13, 2022
c341ec9
Backspace handling.
niegowski Jan 13, 2022
b8c7489
Code cleaning.
niegowski Jan 14, 2022
48ddf76
Backspace handling for non-collapsed selections.
oleq Jan 18, 2022
b50aa13
Add tests to stringifyList()
CatStrategist Jan 18, 2022
f81916a
Simplified DocumentListMergeCommand.
oleq Jan 19, 2022
79a5a62
Tests: Settled on expected output in corner cases when merging list i…
oleq Jan 19, 2022
3b61938
Improvements to backspace handling in lists.
oleq Jan 19, 2022
e78e518
Started forward delete integration in document lists.
oleq Jan 19, 2022
96064ae
Support for forward delete.
niegowski Jan 20, 2022
8237e50
WiP.
niegowski Jan 20, 2022
32fd295
WiP.
niegowski Jan 24, 2022
9c3f0a0
Add tests to document list editing integrations
CatStrategist Jan 24, 2022
03b1406
Code refactoring: split DocumentListEditing integration tests into se…
oleq Jan 25, 2022
1f1173b
Code refactoring in delete/backspace tests.
oleq Jan 25, 2022
7b908fa
Skip DocumentListMergeCommand tests for now. They need refactoring an…
oleq Jan 25, 2022
dd4d81a
Delete and backspace actions in DocumentListEditing should announce t…
oleq Jan 26, 2022
a24effc
WIP.
oleq Jan 27, 2022
253f68c
WiP.
niegowski Jan 27, 2022
c7b3a7f
Code refactoring.
oleq Jan 27, 2022
9adc93e
Allowed selected objects and nested elements to be passed into modelL…
oleq Jan 27, 2022
12f5cfd
Added block and inline widgets to the list mocking manual test.
oleq Jan 27, 2022
0876914
WIP: Added block&inline widget tests to the DocumentListEditing backs…
oleq Jan 27, 2022
8e3e480
Implemented delete/backspace handling in document lists around block …
oleq Feb 1, 2022
1656a9f
Added missing tests for document list integration with backspace/dele…
oleq Feb 1, 2022
519a807
Code refactoring.
oleq Feb 1, 2022
8d3e2fc
Merge branch 'ck/10812-document-list-editing' into ck/10878-document-…
oleq Feb 1, 2022
ac426c1
Code refactoring in tests
oleq Feb 1, 2022
8e511a0
Added tests for DocumentListMerge command. Docs.
oleq Feb 2, 2022
39edfae
Code refactoring.
oleq Feb 2, 2022
62ce46e
Code refactoring.
oleq Feb 2, 2022
1765012
Tests: Improved the document list mocking test.
oleq Feb 2, 2022
fb02b68
Removed obsolete param from parse() document lists dev util.
oleq Feb 2, 2022
635ddbf
Code refactoring.
oleq Feb 2, 2022
f6a19d7
Made it possible to pass an element instance to the stringifyList() h…
oleq Feb 2, 2022
15a2bf9
Internal: Fixed license headers.
oleq Feb 2, 2022
2f096f0
Docs.
oleq Feb 2, 2022
03a59d9
Code refactoring in tests
oleq Feb 2, 2022
242be47
Code refactoring in tests
oleq Feb 2, 2022
d30f8e5
Apply suggestions from code review.
niegowski Feb 2, 2022
b9cedf5
Code refactoring.
oleq Feb 2, 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
1 change: 1 addition & 0 deletions packages/ckeditor5-list/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"@ckeditor/ckeditor5-typing": "^31.1.0",
"@ckeditor/ckeditor5-undo": "^31.1.0",
"@ckeditor/ckeditor5-utils": "^31.1.0",
"@ckeditor/ckeditor5-widget": "^31.1.0",
"webpack": "^5.58.1",
"webpack-cli": "^4.9.0"
},
Expand Down
237 changes: 181 additions & 56 deletions packages/ckeditor5-list/src/documentlist/documentlistediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { CKEditorError } from 'ckeditor5/src/utils';

import DocumentListIndentCommand from './documentlistindentcommand';
import DocumentListCommand from './documentlistcommand';
import DocumentListMergeCommand, { getSelectedBlockObject } from './documentlistmergecommand';
import DocumentListSplitCommand from './documentlistsplitcommand';
import {
listItemDowncastConverter,
Expand All @@ -31,9 +32,10 @@ import {
import {
getAllListItemBlocks,
isFirstBlockOfListItem,
isLastBlockOfListItem
isLastBlockOfListItem,
isSingleListItem
} from './utils/model';
import { iterateSiblingListBlocks } from './utils/listwalker';
import ListWalker, { iterateSiblingListBlocks } from './utils/listwalker';

import '../../theme/documentlist.css';

Expand Down Expand Up @@ -63,8 +65,6 @@ export default class DocumentListEditing extends Plugin {
init() {
const editor = this.editor;
const model = editor.model;
const commands = editor.commands;
const enterCommand = commands.get( 'enter' );

if ( editor.plugins.has( 'ListEditing' ) ) {
/**
Expand All @@ -84,18 +84,160 @@ export default class DocumentListEditing extends Plugin {

model.on( 'insertContent', createModelIndentPasteFixer( model ), { priority: 'high' } );

this._setupConversion();

// Register commands.
editor.commands.add( 'numberedList', new DocumentListCommand( editor, 'numbered' ) );
editor.commands.add( 'bulletedList', new DocumentListCommand( editor, 'bulleted' ) );

editor.commands.add( 'indentList', new DocumentListIndentCommand( editor, 'forward' ) );
editor.commands.add( 'outdentList', new DocumentListIndentCommand( editor, 'backward' ) );

editor.commands.add( 'mergeListItemBackward', new DocumentListMergeCommand( editor, 'backward' ) );
editor.commands.add( 'mergeListItemForward', new DocumentListMergeCommand( editor, 'forward' ) );

editor.commands.add( 'splitListItemBefore', new DocumentListSplitCommand( editor, 'before' ) );
editor.commands.add( 'splitListItemAfter', new DocumentListSplitCommand( editor, 'after' ) );

this._setupConversion();
this._setupDeleteIntegration();
this._setupEnterIntegration();
}

/**
* @inheritDoc
*/
afterInit() {
const editor = this.editor;
const commands = editor.commands;
const indent = commands.get( 'indent' );
const outdent = commands.get( 'outdent' );

if ( indent ) {
indent.registerChildCommand( commands.get( 'indentList' ) );
}

if ( outdent ) {
outdent.registerChildCommand( commands.get( 'outdentList' ) );
}
}

/**
* Registers the conversion helpers for the document-list feature.
* @private
*/
_setupConversion() {
const editor = this.editor;
const model = editor.model;
const attributes = [ 'listItemId', 'listType', 'listIndent' ];

editor.conversion.for( 'upcast' )
.elementToElement( { view: 'li', model: 'paragraph' } )
.add( dispatcher => {
dispatcher.on( 'element:li', listItemUpcastConverter() );
dispatcher.on( 'element:ul', listUpcastCleanList(), { priority: 'high' } );
dispatcher.on( 'element:ol', listUpcastCleanList(), { priority: 'high' } );
} );

editor.conversion.for( 'editingDowncast' ).add( dispatcher => downcastConverters( dispatcher ) );
editor.conversion.for( 'dataDowncast' ).add( dispatcher => downcastConverters( dispatcher, { dataPipeline: true } ) );

function downcastConverters( dispatcher, options = {} ) {
dispatcher.on( 'insert:paragraph', listItemParagraphDowncastConverter( attributes, model, options ), { priority: 'high' } );

for ( const attributeName of attributes ) {
dispatcher.on( `attribute:${ attributeName }`, listItemDowncastConverter( attributes, model ) );
}
}

editor.data.mapper.registerViewToModelLength( 'li', listItemViewToModelLengthMapper( editor.data.mapper, model.schema ) );
this.listenTo( model.document, 'change:data', reconvertItemsOnDataChange( model, editor.editing ) );
}

/**
* Attaches the listener to the {@link module:engine/view/document~Document#event:delete} event and handles backspace/delete
* keys in and around document lists.
*
* @private
*/
_setupDeleteIntegration() {
const editor = this.editor;
const mergeBackwardCommand = editor.commands.get( 'mergeListItemBackward' );
const mergeForwardCommand = editor.commands.get( 'mergeListItemForward' );

this.listenTo( editor.editing.view.document, 'delete', ( evt, data ) => {
const selection = editor.model.document.selection;

editor.model.change( () => {
const firstPosition = selection.getFirstPosition();

if ( selection.isCollapsed && data.direction == 'backward' ) {
if ( !firstPosition.isAtStart ) {
return;
}

const positionParent = firstPosition.parent;

if ( !positionParent.hasAttribute( 'listItemId' ) ) {
return;
}

const previousBlock = ListWalker.first( positionParent, { sameIndent: true, sameItemType: true } );

// Outdent the first block of a first list item.
if ( !previousBlock && positionParent.getAttribute( 'listIndent' ) === 0 ) {
if ( !isLastBlockOfListItem( positionParent ) ) {
editor.execute( 'splitListItemAfter' );
}

editor.execute( 'outdentList' );
}
// Merge block with previous one (on the block level or on the content level).
else {
if ( !mergeBackwardCommand.isEnabled ) {
return;
}

mergeBackwardCommand.execute( {
shouldMergeOnBlocksContentLevel: shouldMergeOnBlocksContentLevel( editor.model, 'backward' )
} );
}

data.preventDefault();
evt.stop();
}
// Non-collapsed selection or forward delete.
else {
// Collapsed selection should trigger forward merging only if at the end of a block.
if ( selection.isCollapsed && !selection.getLastPosition().isAtEnd ) {
return;
}

if ( !mergeForwardCommand.isEnabled ) {
return;
}

mergeForwardCommand.execute( {
shouldMergeOnBlocksContentLevel: shouldMergeOnBlocksContentLevel( editor.model, 'forward' )
} );

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

/**
* Attaches a listener to the {@link module:engine/view/document~Document#event:enter} event and handles enter key press
* in document lists.
*
* @private
*/
_setupEnterIntegration() {
const editor = this.editor;
const model = editor.model;
const commands = editor.commands;
const enterCommand = commands.get( 'enter' );

// Overwrite the default Enter key behavior: outdent or split the list in certain cases.
this.listenTo( editor.editing.view.document, 'enter', ( evt, data ) => {
const doc = model.document;
Expand Down Expand Up @@ -160,56 +302,6 @@ export default class DocumentListEditing extends Plugin {
}
} );
}

/**
* @inheritDoc
*/
afterInit() {
const editor = this.editor;
const commands = editor.commands;
const indent = commands.get( 'indent' );
const outdent = commands.get( 'outdent' );

if ( indent ) {
indent.registerChildCommand( commands.get( 'indentList' ) );
}

if ( outdent ) {
outdent.registerChildCommand( commands.get( 'outdentList' ) );
}
}

/**
* Registers the conversion helpers for the document-list feature.
* @private
*/
_setupConversion() {
const editor = this.editor;
const model = editor.model;
const attributes = [ 'listItemId', 'listType', 'listIndent' ];

editor.conversion.for( 'upcast' )
.elementToElement( { view: 'li', model: 'paragraph' } )
.add( dispatcher => {
dispatcher.on( 'element:li', listItemUpcastConverter() );
dispatcher.on( 'element:ul', listUpcastCleanList(), { priority: 'high' } );
dispatcher.on( 'element:ol', listUpcastCleanList(), { priority: 'high' } );
} );

editor.conversion.for( 'editingDowncast' ).add( dispatcher => downcastConverters( dispatcher ) );
editor.conversion.for( 'dataDowncast' ).add( dispatcher => downcastConverters( dispatcher, { dataPipeline: true } ) );

function downcastConverters( dispatcher, options = {} ) {
dispatcher.on( 'insert:paragraph', listItemParagraphDowncastConverter( attributes, model, options ), { priority: 'high' } );

for ( const attributeName of attributes ) {
dispatcher.on( `attribute:${ attributeName }`, listItemDowncastConverter( attributes, model ) );
}
}

editor.data.mapper.registerViewToModelLength( 'li', listItemViewToModelLengthMapper( editor.data.mapper, model.schema ) );
this.listenTo( model.document, 'change:data', reconvertItemsOnDataChange( model, editor.editing ) );
}
}

// Post-fixer that reacts to changes on document and fixes incorrect model states (invalid `listItemId` and `listIndent` values).
Expand Down Expand Up @@ -367,3 +459,36 @@ function createModelIndentPasteFixer( model ) {
} );
};
}

// Decided whether the merge should be accompanied by the model's `deleteContent()`, for instance to get rid of the inline
niegowski marked this conversation as resolved.
Show resolved Hide resolved
// content in the selection or take advantage of the heuristics in `deleteContent()` that helps convert lists into paragraphs
// in certain cases.
//
// @param {module:engine/model/model~Model} model
// @param {'backward'|'forward'} direction
// @returns {Boolean}
function shouldMergeOnBlocksContentLevel( model, direction ) {
const selection = model.document.selection;

if ( !selection.isCollapsed ) {
return !getSelectedBlockObject( model );
}

if ( direction === 'forward' ) {
return true;
}

const firstPosition = selection.getFirstPosition();
const positionParent = firstPosition.parent;
const previousSibling = positionParent.previousSibling;

if ( model.schema.isObject( previousSibling ) ) {
return false;
}

if ( previousSibling.isEmpty ) {
return true;
}

return isSingleListItem( [ positionParent, previousSibling ] );
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
indentBlocks,
isFirstBlockOfListItem,
isSingleListItem,
outdentBlocks,
outdentBlocksWithMerge,
sortBlocks,
splitListItemBefore
} from './utils/model';
Expand Down Expand Up @@ -82,7 +82,7 @@ export default class DocumentListIndentCommand extends Command {
if ( this._direction == 'forward' ) {
changedBlocks.push( ...indentBlocks( blocks, writer, { expand: true } ) );
} else {
changedBlocks.push( ...outdentBlocks( blocks, writer, { expand: true } ) );
changedBlocks.push( ...outdentBlocksWithMerge( blocks, writer, { expand: true } ) );
niegowski marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
Loading