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

Commit

Permalink
Merge branch 'master' into t/ckeditor5-engine/1555
Browse files Browse the repository at this point in the history
# Conflicts:
#	src/converters.js
  • Loading branch information
jodator committed Oct 26, 2018
2 parents 5e365af + abccef4 commit e1335ab
Show file tree
Hide file tree
Showing 4 changed files with 297 additions and 17 deletions.
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ node_js:
- '8'
cache:
- node_modules
branches:
except:
- stable
before_install:
- export DISPLAY=:99.0
- sh -e /etc/init.d/xvfb start
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Contributing
========================================

Information about contributing can be found on the following page: <https://github.com/ckeditor/ckeditor5/blob/master/CONTRIBUTING.md>.
See the [official contributors' guide to CKEditor 5](https://ckeditor.com/docs/ckeditor5/latest/framework/guides/contributing/contributing.html) to learn more.
79 changes: 63 additions & 16 deletions src/converters.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,22 +366,7 @@ export function viewModelConverter( evt, data, conversionApi ) {

writer.insert( listItem, splitResult.position );

// Remember position after list item, next list items will be inserted at this position.
let nextPosition = writer.createPositionAfter( listItem );

// Check all children of the converted `<li>`.
// At this point we assume there are no "whitespace" view text nodes in view list, between view list items.
// This should be handled by `<ul>` and `<ol>` converters.
for ( const child of data.viewItem.getChildren() ) {
// If this is a view list element, we will convert it after last `listItem` model element.
if ( child.name == 'ul' || child.name == 'ol' ) {
nextPosition = conversionApi.convertItem( child, nextPosition ).modelCursor;
}
// If it was not a list it was a "regular" list item content. Just convert it to `listItem`.
else {
conversionApi.convertItem( child, writer.createPositionAt( listItem, 'end' ) );
}
}
const nextPosition = viewToModelListItemChildrenConverter( listItem, data.viewItem.getChildren(), conversionApi );

conversionStore.indent--;

Expand Down Expand Up @@ -815,6 +800,68 @@ function generateLiInUl( modelItem, conversionApi ) {
return viewItem;
}

// Helper function that converts children of a given `<li>` view element into corresponding model elements.
// The function maintains proper order of elements if model `listItem` is split during the conversion
// due to block children conversion.
//
// @param {module:engine/model/element~Element} listItemModel List item model element to which converted children will be inserted.
// @param {Iterable.<module:engine/view/node~Node>} viewChildren View elements which will be converted.
// @param {Object} conversionApi Conversion interface to be used by the callback.
// @returns {module:engine/model/position~Position} Position on which next elements should be inserted after children conversion.
function viewToModelListItemChildrenConverter( listItemModel, viewChildren, conversionApi ) {
const writer = conversionApi.writer;
let lastListItem = listItemModel;
let nextPosition = writer.createPositionAfter( listItemModel );

// Check all children of the converted `<li>`. At this point we assume there are no "whitespace" view text nodes
// in view list, between view list items. This should be handled by `<ul>` and `<ol>` converters.
for ( const child of viewChildren ) {
// If this is a view list element, we will insert its conversion result after currently handled `listItem`.
if ( child.name == 'ul' || child.name == 'ol' ) {
nextPosition = conversionApi.convertItem( child, nextPosition ).modelCursor;
}
// If it is not a view list element it is a "regular" list item content. Its conversion result will
// be inserted as `listItem` children (block children may split current `listItem`).
else {
const result = conversionApi.convertItem( child, writer.createPositionAt( lastListItem, 'end' ) );
const convertedChild = result.modelRange.start.nodeAfter;

nextPosition = result.modelCursor;

// If there is a block element child being converted it may split the current list item, for example:
//
// <li><p>Foo</p></li>
//
// will be converted to:
//
// <listItem></listItem><paragraph>Foo</paragraph><listItem></listItem>
//
// so we need to update reference to `lastListItem`.
if ( convertedChild && convertedChild.is( 'element' ) &&
!conversionApi.schema.checkChild( lastListItem, convertedChild.name ) ) {
lastListItem = nextPosition.parent;

// Depending on the used converter for block elements, usually the position (`result.modelCursor`
// marked as # below) points to the second list item after conversion:
//
// `<li><p>Foo</p></li>` -> `<listItem></listItem><paragraph>Foo</paragraph><listItem>#</listItem>`
//
// However, in some cases like autoparagraphing the position is placed on the end of the block element:
//
// `<li><h2>Foo</h2></li>` -> `<listItem></listItem><paragraph>Foo#</paragraph><listItem></listItem>`
//
// We need to check for such cases and use proper list item and position based on it.
if ( !lastListItem.is( 'listItem' ) && lastListItem.nextSibling && lastListItem.nextSibling.is( 'listItem' ) ) {
lastListItem = lastListItem.nextSibling;
nextPosition = writer.createPositionAt( lastListItem, 'end' );
}
}
}
}

return nextPosition;
}

// Helper function that seeks for a list item sibling of given model item (or position) which meets given criteria.
// `options` object may contain one or more of given values (by default they are `false`):
// `options.sameIndent` - whether sought sibling should have same indent (default = no),
Expand Down
230 changes: 230 additions & 0 deletions tests/listediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,167 @@ describe( 'ListEditing', () => {

expect( getModelData( model, { withoutSelection: true } ) ).to.equal( expectedModelData );
} );

describe( 'block elements inside list items', () => {
describe( 'single block', () => {
test(
'single item',
'<ul><li><p>Foo</p></li></ul>',
'<p>Foo</p>'
);
test(
'multiple items',
'<ul><li><p>Foo</p></li><li><p>Bar</p></li></ul>',
'<p>Foo</p><p>Bar</p>'
);
test(
'nested items',
'<ul><li><p>Foo</p><ol><li><p>Bar</p></li></ol></li></ul>',
'<p>Foo</p><p>Bar</p>'
);
} );

describe( 'multiple blocks', () => {
test(
'single item',
'<ul><li><h2>Foo</h2><p>Bar</p></li></ul>',
'<p>Foo</p><p>Bar</p>'
);
test(
'multiple items',
'<ol><li><p>123</p></li></ol><ul><li><h2>Foo</h2><p>Bar</p></li></ul>',
'<p>123</p><p>Foo</p><p>Bar</p>'
);
test(
'nested items #2',
'<ol><li><p>123</p><p>456</p><ul><li><h2>Foo</h2><p>Bar</p></li></ul></li></ol>',
'<p>123</p><p>456</p><p>Foo</p><p>Bar</p>'
);
} );

describe.skip( 'multiple blocks', () => { // Skip due to #112 issue.
test(
'nested items #1',
'<ol><li><p>123</p><ul><li><h2>Foo</h2><p>Bar</p></li></ul><p>456</p></li></ol>',
'<p>123</p><p>Foo</p><p>Bar</p><p>456</p>'
);
} );

describe( 'inline + block', () => {
test(
'single item',
'<ul><li>Foo<p>Bar</p></li></ul>',
'<ul><li>Foo</li></ul><p>Bar</p>'
);
test(
'multiple items',
'<ul><li>Foo<p>Bar</p></li><li>Foz<p>Baz</p></li></ul>',
'<ul><li>Foo</li></ul><p>Bar</p><ul><li>Foz</li></ul><p>Baz</p>'
);
test(
'split by list items',
'<ul><li>Foo</li><li><p>Bar</p></li></ul>',
'<ul><li>Foo</li></ul><p>Bar</p>'
);
test(
'nested split by list items',
'<ul><li>Foo<ol><li><p>Bar</p></li></ol></li></ul>',
'<ul><li>Foo</li></ul><p>Bar</p>'
);
test(
'nested items #1',
'<ol><li>Foo<p>Bar</p><ul><li>123<h2>456</h2></li></ul></li></ol>',
'<ol><li>Foo</li></ol><p>Bar</p><ul><li>123</li></ul><p>456</p>'
);
test(
'nested items #2',
'<ol><li>Foo<p>Bar</p><ul><li>123<h2>456</h2></li></ul></li><li>abc<h2>def</h2></li></ol>',
'<ol><li>Foo</li></ol><p>Bar</p><ul><li>123</li></ul><p>456</p><ol><li>abc</li></ol><p>def</p>'
);
} );

describe( 'block + inline', () => {
test(
'single item',
'<ul><li><p>Foo</p>Bar</li></ul>',
'<p>Foo</p><ul><li>Bar</li></ul>'
);
test(
'multiple items',
'<ul><li><p>Foo</p>Bar</li><li><p>Foz</p>Baz</li></ul>',
'<p>Foo</p><ul><li>Bar</li></ul><p>Foz</p><ul><li>Baz</li></ul>'
);
test(
'split by list items',
'<ul><li><p>Bar</p><li>Foo</li></li></ul>',
'<p>Bar</p><ul><li>Foo</li></ul>'
);
test(
'nested split by list items',
'<ul><li><p>Bar</p><ol><li>Foo</li></ol></li></ul>',
'<p>Bar</p><ol><li>Foo</li></ol>'
);
test(
'nested items #1',
'<ol><li><p>Foo</p>Bar<ul><li><h2>123</h2>456</li></ul></li></ol>',
'<p>Foo</p><ol><li>Bar</li></ol><p>123</p><ul><li>456</li></ul>'
);
test(
'nested items #2',
'<ol><li><p>Foo</p>Bar<ul><li><h2>123</h2>456</li></ul></li><li><h2>abc</h2>def</li></ol>',
'<p>Foo</p><ol><li>Bar</li></ol><p>123</p><ul><li>456</li></ul><p>abc</p><ol><li>def</li></ol>'
);
} );

describe( 'complex', () => {
test(
'single item with inline block inline',
'<ul><li>Foo<p>Bar</p>Baz</li></ul>',
'<ul><li>Foo</li></ul><p>Bar</p><ul><li>Baz</li></ul>'
);
test(
'single item with inline block block',
'<ul><li>Text<p>Foo</p><p>Bar</p></li></ul>',
'<ul><li>Text</li></ul><p>Foo</p><p>Bar</p>'
);
test(
'single item with block block inline',
'<ul><li><p>Foo</p><p>Bar</p>Text</li></ul>',
'<p>Foo</p><p>Bar</p><ul><li>Text</li></ul>'
);
test(
'single item with block block block',
'<ul><li><p>Foo</p><p>Bar</p><p>Baz</p></li></ul>',
'<p>Foo</p><p>Bar</p><p>Baz</p>'
);
test(
'item inline + item block and inline',
'<ul><li>Foo</li><li><p>Bar</p>Baz</li></ul>',
'<ul><li>Foo</li></ul><p>Bar</p><ul><li>Baz</li></ul>'
);
test(
'item inline and block + item inline',
'<ul><li>Foo<p>Bar</p></li><li>Baz</li></ul>',
'<ul><li>Foo</li></ul><p>Bar</p><ul><li>Baz</li></ul>'
);
test(
'multiple items inline/block mix',
'<ul><li>Text<p>Foo</p></li><li>Bar<p>Baz</p>123</li></ul>',
'<ul><li>Text</li></ul><p>Foo</p><ul><li>Bar</li></ul><p>Baz</p><ul><li>123</li></ul>'
);
test(
'nested items',
'<ul><li>Foo<p>Bar</p></li><li>Baz<p>123</p>456<ol><li>ABC<p>DEF</p></li><li>GHI</li></ol></li></ul>',
'<ul><li>Foo</li></ul><p>Bar</p><ul><li>Baz</li></ul><p>123</p><ul><li>456<ol><li>ABC</li></ol></li></ul>' +
'<p>DEF</p><ol><li>GHI</li></ol>'
);
test(
'list with empty inline element',
'<ul><li><span></span>Foo<p>Bar</p></li></ul>',
'<ul><li>Foo</li></ul><p>Bar</p>'
);
} );
} );
} );

describe( 'position mapping', () => {
Expand Down Expand Up @@ -3576,6 +3737,75 @@ describe( 'ListEditing', () => {
'<paragraph>Bar</paragraph>'
);
} );

it( 'should handle block elements inside pasted list #1', () => {
setModelData( model,
'<listItem listType="bulleted" listIndent="0">A</listItem>' +
'<listItem listType="bulleted" listIndent="1">B[]</listItem>' +
'<listItem listType="bulleted" listIndent="2">C</listItem>'
);

const clipboard = editor.plugins.get( 'Clipboard' );

clipboard.fire( 'inputTransformation', {
content: parseView( '<ul><li>W<ul><li>X<p>Y</p>Z</li></ul></li></ul>' )
} );

expect( getModelData( model ) ).to.equal(
'<listItem listIndent="0" listType="bulleted">A</listItem>' +
'<listItem listIndent="1" listType="bulleted">BW</listItem>' +
'<listItem listIndent="2" listType="bulleted">X</listItem>' +
'<paragraph>Y</paragraph>' +
'<listItem listIndent="0" listType="bulleted">Z[]</listItem>' +
'<listItem listIndent="1" listType="bulleted">C</listItem>'
);
} );

it( 'should handle block elements inside pasted list #2', () => {
setModelData( model,
'<listItem listType="bulleted" listIndent="0">A[]</listItem>' +
'<listItem listType="bulleted" listIndent="1">B</listItem>' +
'<listItem listType="bulleted" listIndent="2">C</listItem>'
);

const clipboard = editor.plugins.get( 'Clipboard' );

clipboard.fire( 'inputTransformation', {
content: parseView( '<ul><li>W<ul><li>X<p>Y</p>Z</li></ul></li></ul>' )
} );

expect( getModelData( model ) ).to.equal(
'<listItem listIndent="0" listType="bulleted">AW</listItem>' +
'<listItem listIndent="1" listType="bulleted">X</listItem>' +
'<paragraph>Y</paragraph>' +
'<listItem listIndent="0" listType="bulleted">Z[]</listItem>' +
'<listItem listIndent="0" listType="bulleted">B</listItem>' +
'<listItem listIndent="1" listType="bulleted">C</listItem>'
);
} );

it( 'should handle block elements inside pasted list #3', () => {
setModelData( model,
'<listItem listType="bulleted" listIndent="0">A[]</listItem>' +
'<listItem listType="bulleted" listIndent="1">B</listItem>' +
'<listItem listType="bulleted" listIndent="2">C</listItem>'
);

const clipboard = editor.plugins.get( 'Clipboard' );

clipboard.fire( 'inputTransformation', {
content: parseView( '<ul><li><p>W</p><p>X</p><p>Y</p></li><li>Z</li></ul>' )
} );

expect( getModelData( model ) ).to.equal(
'<listItem listIndent="0" listType="bulleted">AW</listItem>' +
'<paragraph>X</paragraph>' +
'<paragraph>Y</paragraph>' +
'<listItem listIndent="0" listType="bulleted">Z[]</listItem>' +
'<listItem listIndent="1" listType="bulleted">B</listItem>' +
'<listItem listIndent="2" listType="bulleted">C</listItem>'
);
} );
} );

describe( 'other', () => {
Expand Down

0 comments on commit e1335ab

Please sign in to comment.