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 #91 from ckeditor/t/ckeditor5-engine/1213
Browse files Browse the repository at this point in the history 
Internal: Changed converters after engine/conversion refactoring.
  • Loading branch information
Piotr Jasiun authored Jan 31, 2018
2 parents 804f85d + 6cc7b03 commit 2523044
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 46 deletions.
74 changes: 34 additions & 40 deletions src/converters.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range';

import ViewContainerElement from '@ckeditor/ckeditor5-engine/src/view/containerelement';
import ViewPosition from '@ckeditor/ckeditor5-engine/src/view/position';
Expand Down Expand Up @@ -341,69 +342,56 @@ export function modelViewMergeAfter( evt, data, conversionApi ) {
*
* To set correct values of the `type` and `indent` attributes the converter:
* * checks `<li>`'s parent,
* * passes the `data.indent` value when `<li>`'s sub-items are converted.
* * stores and increases the `conversionApi.store.indent` value when `<li>`'s sub-items are converted.
*
* @see module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher#event:element
* @param {module:utils/eventinfo~EventInfo} evt An object containing information about the fired event.
* @param {Object} data An object containing conversion input and a placeholder for conversion output and possibly other values.
* @param {module:engine/conversion/viewconsumable~ViewConsumable} consumable Values to consume.
* @param {Object} conversionApi Conversion interface to be used by the callback.
*/
export function viewModelConverter( evt, data, consumable, conversionApi ) {
if ( consumable.consume( data.input, { name: true } ) ) {
export function viewModelConverter( evt, data, conversionApi ) {
if ( conversionApi.consumable.consume( data.viewItem, { name: true } ) ) {
const writer = conversionApi.writer;
const conversionStore = this.conversionApi.store;

// 1. Create `listItem` model element.
const listItem = writer.createElement( 'listItem' );

// 2. Handle `listItem` model element attributes.
data.indent = data.indent ? data.indent : 0;
writer.setAttribute( 'indent', data.indent, listItem );
conversionStore.indent = conversionStore.indent || 0;
writer.setAttribute( 'indent', conversionStore.indent, listItem );

// Set 'bulleted' as default. If this item is pasted into a context,
const type = data.input.parent && data.input.parent.name == 'ol' ? 'numbered' : 'bulleted';
const type = data.viewItem.parent && data.viewItem.parent.name == 'ol' ? 'numbered' : 'bulleted';
writer.setAttribute( 'type', type, listItem );

let alteredContext = false;

// See https://github.com/ckeditor/ckeditor5-list/issues/87
if ( data.context[ data.context.length - 1 ].name != 'listItem' ) {
// 3. Handle `<li>` children.
data.context.push( listItem );
alteredContext = true;
}

// `listItem`s created recursively should have bigger indent.
data.indent++;
conversionStore.indent++;

// `listItem`s will be kept in flat structure.
const items = writer.createDocumentFragment();
writer.append( listItem, items );
writer.insert( listItem, data.cursorPosition );

// Remember position after list item.
let nextPosition = ModelPosition.createAfter( 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.input.getChildren() ) {
// Let's convert the child.
const converted = conversionApi.convertItem( child, consumable, data );

// If this is a view list element, we will convert it and concat the result (`listItem` model elements)
// with already gathered results (in `items` array). `converted` should be a `ModelDocumentFragment`.
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' ) {
writer.append( converted, items );
nextPosition = conversionApi.convertItem( child, nextPosition ).cursorPosition;
}
// If it was not a list it was a "regular" list item content. Just append it to `listItem`.
else {
writer.append( converted, listItem );
conversionApi.convertItem( child, ModelPosition.createAt( listItem, 'end' ) );
}
}

data.indent--;
if ( alteredContext ) {
data.context.pop();
}
conversionStore.indent--;

data.output = items;
data.modelRange = new ModelRange( data.cursorPosition, nextPosition );
data.cursorPosition = data.modelRange.end;
}
}

Expand All @@ -417,10 +405,10 @@ export function viewModelConverter( evt, data, consumable, conversionApi ) {
* @param {Object} data An object containing conversion input and a placeholder for conversion output and possibly other values.
* @param {module:engine/conversion/viewconsumable~ViewConsumable} consumable Values to consume.
*/
export function cleanList( evt, data, consumable ) {
if ( consumable.test( data.input, { name: true } ) ) {
export function cleanList( evt, data, conversionApi ) {
if ( conversionApi.consumable.test( data.viewItem, { name: true } ) ) {
// Caching children because when we start removing them iterating fails.
const children = Array.from( data.input.getChildren() );
const children = Array.from( data.viewItem.getChildren() );

for ( const child of children ) {
if ( !child.is( 'li' ) ) {
Expand All @@ -438,13 +426,13 @@ export function cleanList( evt, data, consumable ) {
* @param {Object} data An object containing conversion input and a placeholder for conversion output and possibly other values.
* @param {module:engine/conversion/viewconsumable~ViewConsumable} consumable Values to consume.
*/
export function cleanListItem( evt, data, consumable ) {
if ( consumable.test( data.input, { name: true } ) ) {
if ( data.input.childCount === 0 ) {
export function cleanListItem( evt, data, conversionApi ) {
if ( conversionApi.consumable.test( data.viewItem, { name: true } ) ) {
if ( data.viewItem.childCount === 0 ) {
return;
}

const children = [ ...data.input.getChildren() ];
const children = [ ...data.viewItem.getChildren() ];

let foundList = false;
let firstNode = true;
Expand Down Expand Up @@ -571,12 +559,18 @@ export function viewToModelPosition( evt, data ) {
/**
* Post-fixer that reacts to changes on document and fixes incorrect model states.
*
* Example:
* In an example below, there is a correct list structure.
* Then the middle element will be removed so the list structure will become incorrect:
*
* <listItem type="bulleted" indent=0>Item 1</listItem>
* <listItem type="bulleted" indent=1>Item 2</listItem> <--- this is removed.
* <listItem type="bulleted" indent=2>Item 3</listItem>
*
* List structure after the middle element removed:
*
* <listItem type="bulleted" indent=0>Item 1</listItem>
* <listItem type="bulleted" indent=2>Item 3</listItem>
*
* Should become:
*
* <listItem type="bulleted" indent=0>Item 1</listItem>
Expand Down
13 changes: 7 additions & 6 deletions tests/listengine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import ListCommand from '../src/listcommand';

import ModelDocumentFragment from '@ckeditor/ckeditor5-engine/src/model/documentfragment';
import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range';
import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
import ModelText from '@ckeditor/ckeditor5-engine/src/model/text';
import ViewPosition from '@ckeditor/ckeditor5-engine/src/view/position';
Expand Down Expand Up @@ -3355,8 +3356,8 @@ describe( 'ListEngine', () => {
} );

it( 'view li converter should not fire if change was already consumed', () => {
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.viewItem, { name: true } );
}, { priority: 'highest' } );

editor.setData( '<p></p><ul><li></li></ul>' );
Expand All @@ -3365,18 +3366,18 @@ describe( 'ListEngine', () => {
} );

it( 'view ul converter should not fire if change was already consumed', () => {
editor.data.viewToModel.on( 'element:ul', ( evt, data, consumable ) => {
consumable.consume( data.input, { name: true } );
editor.data.viewToModel.on( 'element:ul', ( evt, data, conversionApi ) => {
conversionApi.consumable.consume( data.viewItem, { name: true } );
}, { priority: 'highest' } );

editor.setData( '<p></p><ul><li></li></ul>' );

expect( getModelData( model, { withoutSelection: true } ) ).to.equal( '<paragraph></paragraph>' );
} );

it( 'view converter should pass model document fragment in data.output', () => {
it( 'view converter should pass model range in data.modelRange', () => {
editor.data.viewToModel.on( 'element:ul', ( evt, data ) => {
expect( data.output ).to.be.instanceof( ModelDocumentFragment );
expect( data.modelRange ).to.be.instanceof( ModelRange );
}, { priority: 'lowest' } );

editor.setData( '<ul><li>Foo</li><li>Bar</li></ul>' );
Expand Down

0 comments on commit 2523044

Please sign in to comment.