8
0
Просмотр исходного кода

Updated tests, fixed selection converter.

Szymon Kupś 9 лет назад
Родитель
Сommit
1ccaadd6a0

+ 23 - 22
packages/ckeditor5-image/src/converters.js

@@ -11,11 +11,8 @@ import { isImageWidget } from './utils.js';
 const WIDGET_SELECTED_CLASS_NAME = 'ck-widget_selected';
 
 /**
- * Returns model to view selection converter. If model selection is placed around image widget:
- *
- *		[<image src="..." alt="..."></image>]
- *
- * then it will be converted into fake selection.
+ * Returns model to view selection converter. This converter is used after default selection conversion is made.
+ * It creates fake view selection when {@link engine.view.Selection#getSelectedElement} returns instance of image widget.
  *
  * @param {Function} t {@link utils.Locale#t Locale#t function} used to translate default fake selection's label.
  * @returns {Function}
@@ -27,16 +24,18 @@ export function modelToViewSelection( t ) {
 		const viewSelection = conversionApi.viewSelection;
 		const selectedElement = viewSelection.getSelectedElement();
 
-		if ( !selectedElement || !isImageWidget( selectedElement ) ) {
-			return;
-		}
-
+		// Remove selected class from previously selected widget.
 		if ( previouslySelected && previouslySelected.hasClass( WIDGET_SELECTED_CLASS_NAME ) ) {
 			previouslySelected.removeClass( WIDGET_SELECTED_CLASS_NAME );
 		}
 
+		if ( !selectedElement || !isImageWidget( selectedElement ) ) {
+			return;
+		}
+
 		let fakeSelectionLabel = t( 'image widget' );
-		const altText = selectedElement.getAttribute( 'alt' );
+		const imgElement = selectedElement.getChild( 0 );
+		const altText = imgElement.getAttribute( 'alt' );
 
 		if ( altText ) {
 			fakeSelectionLabel = `${ altText } ${ fakeSelectionLabel }`;
@@ -84,26 +83,25 @@ export function viewToModelImage() {
 			return;
 		}
 
-		// Check if 'src' attribute can be converted.
-		if ( !viewImg.hasAttribute( 'src' ) || !consumable.consume( viewImg, { attributes: [ 'src' ] } ) ) {
-			return;
-		}
-
-		// Check if 'alt' attribute can be converted.
-		if ( !viewImg.hasAttribute( 'alt' ) || !consumable.consume( viewImg, { attributes: [ 'alt' ] } ) ) {
+		// Consume img element.
+		if ( !consumable.consume( viewImg, { name: true } ) ) {
 			return;
 		}
 
-		// Consume img element.
-		if ( !consumable.consume( viewImg, { name: true } ) ) {
+		// Check if 'src' attribute can be converted - it is required attribute.
+		if ( !consumable.consume( viewImg, { attribute: [ 'src' ] } ) ) {
 			return;
 		}
 
 		// Create model element.
 		const modelImage = new ModelElement( 'image' );
 
+		// Not required attribute - convert if present.
+		if ( consumable.consume( viewImg, { attribute: [ 'alt' ] } ) ) {
+			modelImage.setAttribute( 'alt', viewImg.getAttribute( 'alt' ) );
+		}
+
 		modelImage.setAttribute( 'src', viewImg.getAttribute( 'src' ) );
-		modelImage.setAttribute( 'alt', viewImg.getAttribute( 'alt' ) );
 
 		data.output = modelImage;
 	};
@@ -119,9 +117,12 @@ export function viewToModelImage() {
  */
 export function modelToViewImage( modelElement ) {
 	const viewImg = new ViewEmptyElement( 'img', {
-		src: modelElement.getAttribute( 'src' ),
-		alt: modelElement.getAttribute( 'alt' )
+		src: modelElement.getAttribute( 'src' )
 	} );
 
+	if ( modelElement.hasAttribute( 'alt' ) ) {
+		viewImg.setAttribute( 'alt', modelElement.getAttribute( 'alt' ) );
+	}
+
 	return new ViewContainerElement( 'figure', { class: 'image' }, viewImg );
 }

+ 1 - 1
packages/ckeditor5-image/src/image.js

@@ -47,7 +47,7 @@ export default class Image extends Feature {
 		let widgetElement = domEventData.target;
 		const viewDocument = this.editor.editing.view;
 
-		// If target is not a widgetElement - check if one of the parents is.
+		// If target is not a widget element - check if one of the ancestors is.
 		if ( !isImageWidget( widgetElement ) ) {
 			widgetElement = widgetElement.findAncestor( element => isImageWidget( element ) );
 

+ 17 - 3
packages/ckeditor5-image/tests/image.js

@@ -9,7 +9,7 @@ import ClassicTestEditor from 'tests/core/_utils/classictesteditor.js';
 import Image from 'ckeditor5/image/image.js';
 import Paragraph from 'ckeditor5/paragraph/paragraph.js';
 import ImageEngine from 'ckeditor5/image/imageengine.js';
-import MouseDownObserver from 'ckeditor5/image/mousedownobserver.js';
+import MouseObserver from 'ckeditor5/engine/view/observer/mouseobserver.js';
 import { getData as getModelData } from 'ckeditor5/engine/dev-utils/model.js';
 
 describe( 'Image', () => {
@@ -38,8 +38,8 @@ describe( 'Image', () => {
 		expect( editor.plugins.get( ImageEngine ) ).to.instanceOf( ImageEngine );
 	} );
 
-	it( 'should add MouseDownObserver', () => {
-		expect( viewDocument.getObserver( MouseDownObserver ) ).to.be.instanceOf( MouseDownObserver );
+	it( 'should add MouseObserver', () => {
+		expect( viewDocument.getObserver( MouseObserver ) ).to.be.instanceOf( MouseObserver );
 	} );
 
 	it( 'should create selection in model on mousedown event', () => {
@@ -56,6 +56,20 @@ describe( 'Image', () => {
 		expect( getModelData( document ) ).to.equal( '[<image alt="alt text" src="image.png"></image>]' );
 	} );
 
+	it( 'should create selection in model on mousedown event when target is widget\'s child element', () => {
+		editor.setData( '<figure class="image"><img src="image.png" alt="alt text" /></figure>' );
+		const imgElement = viewDocument.getRoot().getChild( 0 ).getChild( 0 );
+		const domEventDataMock = {
+			target: imgElement,
+			preventDefault: sinon.spy()
+		};
+
+		viewDocument.fire( 'mousedown', domEventDataMock );
+
+		sinon.assert.calledOnce( domEventDataMock.preventDefault );
+		expect( getModelData( document ) ).to.equal( '[<image alt="alt text" src="image.png"></image>]' );
+	} );
+
 	it( 'should do not change model if mousedown event is not on image', () => {
 		editor.setData( '<p>foo bar</p><figure class="image"><img src="image.png" alt="alt text" /></figure>' );
 

+ 8 - 2
packages/ckeditor5-image/tests/imageengine.js

@@ -40,6 +40,12 @@ describe( `ImageEngine`, () => {
 
 				expect( editor.getData() ).to.equal( '<figure class="image"><img src="foo.png" alt="alt text"></figure>' );
 			} );
+
+			it( 'should convert without alt attribute', () => {
+				setModelData( document, '<image src="foo.png"></image>' );
+
+				expect( editor.getData() ).to.equal( '<figure class="image"><img src="foo.png"></figure>' );
+			} );
 		} );
 
 		describe( 'view to model', () => {
@@ -71,11 +77,11 @@ describe( `ImageEngine`, () => {
 					.to.equal( '' );
 			} );
 
-			it( 'should not convert without alt attribute', () => {
+			it( 'should convert without alt attribute', () => {
 				editor.setData( '<figure class="image"><img src="foo.png" /></figure>' );
 
 				expect( getModelData( document, { withoutSelection: true } ) )
-					.to.equal( '' );
+					.to.equal( '<image src="foo.png"></image>' );
 			} );
 
 			it( 'should not convert without src attribute', () => {

+ 1 - 1
packages/ckeditor5-image/theme/theme.scss

@@ -1,4 +1,4 @@
-// Common styles applied to all widget.
+// Common styles applied to all widgets.
 .ck-widget {
 	&.ck-widget_selected, &.ck-widget_selected:hover {
 		outline: 2px solid #ace;