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

Introducing list.multiBlock config option #14781

Merged
merged 21 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion packages/ckeditor5-engine/src/view/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ export default class Renderer extends ObservableMixin() {
//
// Converting live list to an array to make the list static.
const actualDomChildren = Array.from(
this.domConverter.mapViewToDom( viewElement )!.childNodes
domElement.childNodes
);
const expectedDomChildren = Array.from(
this.domConverter.viewChildrenToDom( viewElement, { withChildren: false } )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,7 @@ export default class DocumentListElementSupport extends Plugin {

const allowAttributes = viewElements.map( element => getHtmlAttributeName( element ) );

schema.extend( '$block', { allowAttributes } );
schema.extend( '$blockObject', { allowAttributes } );
schema.extend( '$container', { allowAttributes } );
schema.extend( '$listItem', { allowAttributes } );

conversion.for( 'upcast' ).add( dispatcher => {
dispatcher.on<UpcastElementEvent>(
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-list/src/documentlist/converters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ export function reconvertItemsOnDataChange(
return true;
}

if ( !item.is( 'element', 'paragraph' ) ) {
if ( !item.is( 'element', 'paragraph' ) && !item.is( 'element', 'listItem' ) ) {
return false;
}

Expand Down
14 changes: 10 additions & 4 deletions packages/ckeditor5-list/src/documentlist/documentlistcommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import {
ListItemUid,
sortBlocks,
getSelectedBlockObject,
isListItemBlock
isListItemBlock,
canBecomeSimpleListItem
} from './utils/model';

/**
Expand Down Expand Up @@ -75,7 +76,7 @@ export default class DocumentListCommand extends Command {
const selectedBlockObject = getSelectedBlockObject( model );

const blocks = Array.from( document.selection.getSelectedBlocks() )
.filter( block => model.schema.checkAttribute( block, 'listType' ) );
.filter( block => model.schema.checkAttribute( block, 'listType' ) || canBecomeSimpleListItem( block, model.schema ) );

// Whether we are turning off some items.
const turnOff = options.forceValue !== undefined ? !options.forceValue : this.value;
Expand All @@ -92,7 +93,7 @@ export default class DocumentListCommand extends Command {
changedBlocks.push( ...splitListItemBefore( itemBlocks[ 1 ], writer ) );
}

// Convert list blocks to plain blocks.
// Strip list attributes.
changedBlocks.push( ...removeListAttributes( blocks, writer ) );

// Outdent items following the selected list item.
Expand All @@ -117,6 +118,11 @@ export default class DocumentListCommand extends Command {
for ( const block of blocks ) {
// Promote the given block to the list item.
if ( !block.hasAttribute( 'listType' ) ) {
// Rename block to a simple list item if this option is enabled.
if ( !block.is( 'element', 'listItem' ) && canBecomeSimpleListItem( block, model.schema ) ) {
writer.rename( block, 'listItem' );
}

writer.setAttributes( {
listIndent: 0,
listItemId: ListItemUid.next(),
Expand Down Expand Up @@ -194,7 +200,7 @@ export default class DocumentListCommand extends Command {
}

for ( const block of blocks ) {
if ( schema.checkAttribute( block, 'listType' ) ) {
if ( schema.checkAttribute( block, 'listType' ) || canBecomeSimpleListItem( block, schema ) ) {
return true;
}
}
Expand Down
79 changes: 64 additions & 15 deletions packages/ckeditor5-list/src/documentlist/documentlistediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import {
Plugin,
type Editor,
type MultiCommand
} from 'ckeditor5/src/core';

Expand Down Expand Up @@ -108,12 +109,22 @@ export default class DocumentListEditing extends Plugin {
return [ Enter, Delete, DocumentListUtils ] as const;
}

/**
* @inheritDoc
*/
constructor( editor: Editor ) {
super( editor );

editor.config.define( 'list.multiBlock', true );
}

/**
* @inheritDoc
*/
public init(): void {
const editor = this.editor;
const model = editor.model;
const multiBlock = editor.config.get( 'list.multiBlock' );

if ( editor.plugins.has( 'ListEditing' ) ) {
/**
Expand All @@ -125,9 +136,18 @@ export default class DocumentListEditing extends Plugin {
throw new CKEditorError( 'document-list-feature-conflict', this, { conflictPlugin: 'ListEditing' } );
}

model.schema.extend( '$container', { allowAttributes: LIST_BASE_ATTRIBUTES } );
model.schema.extend( '$block', { allowAttributes: LIST_BASE_ATTRIBUTES } );
model.schema.extend( '$blockObject', { allowAttributes: LIST_BASE_ATTRIBUTES } );
model.schema.register( '$listItem', { allowAttributes: LIST_BASE_ATTRIBUTES } );

if ( multiBlock ) {
model.schema.extend( '$container', { allowAttributesOf: '$listItem' } );
model.schema.extend( '$block', { allowAttributesOf: '$listItem' } );
model.schema.extend( '$blockObject', { allowAttributesOf: '$listItem' } );
} else {
model.schema.register( 'listItem', {
inheritAllFrom: '$block',
allowAttributesOf: '$listItem'
} );
}

for ( const attribute of LIST_BASE_ATTRIBUTES ) {
model.schema.setAttributeProperties( attribute, {
Expand All @@ -142,12 +162,14 @@ export default class DocumentListEditing extends Plugin {
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' ) );

if ( multiBlock ) {
editor.commands.add( 'mergeListItemBackward', new DocumentListMergeCommand( editor, 'backward' ) );
editor.commands.add( 'mergeListItemForward', new DocumentListMergeCommand( editor, 'forward' ) );
}

this._setupDeleteIntegration();
this._setupEnterIntegration();
this._setupTabIntegration();
Expand Down Expand Up @@ -208,8 +230,8 @@ export default class DocumentListEditing extends Plugin {
*/
private _setupDeleteIntegration() {
const editor = this.editor;
const mergeBackwardCommand: DocumentListMergeCommand = editor.commands.get( 'mergeListItemBackward' )!;
const mergeForwardCommand: DocumentListMergeCommand = editor.commands.get( 'mergeListItemForward' )!;
const mergeBackwardCommand: DocumentListMergeCommand | undefined = editor.commands.get( 'mergeListItemBackward' );
const mergeForwardCommand: DocumentListMergeCommand | undefined = editor.commands.get( 'mergeListItemForward' );

this.listenTo<ViewDocumentDeleteEvent>( editor.editing.view.document, 'delete', ( evt, data ) => {
const selection = editor.model.document.selection;
Expand Down Expand Up @@ -248,7 +270,7 @@ export default class DocumentListEditing extends Plugin {
}
// Merge block with previous one (on the block level or on the content level).
else {
if ( !mergeBackwardCommand.isEnabled ) {
if ( !mergeBackwardCommand || !mergeBackwardCommand.isEnabled ) {
return;
}

Expand All @@ -267,7 +289,7 @@ export default class DocumentListEditing extends Plugin {
return;
}

if ( !mergeForwardCommand.isEnabled ) {
if ( !mergeForwardCommand || !mergeForwardCommand.isEnabled ) {
return;
}

Expand Down Expand Up @@ -390,13 +412,18 @@ export default class DocumentListEditing extends Plugin {
const editor = this.editor;
const model = editor.model;
const attributeNames = this.getListAttributeNames();
const multiBlock = editor.config.get( 'list.multiBlock' );
const elementName = multiBlock ? 'paragraph' : 'listItem';

editor.conversion.for( 'upcast' )
// Convert <li> to a generic paragraph so the content of <li> is always inside a block.
// Convert <li> to a generic paragraph (or listItem element) so the content of <li> is always inside a block.
// Setting the listType attribute to let other features (to-do list) know that this is part of a list item.
// This is also important to properly handle simple lists so that paragraphs inside a list item won't break the list item.
// <li> <-- converted to listItem
// <p></p> <-- should be also converted to listItem, so it won't split and replace the listItem generated from the above li.
.elementToElement( {
view: 'li',
model: ( viewElement, { writer } ) => writer.createElement( 'paragraph', { listType: '' } )
model: ( viewElement, { writer } ) => writer.createElement( elementName, { listType: '' } )
} )
// Convert paragraph to the list block (without list type defined yet).
// This is important to properly handle bogus paragraph and to-do lists.
Expand All @@ -407,7 +434,7 @@ export default class DocumentListEditing extends Plugin {
view: 'p',
model: ( viewElement, { writer } ) => {
if ( viewElement.parent && viewElement.parent.is( 'element', 'li' ) ) {
return writer.createElement( 'paragraph', { listType: '' } );
return writer.createElement( elementName, { listType: '' } );
}

return null;
Expand All @@ -420,9 +447,17 @@ export default class DocumentListEditing extends Plugin {
dispatcher.on<UpcastElementEvent>( 'element:ol', listUpcastCleanList(), { priority: 'high' } );
} );

if ( !multiBlock ) {
editor.conversion.for( 'downcast' )
.elementToElement( {
model: 'listItem',
view: 'p'
} );
}

editor.conversion.for( 'editingDowncast' )
.elementToElement( {
model: 'paragraph',
model: elementName,
view: bogusParagraphCreator( attributeNames ),
converterPriority: 'high'
} )
Expand All @@ -435,7 +470,7 @@ export default class DocumentListEditing extends Plugin {

editor.conversion.for( 'dataDowncast' )
.elementToElement( {
model: 'paragraph',
model: elementName,
view: bogusParagraphCreator( attributeNames, { dataPipeline: true } ),
converterPriority: 'high'
} )
Expand Down Expand Up @@ -649,6 +684,7 @@ function modelChangePostFixer(
) {
const changes = model.document.differ.getChanges();
const itemToListHead = new Map<ListElement, ListElement>();
const multiBlock = documentListEditing.editor.config.get( 'list.multiBlock' );

let applied = false;

Expand Down Expand Up @@ -693,6 +729,19 @@ function modelChangePostFixer(
findAndAddListHeadToMap( entry.range.start.getShiftedBy( 1 ), itemToListHead );
}
}

// Make sure that there is no left over listItem element without attributes or a block with list attributes that is not a listItem.
if ( !multiBlock && entry.type == 'attribute' && LIST_BASE_ATTRIBUTES.includes( entry.attributeKey ) ) {
const element = entry.range.start.nodeAfter!;

if ( entry.attributeNewValue === null && element && element.is( 'element', 'listItem' ) ) {
writer.rename( element, 'paragraph' );
applied = true;
} else if ( entry.attributeOldValue === null && element && element.is( 'element' ) && element.name != 'listItem' ) {
writer.rename( element, 'listItem' );
applied = true;
}
}
}

// Make sure that IDs are not shared by split list.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export default class DocumentListMergeCommand extends Command {
// Check if the element after it was in the same list item and adjust it if needed.
const nextSibling = lastElementAfterDelete.nextSibling;

changedBlocks.push( lastElementAfterDelete as any );
changedBlocks.push( lastElementAfterDelete as Element );

if ( nextSibling && nextSibling !== lastElement && nextSibling.getAttribute( 'listItemId' ) == lastElementId ) {
changedBlocks.push( ...mergeListItemBefore( nextSibling, lastElementAfterDelete, writer ) );
Expand Down
24 changes: 23 additions & 1 deletion packages/ckeditor5-list/src/documentlist/utils/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,14 @@ export function removeListAttributes(
): Array<Element> {
blocks = toArray( blocks );

// Convert simple list items to plain paragraphs.
for ( const block of blocks ) {
if ( block.is( 'element', 'listItem' ) ) {
writer.rename( block, 'paragraph' );
}
}

// Remove list attributes.
for ( const block of blocks ) {
for ( const attributeKey of block.getAttributeKeys() ) {
if ( attributeKey.startsWith( 'list' ) ) {
Expand Down Expand Up @@ -546,7 +554,21 @@ export function getSelectedBlockObject( model: Model ): Element | null {
return null;
}

// Merges a given block to the given parent block if parent is a list item and there is no more blocks in the same item.
/**
* Checks whether the given block can be replaced by a listItem.
*
* Note that this is possible only when multiBlock = false option is set in feature config.
*
* @param block A block to be tested.
* @param schema The schema of the document.
*/
export function canBecomeSimpleListItem( block: Element, schema: Schema ): boolean {
return schema.checkChild( block.parent as Element, 'listItem' ) && schema.checkChild( block, '$text' ) && !schema.isObject( block );
}

/**
* Merges a given block to the given parent block if parent is a list item and there is no more blocks in the same item.
*/
function mergeListItemIfNotLast(
block: ListElement,
parentBlock: ListElement,
Expand Down
11 changes: 11 additions & 0 deletions packages/ckeditor5-list/src/documentlist/utils/postfixers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,17 @@ export function fixListItemIds(

seenIds.add( listItemId );

// Make sure that all items in a simple list have unique IDs.
if ( node.is( 'element', 'listItem' ) ) {
if ( node.getAttribute( 'listItemId' ) != listItemId ) {
writer.setAttribute( 'listItemId', listItemId, node );

applied = true;
}

continue;
}

for ( const block of getListItemBlocks( node, { direction: 'forward' } ) ) {
visited.add( block );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,10 @@ export default class DocumentListPropertiesEditing extends Plugin {
constructor( editor: Editor ) {
super( editor );

editor.config.define( 'list', {
properties: {
styles: true,
startIndex: false,
reversed: false
}
editor.config.define( 'list.properties', {
styles: true,
startIndex: false,
reversed: false
} );
}

Expand All @@ -91,9 +89,7 @@ export default class DocumentListPropertiesEditing extends Plugin {
for ( const strategy of strategies ) {
strategy.addCommand( editor );

model.schema.extend( '$container', { allowAttributes: strategy.attributeName } );
model.schema.extend( '$block', { allowAttributes: strategy.attributeName } );
model.schema.extend( '$blockObject', { allowAttributes: strategy.attributeName } );
model.schema.extend( '$listItem', { allowAttributes: strategy.attributeName } );

// Register downcast strategy.
documentListEditing.registerDowncastStrategy( {
Expand Down
7 changes: 7 additions & 0 deletions packages/ckeditor5-list/src/listconfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ export interface ListConfig {
* Read more in {@link module:list/listconfig~ListPropertiesConfig}.
*/
properties?: ListPropertiesConfig;

/**
* Allows multiple blocks in single list item.
arkflpc marked this conversation as resolved.
Show resolved Hide resolved
*
* @default true
*/
multiBlock?: boolean;
}

/**
Expand Down
Loading