Refactored view->model conversion to use position. #1256
Conversation
| this.fire( 'viewCleanup', viewItem ); | ||
| // Create set for split elements. We need to remember this elements, because at the end of conversion | ||
| // we want to remove all empty elements that was created as a result of the split. | ||
| this.conversionApi.splitElements = new Set(); |
There was a problem hiding this comment.
I'm going to move splitElements from conversionApi to private ViewConversiondispatcher property.
src/conversion/buildviewconverter.js
Outdated
| // element, so we need to move range after parent of the last converted child. | ||
| // | ||
| // after: <allowed>[]</allowed> | ||
| // before: <allowed>[<converted><child></child></converted><child></child><converted>]</converted></allowed> |
There was a problem hiding this comment.
after, before? Shouldn't it be: before, after?
src/conversion/buildviewconverter.js
Outdated
| // after element. | ||
| // | ||
| // after: <allowed>[]</allowed> | ||
| // before: <allowed>[<converted></converted>]</allowed> |
There was a problem hiding this comment.
Also, as far as I understand, this is a case when there are no children. Otherwise, the previous one will do the job.
There was a problem hiding this comment.
Hm... I changed it yesterday.
|
|
||
| // Remember all elements that are created as a result of split. | ||
| // This is important because at the end of conversion we want to remove all empty split elements. | ||
| for ( const pos of data.range.getPositions() ) { |
There was a problem hiding this comment.
A sample in the comment would be useful to explain what this loop does.
src/model/schema.js
Outdated
| args[ 0 ] = new SchemaContext( args[ 0 ] ); | ||
| if ( !( args[ 0 ] instanceof SchemaContext ) ) { | ||
| args[ 0 ] = new SchemaContext( args[ 0 ] ); | ||
| } |
…ersionApi#convertChildren.
|
Everything is fixed (I hope). |
|
|
||
| // When allowed parent is in context tree. | ||
| if ( this._modelCursor.parent.getAncestors().includes( allowedParent ) ) { | ||
| return null; |
There was a problem hiding this comment.
Are you sure that null should be returned here? Shouldn't we break it as much as possible in such case, and let insertContent do the rest?
There was a problem hiding this comment.
When insertContent is inserting conversion result it means that it is a pasting. Pasting is converted in $clipboardHolder context.
We do not allow anywhere to create invalid (from schema perspective) model tree. The idea of splitting was introduced to avoid creating invalid tree.
…lement had an attribute that didn't change.
Suggested merge commit message (convention)
Feature: Convert view to model using position. Closes ckeditor/ckeditor5#4225. Closes https://github.com/ckeditor/ckeditor5-engine/issues/1250.
BREAKING CHANGE:
DataController#parse,DataController#toModel,ViewConversionDispatcher#convertgetsSchemaContextDefinitionas a contex instead ofString.BREAKING CHANGE:
ViewConversionApi#splitToAllowedParenthas been introduced.BREAKING CHANGE:
ViewConversionApi#storagehas been introduced.BREAKING CHANGE:
ViewConsumablehas been merged toViewConversionApi.BREAKING CHANGE: Format od data object passed across conversion callback has been changed.
BREAKING CHANGE:
Schema#findAllowedParenthas been introduced.BREAKING CHANGE:
SchemaContext#concathas been introduced.Constelation: https://github.com/ckeditor/ckeditor5/tree/t/ckeditor5-engine/1213
Related PR's:
Related branches (small changes):