Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #29 from ckeditor/t/ckeditor5-engine/1213
Browse files Browse the repository at this point in the history 
Internal: Simplified autoparagraphing after engine view -> model conversion refactoring. Closes https://github.com/ckeditor/ckeditor5-engine/issues/1213.
  • Loading branch information
Piotr Jasiun authored Jan 31, 2018
2 parents 40a04b5 + 53a0acd commit b84b65d
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 135 deletions.
187 changes: 63 additions & 124 deletions src/paragraph.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import ParagraphCommand from './paragraphcommand';
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter';
import { SchemaContext } from '@ckeditor/ckeditor5-engine/src/model/schema';
import Position from '@ckeditor/ckeditor5-engine/src/model/position';
import Range from '@ckeditor/ckeditor5-engine/src/model/range';

/**
* The paragraph feature for the editor.
Expand All @@ -36,14 +39,6 @@ export default class Paragraph extends Plugin {
const data = editor.data;
const editing = editor.editing;

/**
* List of empty roots and batches that made them empty.
*
* @private
* @type {Map<module:engine/model/rootelement~RootElement,module:engine/model/batch~Batch>}
*/
this._rootsToFix = new Map();

editor.commands.add( 'paragraph', new ParagraphCommand( editor ) );

// Schema.
Expand All @@ -61,23 +56,61 @@ export default class Paragraph extends Plugin {

// Content autoparagraphing. --------------------------------------------------

// Step 1.
// "Second chance" converters for elements and texts which were not allowed in their original locations.
// They check if this element/text could be converted if it was in a paragraph.
// Forcefully converted items will be temporarily in an invalid context. It's going to be fixed in step 2.
// Handles elements not converted by plugins and checks if would be converted if
// we wraps them by a paragraph or changes them to a paragraph.
data.viewToModel.on( 'element', ( evt, data, conversionApi ) => {
// When element is already consumed by higher priority converters then do nothing.
if ( !conversionApi.consumable.test( data.viewItem, { name: data.viewItem.name } ) ) {
return;
}

// When element is paragraph-like lets try to change it into a paragraph.
if ( Paragraph.paragraphLikeElements.has( data.viewItem.name ) ) {
if ( data.viewItem.isEmpty ) {
return;
}

const paragraph = conversionApi.writer.createElement( 'paragraph' );

// Executed after converter added by a feature, but before "default" to-model-fragment converter.
data.viewToModel.on( 'element', convertAutoparagraphableItem, { priority: 'low' } );
// Executed after default text converter.
data.viewToModel.on( 'text', convertAutoparagraphableItem, { priority: 'lowest' } );
// Find allowed parent for paragraph that we are going to insert.
// If current parent does not allow to insert paragraph but one of the ancestors does
// then split nodes to allowed parent.
const splitResult = conversionApi.splitToAllowedParent( paragraph, data.cursorPosition );

// Step 2.
// After an item is "forced" to be converted by `convertAutoparagraphableItem`, we need to actually take
// care of adding the paragraph (assumed in `convertAutoparagraphableItem`) and wrap that item in it.
// When there is no split result it means that we can't insert paragraph in this position.
if ( !splitResult ) {
return;
}

// Insert paragraph in allowed position.
conversionApi.writer.insert( paragraph, splitResult.position );

// Convert children to paragraph.
const { modelRange } = conversionApi.convertChildren( data.viewItem, Position.createAt( paragraph ) );

// Executed after all converters (even default ones).
data.viewToModel.on( 'element', autoparagraphItems, { priority: 'lowest' } );
data.viewToModel.on( 'documentFragment', autoparagraphItems, { priority: 'lowest' } );
// Output range starts before paragraph but ends inside it after last child.
// This is because we want to keep siblings inside the same paragraph as long as it is possible.
// When next node won't be allowed in a paragraph it will split this paragraph anyway.
data.modelRange = new Range( Position.createBefore( paragraph ), modelRange.end );
data.cursorPosition = data.modelRange.end;

// When element is not paragraph-like lets try to wrap it by a paragraph.
} else if ( isParagraphable( data.viewItem, data.cursorPosition, conversionApi.schema ) ) {
data = Object.assign( data, wrapInParagraph( data.viewItem, data.cursorPosition, conversionApi ) );
}
}, { priority: 'low' } );

// Handles not converted text nodes and checks if would be converted if we wraps then by a paragraph.
data.viewToModel.on( 'text', ( evt, data, conversionApi ) => {
// When node is already converted then do nothing.
if ( data.modelRange ) {
return;
}

if ( isParagraphable( data.viewItem, data.cursorPosition, conversionApi.schema ) ) {
data = Object.assign( data, wrapInParagraph( data.viewItem, data.cursorPosition, conversionApi ) );
}
}, { priority: 'lowest' } );

// Empty roots autoparagraphing. -----------------------------------------------

Expand Down Expand Up @@ -162,117 +195,23 @@ Paragraph.paragraphLikeElements = new Set( [
'td'
] );

// This converter forces a conversion of a non-consumed view item, if that item would be allowed by schema and converted it if was
// inside a paragraph element. The converter checks whether conversion would be possible if there was a paragraph element
// between `data.input` item and its parent. If the conversion would be allowed, the converter adds `"paragraph"` to the
// context and fires conversion for `data.input` again.
function convertAutoparagraphableItem( evt, data, consumable, conversionApi ) {
// If the item wasn't consumed by some of the dedicated converters...
if ( !consumable.test( data.input, { name: data.input.name } ) ) {
return;
}

// But would be allowed if it was in a paragraph...
if ( !isParagraphable( data.input, data.context, conversionApi.schema, false ) ) {
return;
}

// Convert that item in a paragraph context.
data.context.push( 'paragraph' );
const item = conversionApi.convertItem( data.input, consumable, data );
data.context.pop();
function wrapInParagraph( input, position, conversionApi ) {
const paragraph = conversionApi.writer.createElement( 'paragraph' );

data.output = item;
conversionApi.writer.insert( paragraph, position );
return conversionApi.convertItem( input, Position.createAt( paragraph ) );
}

// This converter checks all children of an element or document fragment that has been converted and wraps
// children in a paragraph element if it is allowed by schema.
//
// Basically, after an item is "forced" to be converted by `convertAutoparagraphableItem`, we need to actually take
// care of adding the paragraph (assumed in `convertAutoparagraphableItem`) and wrap that item in it.
function autoparagraphItems( evt, data, consumable, conversionApi ) {
// Autoparagraph only if the element has been converted.
if ( !data.output ) {
return;
}

const isParagraphLike = Paragraph.paragraphLikeElements.has( data.input.name ) && !data.output.is( 'element' );

// Keep in mind that this converter is added to all elements and document fragments.
// This means that we have to make a smart decision in which elements (at what level) auto-paragraph should be inserted.
// There are three situations when it is correct to add paragraph:
// - we are converting a view document fragment: this means that we are at the top level of conversion and we should
// add paragraph elements for "bare" texts (unless converting in $clipboardHolder, but this is covered by schema),
// - we are converting an element that was converted to model element: this means that it will be represented in model
// and has added its context when converting children - we should add paragraph for those items that passed
// in `convertAutoparagraphableItem`, because it is correct for them to be autoparagraphed,
// - we are converting "paragraph-like" element, which children should always be autoparagraphed (if it is allowed by schema,
// so we won't end up with, i.e., paragraph inside paragraph, if paragraph was in paragraph-like element).
const shouldAutoparagraph =
( data.input.is( 'documentFragment' ) ) ||
( data.input.is( 'element' ) && data.output.is( 'element' ) ) ||
isParagraphLike;

if ( !shouldAutoparagraph ) {
return;
}

// Take care of proper context. This is important for `isParagraphable` checks.
const needsNewContext = data.output.is( 'element' );

if ( needsNewContext ) {
data.context.push( data.output );
}

// `paragraph` element that will wrap auto-paragraphable children.
let autoParagraph = null;

// Check children and wrap them in a `paragraph` element if they need to be wrapped.
// Be smart when wrapping children and put all auto-paragraphable siblings in one `paragraph` parent:
// foo<$text bold="true">bar</$text><paragraph>xxx</paragraph>baz --->
// <paragraph>foo<$text bold="true">bar</$text></paragraph><paragraph>xxx</paragraph><paragraph>baz</paragraph>
for ( let i = 0; i < data.output.childCount; i++ ) {
const child = data.output.getChild( i );

if ( isParagraphable( child, data.context, conversionApi.schema, isParagraphLike ) ) {
// If there is no wrapping `paragraph` element, create it.
if ( !autoParagraph ) {
autoParagraph = conversionApi.writer.createElement( 'paragraph' );
conversionApi.writer.insert( autoParagraph, data.output, child.index );
}
// Otherwise, use existing `paragraph` and just fix iterator.
// Thanks to reusing `paragraph` element, multiple siblings ends up in same container.
else {
i--;
}

conversionApi.writer.append( child, autoParagraph );
} else {
// That was not a paragraphable children, reset `paragraph` wrapper - following auto-paragraphable children
// need to be placed in a new `paragraph` element.
autoParagraph = null;
}
}

if ( needsNewContext ) {
data.context.pop();
}
}

function isParagraphable( node, context, schema, insideParagraphLikeElement ) {
// Node is paragraphable if it is inside paragraph like element, or...
// It is not allowed at this context...
if ( !insideParagraphLikeElement && schema.checkChild( context, node ) ) {
return false;
}
function isParagraphable( node, position, schema ) {
const context = new SchemaContext( position );

// And paragraph is allowed in this context...
// When paragraph is allowed in this context...
if ( !schema.checkChild( context, 'paragraph' ) ) {
return false;
}

// And a node would be allowed in this paragraph...
if ( !schema.checkChild( [ ...context, 'paragraph' ], node ) ) {
if ( !schema.checkChild( context.concat( 'paragraph' ), node ) ) {
return false;
}

Expand Down
28 changes: 17 additions & 11 deletions tests/paragraph.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ describe( 'Paragraph feature', () => {
} );

it( 'should not autoparagraph text (in clipboard holder)', () => {
const modelFragment = editor.data.parse( 'foo', '$clipboardHolder' );
const modelFragment = editor.data.parse( 'foo', [ '$clipboardHolder' ] );

expect( stringifyModel( modelFragment ) )
.to.equal( 'foo' );
Expand All @@ -101,7 +101,7 @@ describe( 'Paragraph feature', () => {
it( 'should not autoparagraph text (in a context which does not allow paragraphs', () => {
model.schema.register( 'specialRoot' );

const modelFragment = editor.data.parse( 'foo', 'specialRoot' );
const modelFragment = editor.data.parse( 'foo', [ 'specialRoot' ] );

expect( stringifyModel( modelFragment ) )
.to.equal( '' );
Expand All @@ -125,7 +125,7 @@ describe( 'Paragraph feature', () => {
} );

it( 'should not autoparagraph 3 inline nodes (in clipboardHolder)', () => {
const modelFragment = editor.data.parse( 'foo<b>bar</b>bom', '$clipboardHolder' );
const modelFragment = editor.data.parse( 'foo<b>bar</b>bom', [ '$clipboardHolder' ] );

expect( stringifyModel( modelFragment ) )
.to.equal( 'foobarbom' );
Expand Down Expand Up @@ -189,8 +189,8 @@ describe( 'Paragraph feature', () => {

// This test was taken from the list package.
it( 'does not break when some converter returns nothing', () => {
editor.data.viewToModel.on( 'element:li', ( evt, data, consumable ) => {
consumable.consume( data.input, { name: true } );
editor.data.viewToModel.on( 'element:li', ( evt, data, conversionApi ) => {
conversionApi.consumable.consume( data.input, { name: true } );
}, { priority: 'highest' } );

const modelFragment = editor.data.parse( '<ul><li></li></ul>' );
Expand Down Expand Up @@ -243,7 +243,7 @@ describe( 'Paragraph feature', () => {
} );

it( 'should convert h1+h2 (in clipboard holder)', () => {
const modelFragment = editor.data.parse( '<h1>foo</h1><h2>bar</h2>', '$clipboardHolder' );
const modelFragment = editor.data.parse( '<h1>foo</h1><h2>bar</h2>', [ '$clipboardHolder' ] );

expect( stringifyModel( modelFragment ) )
.to.equal( '<paragraph>foo</paragraph><paragraph>bar</paragraph>' );
Expand All @@ -257,7 +257,7 @@ describe( 'Paragraph feature', () => {

buildViewConverter().for( editor.data.viewToModel ).fromElement( 'div' ).toElement( 'div' );

const modelFragment = editor.data.parse( '<h1>foo</h1><h2>bar</h2><div>bom</div>', 'specialRoot' );
const modelFragment = editor.data.parse( '<h1>foo</h1><h2>bar</h2><div>bom</div>', [ 'specialRoot' ] );

expect( stringifyModel( modelFragment ) )
.to.equal( '<div>bom</div>' );
Expand All @@ -271,7 +271,7 @@ describe( 'Paragraph feature', () => {
} );

it( 'should convert ul,ol>li (in clipboard holder)', () => {
const modelFragment = editor.data.parse( '<ul><li>a</li><li>b</li></ul><ol><li>c</li></ol>', '$clipboardHolder' );
const modelFragment = editor.data.parse( '<ul><li>a</li><li>b</li></ul><ol><li>c</li></ol>', [ '$clipboardHolder' ] );

expect( stringifyModel( modelFragment ) )
.to.equal( '<paragraph>a</paragraph><paragraph>b</paragraph><paragraph>c</paragraph>' );
Expand All @@ -287,7 +287,7 @@ describe( 'Paragraph feature', () => {
// "b" is not autoparagraphed because clipboard holder allows text nodes.
// There's a similar integrational test what's going to happen when pasting in paragraph-integration.js.
it( 'should convert ul>li>ul>li+li (in clipboard holder)', () => {
const modelFragment = editor.data.parse( '<ul><li>a<ul><li>b</li><li>c</li></ul></li></ul>', '$clipboardHolder' );
const modelFragment = editor.data.parse( '<ul><li>a<ul><li>b</li><li>c</li></ul></li></ul>', [ '$clipboardHolder' ] );

expect( stringifyModel( modelFragment ) )
.to.equal( '<paragraph>a</paragraph><paragraph>b</paragraph><paragraph>c</paragraph>' );
Expand All @@ -303,7 +303,7 @@ describe( 'Paragraph feature', () => {
// "b" is not autoparagraphed because clipboard holder allows text nodes.
// There's a similar integrational test what's going to happen when pasting in paragraph-integration.js.
it( 'should convert ul>li>p,text (in clipboard holder)', () => {
const modelFragment = editor.data.parse( '<ul><li><p>a</p>b</li></ul>', '$clipboardHolder' );
const modelFragment = editor.data.parse( '<ul><li><p>a</p>b</li></ul>', [ '$clipboardHolder' ] );

expect( stringifyModel( modelFragment ) )
.to.equal( '<paragraph>a</paragraph><paragraph>b</paragraph>' );
Expand All @@ -321,7 +321,7 @@ describe( 'Paragraph feature', () => {
it( 'should convert td (in clipboardHolder)', () => {
const modelFragment = editor.data.parse(
'<table><tr><td>a</td><td>b</td></tr><tr><td>c</td><td>d</td></tr></table>',
'$clipboardHolder'
[ '$clipboardHolder' ]
);

expect( stringifyModel( modelFragment ) )
Expand Down Expand Up @@ -363,6 +363,12 @@ describe( 'Paragraph feature', () => {
expect( modelFragment.getChild( 0 ).childCount ).to.equal( 1 );
expect( modelFragment.getChild( 0 ).getChild( 0 ) ).to.be.instanceOf( ModelText );
} );

it( 'should not convert empty elements', () => {
const modelFragment = editor.data.parse( '<ul><li></li><ul>' );

expect( stringifyModel( modelFragment ) ).to.equal( '' );
} );
} );
} );

Expand Down

0 comments on commit b84b65d

Please sign in to comment.