Bladeren bron

Merge pull request #175 from ckeditor/t/171

Internal: Improved figure conversion. Closes #171.
Piotr Jasiun 8 jaren geleden
bovenliggende
commit
1c95a8a02e

+ 12 - 11
packages/ckeditor5-image/src/image/converters.js

@@ -8,7 +8,6 @@
  */
  */
 
 
 import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
 import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
-import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range';
 import first from '@ckeditor/ckeditor5-utils/src/first';
 import first from '@ckeditor/ckeditor5-utils/src/first';
 
 
 /**
 /**
@@ -32,11 +31,6 @@ export function viewFigureToModel() {
 			return;
 			return;
 		}
 		}
 
 
-		// Do not convert if image cannot be placed in model at current position.
-		if ( !conversionApi.schema.checkChild( data.modelCursor, 'image' ) ) {
-			return;
-		}
-
 		// Find an image element inside the figure element.
 		// Find an image element inside the figure element.
 		const viewImage = Array.from( data.viewItem.getChildren() ).find( viewChild => viewChild.is( 'img' ) );
 		const viewImage = Array.from( data.viewItem.getChildren() ).find( viewChild => viewChild.is( 'img' ) );
 
 
@@ -46,17 +40,24 @@ export function viewFigureToModel() {
 		}
 		}
 
 
 		// Convert view image to model image.
 		// Convert view image to model image.
-		const { modelRange } = conversionApi.convertItem( viewImage, data.modelCursor );
+		const conversionResult = conversionApi.convertItem( viewImage, data.modelCursor );
 
 
 		// Get image element from conversion result.
 		// Get image element from conversion result.
-		const modelImage = first( modelRange.getItems() );
+		const modelImage = first( conversionResult.modelRange.getItems() );
+
+		// When image wasn't successfully converted then finish conversion.
+		if ( !modelImage ) {
+			return;
+		}
 
 
 		// Convert rest of the figure element's children as an image children.
 		// Convert rest of the figure element's children as an image children.
 		conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( modelImage ) );
 		conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( modelImage ) );
 
 
-		// Set model image as conversion result.
-		data.modelRange = ModelRange.createOn( modelImage );
-		data.modelCursor = data.modelRange.end;
+		// Set image range as conversion result.
+		data.modelRange = conversionResult.modelRange;
+
+		// Continue conversion where image conversion ends.
+		data.modelCursor = conversionResult.modelCursor;
 	};
 	};
 }
 }
 
 

+ 71 - 16
packages/ckeditor5-image/tests/image/converters.js

@@ -13,6 +13,7 @@ import { toImageWidget } from '../../src/image/utils';
 import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
 import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
 import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter';
 import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter';
 import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range';
 import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range';
+import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
 import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
 import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
 import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
 
@@ -53,25 +54,23 @@ describe( 'Image converters', () => {
 		let dispatcher, schema, imgConverterCalled;
 		let dispatcher, schema, imgConverterCalled;
 
 
 		beforeEach( () => {
 		beforeEach( () => {
-			imgConverterCalled = false;
+			// Since this part of test tests only view->model conversion editing pipeline is not necessary
+			// so defining model->view converters won't be necessary.
+			editor.editing.destroy();
 
 
 			schema = model.schema;
 			schema = model.schema;
 			schema.extend( '$text', { allowIn: 'image' } );
 			schema.extend( '$text', { allowIn: 'image' } );
 
 
 			dispatcher = editor.data.viewToModel;
 			dispatcher = editor.data.viewToModel;
 			dispatcher.on( 'element:figure', viewFigureToModel() );
 			dispatcher.on( 'element:figure', viewFigureToModel() );
-			dispatcher.on( 'element:img', ( evt, data, conversionApi ) => {
-				if ( conversionApi.consumable.consume( data.viewItem, { name: true, attribute: 'src' } ) ) {
-					const image = conversionApi.writer.createElement( 'image', { src: data.viewItem.getAttribute( 'src' ) } );
-
-					conversionApi.writer.insert( image, data.modelCursor );
-
-					data.modelRange = ModelRange.createOn( image );
-					data.modelCursor = data.modelRange.end;
 
 
+			buildViewConverter().for( dispatcher )
+				.from( { name: 'img', attribute: { src: true } } )
+				.toElement( viewImage => {
 					imgConverterCalled = true;
 					imgConverterCalled = true;
-				}
-			} );
+
+					return new ModelElement( 'image', { src: viewImage.getAttribute( 'src' ) } );
+				} );
 		} );
 		} );
 
 
 		it( 'should find img element among children and convert it using already defined converters', () => {
 		it( 'should find img element among children and convert it using already defined converters', () => {
@@ -81,14 +80,13 @@ describe( 'Image converters', () => {
 			expect( imgConverterCalled ).to.be.true;
 			expect( imgConverterCalled ).to.be.true;
 		} );
 		} );
 
 
-		it( 'should convert non-img children in image context and append them to model image element', () => {
+		it( 'should convert children allowed by schema and omit disallowed', () => {
 			buildViewConverter().for( editor.data.viewToModel ).fromElement( 'foo' ).toElement( 'foo' );
 			buildViewConverter().for( editor.data.viewToModel ).fromElement( 'foo' ).toElement( 'foo' );
 			buildViewConverter().for( editor.data.viewToModel ).fromElement( 'bar' ).toElement( 'bar' );
 			buildViewConverter().for( editor.data.viewToModel ).fromElement( 'bar' ).toElement( 'bar' );
 
 
-			schema.register( 'foo' );
-			schema.register( 'bar' );
-
-			schema.extend( 'foo', { allowIn: 'image' } );
+			schema.register( 'foo', { allowIn: 'image' } );
+			// Is allowed in root, but should not try to split image element.
+			schema.register( 'bar', { allowIn: '$root' } );
 
 
 			editor.setData( '<figure class="image">x<img src="foo.png" />y<foo></foo><bar></bar></figure>' );
 			editor.setData( '<figure class="image">x<img src="foo.png" />y<foo></foo><bar></bar></figure>' );
 
 
@@ -96,6 +94,63 @@ describe( 'Image converters', () => {
 			expectModel( '<image src="foo.png">xy<foo></foo></image>' );
 			expectModel( '<image src="foo.png">xy<foo></foo></image>' );
 		} );
 		} );
 
 
+		it( 'should split parent element when image is not allowed - in the middle', () => {
+			buildViewConverter().for( editor.data.viewToModel ).fromElement( 'div' ).toElement( 'div' );
+
+			schema.register( 'div', { inheritAllFrom: '$block' } );
+			schema.extend( 'image', { disallowIn: 'div' } );
+
+			editor.setData(
+				'<div>' +
+					'abc' +
+					'<figure class="image">' +
+						'<img src="foo.jpg"/>' +
+					'</figure>' +
+					'def' +
+				'</div>'
+			);
+
+			expectModel( '<div>abc</div><image src="foo.jpg"></image><div>def</div>' );
+		} );
+
+		it( 'should split parent element when image is not allowed - at the end', () => {
+			buildViewConverter().for( editor.data.viewToModel ).fromElement( 'div' ).toElement( 'div' );
+			buildModelConverter().for( editor.editing.modelToView ).fromElement( 'div' ).toElement( 'div' );
+
+			schema.register( 'div', { inheritAllFrom: '$block' } );
+			schema.extend( 'image', { disallowIn: 'div' } );
+
+			editor.setData(
+				'<div>' +
+					'abc' +
+					'<figure class="image">' +
+						'<img src="foo.jpg"/>' +
+					'</figure>' +
+				'</div>'
+			);
+
+			expectModel( '<div>abc</div><image src="foo.jpg"></image>' );
+		} );
+
+		it( 'should split parent element when image is not allowed - at the beginning', () => {
+			buildViewConverter().for( editor.data.viewToModel ).fromElement( 'div' ).toElement( 'div' );
+			buildModelConverter().for( editor.editing.modelToView ).fromElement( 'div' ).toElement( 'div' );
+
+			schema.register( 'div', { inheritAllFrom: '$block' } );
+			schema.extend( 'image', { disallowIn: 'div' } );
+
+			editor.setData(
+				'<div>' +
+					'<figure class="image">' +
+						'<img src="foo.jpg"/>' +
+					'</figure>' +
+					'def' +
+				'</div>'
+			);
+
+			expectModel( '<image src="foo.jpg"></image><div>def</div>' );
+		} );
+
 		it( 'should be possible to overwrite', () => {
 		it( 'should be possible to overwrite', () => {
 			dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => {
 			dispatcher.on( 'element:figure', ( evt, data, conversionApi ) => {
 				conversionApi.consumable.consume( data.viewItem, { name: true } );
 				conversionApi.consumable.consume( data.viewItem, { name: true } );

+ 0 - 11
packages/ckeditor5-image/tests/image/imageengine.js

@@ -248,17 +248,6 @@ describe( 'ImageEngine', () => {
 				expect( getModelData( model, { withoutSelection: true } ) ).to.equal( '<div></div>' );
 				expect( getModelData( model, { withoutSelection: true } ) ).to.equal( '<div></div>' );
 			} );
 			} );
 
 
-			it( 'should handle figure with two images', () => {
-				model.schema.extend( '$text', { allowIn: 'image' } );
-
-				editor.setData( '<figure class="image"><img src="foo.jpg" /><img src="bar.jpg" />abc</figure>' );
-
-				// The foo.jpg image is properly converted using figure converter. The other image was tried to
-				// be added as a child of foo.jpg and then was autohoisted.
-				expect( getModelData( model, { withoutSelection: true } ) )
-					.to.equal( '<image src="bar.jpg"></image><image src="foo.jpg">abc</image>' );
-			} );
-
 			it( 'should convert image with srcset attribute', () => {
 			it( 'should convert image with srcset attribute', () => {
 				editor.setData(
 				editor.setData(
 					'<figure class="image">' +
 					'<figure class="image">' +