浏览代码

Merge pull request #146 from ckeditor/t/145

Feature: Replaced srcset attribute in the model by responsive attribute which is now converted to three attributes in the view: srcset, sizes and width. Closes #145. Closes ckeditor/ckeditor5-easy-image#4.

BREAKING CHANGE: The srcset attribute is now replaced by responsive attribute in the model.
Piotr Jasiun 8 年之前
父节点
当前提交
9efb550ed9

+ 56 - 15
packages/ckeditor5-image/src/image/converters.js

@@ -69,32 +69,77 @@ export function viewFigureToModel() {
  *
  * @param {Array.<module:engine/conversion/modelconversiondispatcher~ModelConversionDispatcher>} dispatchers
  * @param {String} attributeName
- * @param {function} [conversionCallback] The function that will be called each time given attribute conversion is performed.
- * It will be called with two parameters: a view image element and the type of the conversion: 'addAttribute`, `changeAttribute` or
- * `removeAttribute`. This callback can be used to perform additional processing on view image element.
+ * @param {Function} [converter] Custom converter for the attribute - default one converts attribute from model `image` element
+ * to the same attribute in `img` in the view.
  */
-export function createImageAttributeConverter( dispatchers, attributeName, conversionCallback ) {
+export function createImageAttributeConverter( dispatchers, attributeName, converter = modelToViewAttributeConverter ) {
 	for ( const dispatcher of dispatchers ) {
-		dispatcher.on( `addAttribute:${ attributeName }:image`, modelToViewAttributeConverter( conversionCallback ) );
-		dispatcher.on( `changeAttribute:${ attributeName }:image`, modelToViewAttributeConverter( conversionCallback ) );
-		dispatcher.on( `removeAttribute:${ attributeName }:image`, modelToViewAttributeConverter( conversionCallback ) );
+		dispatcher.on( `addAttribute:${ attributeName }:image`, converter() );
+		dispatcher.on( `changeAttribute:${ attributeName }:image`, converter() );
+		dispatcher.on( `removeAttribute:${ attributeName }:image`, converter() );
 	}
 }
 
+/**
+ * Converter used to convert `responsive` image's attribute to `srcset`, `sizes` and `width` attributes in the view.
+ *
+ * @return {Function}
+ */
+export function responsiveAttributeConverter() {
+	return ( evt, data, consumable, conversionApi ) => {
+		const parts = evt.name.split( ':' );
+		const consumableType = parts[ 0 ] + ':' + parts[ 1 ];
+		const modelImage = data.item;
+
+		if ( !consumable.consume( modelImage, consumableType ) ) {
+			return;
+		}
+
+		const figure = conversionApi.mapper.toViewElement( modelImage );
+		const img = figure.getChild( 0 );
+		const type = parts[ 0 ];
+
+		if ( type == 'removeAttribute' ) {
+			const oldData = data.attributeOldValue;
+
+			if ( oldData.srcset ) {
+				img.removeAttribute( 'srcset' );
+				img.removeAttribute( 'sizes' );
+
+				if ( oldData.width ) {
+					img.removeAttribute( 'width' );
+				}
+			}
+		} else {
+			const newData = data.attributeNewValue;
+
+			if ( newData.srcset ) {
+				img.setAttribute( 'srcset', newData.srcset );
+				// Always outputting `100vw`. See https://github.com/ckeditor/ckeditor5-image/issues/2.
+				img.setAttribute( 'sizes', '100vw' );
+
+				if ( newData.width ) {
+					img.setAttribute( 'width', newData.width );
+				}
+			}
+		}
+	};
+}
+
 // Returns model to view image converter converting given attribute, and adding it to `img` element nested inside `figure` element.
-// Allows to provide `conversionCallback` called each time conversion is made.
 //
 // @private
-function modelToViewAttributeConverter( conversionCallback ) {
+function modelToViewAttributeConverter() {
 	return ( evt, data, consumable, conversionApi ) => {
 		const parts = evt.name.split( ':' );
 		const consumableType = parts[ 0 ] + ':' + parts[ 1 ];
+		const modelImage = data.item;
 
-		if ( !consumable.consume( data.item, consumableType ) ) {
+		if ( !consumable.consume( modelImage, consumableType ) ) {
 			return;
 		}
 
-		const figure = conversionApi.mapper.toViewElement( data.item );
+		const figure = conversionApi.mapper.toViewElement( modelImage );
 		const img = figure.getChild( 0 );
 		const type = parts[ 0 ];
 
@@ -103,10 +148,6 @@ function modelToViewAttributeConverter( conversionCallback ) {
 		} else {
 			img.setAttribute( data.attributeKey, data.attributeNewValue );
 		}
-
-		if ( conversionCallback ) {
-			conversionCallback( img, type );
-		}
 	};
 }
 

+ 24 - 12
packages/ckeditor5-image/src/image/imageengine.js

@@ -10,7 +10,13 @@
 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 { viewFigureToModel, createImageAttributeConverter, convertHoistableImage, hoistImageThroughElement } from './converters';
+import {
+	viewFigureToModel,
+	createImageAttributeConverter,
+	convertHoistableImage,
+	hoistImageThroughElement,
+	responsiveAttributeConverter
+} from './converters';
 import { toImageWidget } from './utils';
 import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
 import ViewContainerElement from '@ckeditor/ckeditor5-engine/src/view/containerelement';
@@ -18,7 +24,7 @@ import ViewEmptyElement from '@ckeditor/ckeditor5-engine/src/view/emptyelement';
 
 /**
  * The image engine plugin.
- * Registers `<image>` as a block element in the document schema and allows it to have two attributes: `src` and `alt`.
+ * Registers `<image>` as a block element in the document schema, and allows `alt`, `src` and `responsive` attributes.
  * Registers converters for editing and data pipelines.
  *
  * @extends module:core/plugin~Plugin
@@ -38,7 +44,7 @@ export default class ImageEngine extends Plugin {
 		// Configure schema.
 		schema.registerItem( 'image' );
 		schema.requireAttributes( 'image', [ 'src' ] );
-		schema.allow( { name: 'image', attributes: [ 'alt', 'src', 'srcset' ], inside: '$root' } );
+		schema.allow( { name: 'image', attributes: [ 'alt', 'src', 'responsive' ], inside: '$root' } );
 		schema.objects.add( 'image' );
 
 		// Build converter from model to view for data pipeline.
@@ -55,14 +61,7 @@ export default class ImageEngine extends Plugin {
 		createImageAttributeConverter( [ editing.modelToView, data.modelToView ], 'alt' );
 
 		// Convert `srcset` attribute changes and add or remove `sizes` attribute when necessary.
-		createImageAttributeConverter( [ editing.modelToView, data.modelToView ], 'srcset', ( viewImg, type ) => {
-			if ( type == 'removeAttribute' ) {
-				viewImg.removeAttribute( 'sizes' );
-			} else {
-				// Always outputting `100vw`. See https://github.com/ckeditor/ckeditor5-image/issues/2.
-				viewImg.setAttribute( 'sizes', '100vw' );
-			}
-		} );
+		createImageAttributeConverter( [ editing.modelToView, data.modelToView ], 'responsive', responsiveAttributeConverter );
 
 		// Build converter for view img element to model image element.
 		buildViewConverter().for( data.viewToModel )
@@ -84,7 +83,20 @@ export default class ImageEngine extends Plugin {
 		buildViewConverter().for( data.viewToModel )
 			.from( { name: 'img', attribute: { srcset: /./ } } )
 			.consuming( { attribute: [ 'srcset' ] } )
-			.toAttribute( viewImage => ( { key: 'srcset', value: viewImage.getAttribute( 'srcset' ) } ) );
+			.toAttribute( viewImage => {
+				const value = {
+					srcset: viewImage.getAttribute( 'srcset' )
+				};
+
+				if ( viewImage.hasAttribute( 'width' ) ) {
+					value.width = viewImage.getAttribute( 'width' );
+				}
+
+				return {
+					key: 'responsive',
+					value
+				};
+			} );
 
 		// Converter for figure element from view to model.
 		data.viewToModel.on( 'element:figure', viewFigureToModel() );

+ 172 - 9
packages/ckeditor5-image/tests/image/imageengine.js

@@ -49,8 +49,10 @@ describe( 'ImageEngine', () => {
 				expect( editor.getData() ).to.equal( '<figure class="image"><img src="foo.png"></figure>' );
 			} );
 
-			it( 'should convert with srcset attribute and add sizes attribute', () => {
-				setModelData( document, '<image src="foo.png" alt="alt text" srcset="small.png 148w, big.png 1024w"></image>' );
+			it( 'should convert responsive attribute to srcset and sizes attribute', () => {
+				setModelData( document,
+					'<image src="foo.png" alt="alt text" responsive=\'{ "srcset": "small.png 148w, big.png 1024w" }\'></image>'
+				);
 
 				expect( editor.getData() ).to.equal(
 					'<figure class="image">' +
@@ -58,6 +60,68 @@ describe( 'ImageEngine', () => {
 					'</figure>'
 				);
 			} );
+
+			it( 'should convert responsive attribute to width, srcset and add sizes attribute', () => {
+				setModelData( document,
+					'<image ' +
+						'src="foo.png" ' +
+						'alt="alt text" ' +
+						'responsive=\'{ "srcset": "small.png 148w, big.png 1024w", "width": "1024" }\'>' +
+					'</image>'
+				);
+
+				expect( editor.getData() ).to.equal(
+					'<figure class="image">' +
+						'<img srcset="small.png 148w, big.png 1024w" sizes="100vw" width="1024" alt="alt text" src="foo.png">' +
+					'</figure>'
+				);
+			} );
+
+			it( 'should not convert responsive attribute if is already consumed', () => {
+				editor.data.modelToView.on( 'addAttribute:responsive:image', ( evt, data, consumable ) => {
+					const parts = evt.name.split( ':' );
+					const consumableType = parts[ 0 ] + ':' + parts[ 1 ];
+					const modelImage = data.item;
+
+					consumable.consume( modelImage, consumableType );
+				}, { priority: 'high' } );
+
+				setModelData( document,
+					'<image ' +
+						'src="foo.png" ' +
+						'alt="alt text" ' +
+						'responsive=\'{ "srcset": "small.png 148w, big.png 1024w", "width": "1024" }\'>' +
+					'</image>'
+				);
+
+				expect( editor.getData() ).to.equal(
+					'<figure class="image">' +
+						'<img alt="alt text" src="foo.png">' +
+					'</figure>'
+				);
+			} );
+
+			it( 'should not convert responsive attribute if has wrong data', () => {
+				setModelData( document,
+					'<image ' +
+						'src="foo.png" ' +
+						'alt="alt text" ' +
+						'responsive=\'{ "foo":"bar" }\'>' +
+					'</image>' );
+
+				const image = document.getRoot().getChild( 0 );
+				document.enqueueChanges( () => {
+					const batch = document.batch();
+
+					batch.removeAttribute( image, 'responsive' );
+				} );
+
+				expect( editor.getData() ).to.equal(
+					'<figure class="image">' +
+						'<img alt="alt text" src="foo.png">' +
+					'</figure>'
+				);
+			} );
 		} );
 
 		describe( 'view to model', () => {
@@ -193,7 +257,18 @@ describe( 'ImageEngine', () => {
 				);
 
 				expect( getModelData( document, { withoutSelection: true } ) )
-					.to.equal( '<image alt="alt text" src="foo.png" srcset="small.png 148w, big.png 1024w"></image>' );
+					.to.equal( '<image alt="alt text" responsive="{"srcset":"small.png 148w, big.png 1024w"}" src="foo.png"></image>' );
+			} );
+
+			it( 'should convert image with srcset and width attributes', () => {
+				editor.setData(
+					'<figure class="image">' +
+					'<img src="foo.png" alt="alt text" srcset="small.png 148w, big.png 1024w" width="1024" />' +
+					'</figure>'
+				);
+
+				expect( getModelData( document, { withoutSelection: true } ) ).to.equal(
+					'<image alt="alt text" responsive="{"srcset":"small.png 148w, big.png 1024w","width":"1024"}" src="foo.png"></image>' );
 			} );
 
 			it( 'should ignore sizes attribute', () => {
@@ -204,7 +279,7 @@ describe( 'ImageEngine', () => {
 				);
 
 				expect( getModelData( document, { withoutSelection: true } ) )
-					.to.equal( '<image alt="alt text" src="foo.png" srcset="small.png 148w, big.png 1024w"></image>' );
+					.to.equal( '<image alt="alt text" responsive="{"srcset":"small.png 148w, big.png 1024w"}" src="foo.png"></image>' );
 			} );
 
 			describe( 'should autohoist images', () => {
@@ -385,8 +460,13 @@ describe( 'ImageEngine', () => {
 				);
 			} );
 
-			it( 'should convert srcset attribute to srcset and sizes', () => {
-				setModelData( document, '<image src="foo.png" alt="alt text" srcset="small.png 148w, big.png 1024w"></image>' );
+			it( 'should convert responsive attribute to srcset and sizes', () => {
+				setModelData( document,
+					'<image ' +
+						'src="foo.png" ' +
+						'alt="alt text" ' +
+						'responsive=\'{ "srcset":"small.png 148w, big.png 1024w" }\'>' +
+					'</image>' );
 
 				expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
 					'<figure class="image ck-widget" contenteditable="false">' +
@@ -395,14 +475,51 @@ describe( 'ImageEngine', () => {
 				);
 			} );
 
-			it( 'should remove sizes attribute when srcset attribute is removed', () => {
-				setModelData( document, '<image src="foo.png" srcset="small.png 148w, big.png 1024w"></image>' );
+			it( 'should not convert responsive attribute if has wrong data', () => {
+				setModelData( document,
+					'<image ' +
+						'src="foo.png" ' +
+						'alt="alt text" ' +
+						'responsive=\'{ "foo":"bar" }\'>' +
+					'</image>' );
+
+				const image = document.getRoot().getChild( 0 );
+				document.enqueueChanges( () => {
+					const batch = document.batch();
+
+					batch.removeAttribute( image, 'responsive' );
+				} );
+
+				expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
+					'<figure class="image ck-widget" contenteditable="false">' +
+						'<img alt="alt text" src="foo.png"></img>' +
+					'</figure>'
+				);
+			} );
+
+			it( 'should convert responsive attribute to srcset, width and sizes', () => {
+				setModelData( document,
+					'<image ' +
+						'src="foo.png" ' +
+						'alt="alt text" ' +
+						'responsive=\'{ "srcset":"small.png 148w, big.png 1024w", "width":"1024" }\'>' +
+					'</image>' );
+
+				expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
+					'<figure class="image ck-widget" contenteditable="false">' +
+						'<img alt="alt text" sizes="100vw" src="foo.png" srcset="small.png 148w, big.png 1024w" width="1024"></img>' +
+					'</figure>'
+				);
+			} );
+
+			it( 'should remove sizes and srcsset attribute when responsive attribute is removed', () => {
+				setModelData( document, '<image src="foo.png" responsive=\'{ "srcset": "small.png 148w, big.png 1024w" }\'></image>' );
 				const image = document.getRoot().getChild( 0 );
 
 				document.enqueueChanges( () => {
 					const batch = document.batch();
 
-					batch.removeAttribute( image, 'srcset' );
+					batch.removeAttribute( image, 'responsive' );
 				} );
 
 				expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
@@ -411,6 +528,52 @@ describe( 'ImageEngine', () => {
 					'</figure>'
 				);
 			} );
+
+			it( 'should remove width, sizes and srcsset attribute when responsive attribute is removed', () => {
+				setModelData( document,
+					'<image ' +
+						'src="foo.png" ' +
+						'responsive=\'{ "srcset": "small.png 148w, big.png 1024w", "width": "1024" }\'>' +
+					'</image>'
+				);
+				const image = document.getRoot().getChild( 0 );
+
+				document.enqueueChanges( () => {
+					const batch = document.batch();
+
+					batch.removeAttribute( image, 'responsive' );
+				} );
+
+				expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
+					'<figure class="image ck-widget" contenteditable="false">' +
+					'<img src="foo.png"></img>' +
+					'</figure>'
+				);
+			} );
+
+			it( 'should not convert responsive attribute if is already consumed', () => {
+				editor.editing.modelToView.on( 'addAttribute:responsive:image', ( evt, data, consumable ) => {
+					const parts = evt.name.split( ':' );
+					const consumableType = parts[ 0 ] + ':' + parts[ 1 ];
+					const modelImage = data.item;
+
+					consumable.consume( modelImage, consumableType );
+				}, { priority: 'high' } );
+
+				setModelData( document,
+					'<image ' +
+						'src="foo.png" ' +
+						'alt="alt text" ' +
+						'responsive=\'{ "srcset": "small.png 148w, big.png 1024w", "width": "1024" }\'>' +
+					'</image>'
+				);
+
+				expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
+					'<figure class="image ck-widget" contenteditable="false">' +
+						'<img alt="alt text" src="foo.png"></img>' +
+					'</figure>'
+				);
+			} );
 		} );
 	} );
 } );

+ 1 - 1
packages/ckeditor5-image/tests/imagestyle/imagestyleengine.js

@@ -141,7 +141,7 @@ describe( 'ImageStyleEngine', () => {
 		} );
 
 		it( 'should convert model to view: removing attribute', () => {
-			setModelData( document, '<image src="foo.png" imageStyle="imageStyleSide"></image>' );
+			setModelData( document, '<image src="foo.png" imageStyle="sideStyle"></image>' );
 			const image = document.getRoot().getChild( 0 );
 			const batch = document.batch();