Merge pull request #51 from ckeditor/t/9

Fix: Enhanced how selection labels for widgets are defined. Closes #9.

src/image/converters.js

@@ -10,7 +10,6 @@
 import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
 import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
 import modelWriter from '@ckeditor/ckeditor5-engine/src/model/writer';
-import { isImageWidget } from './utils';
 
 /**
  * Returns function that converts image view representation:
@@ -71,35 +70,6 @@ export function viewToModelImage() {
 	};
 }
 
-/**
- * Returns model to view selection converter. This converter is applied after default selection conversion is made.
- * It creates fake view selection when {@link module:engine/view/selection~Selection#getSelectedElement} returns instance
- * of image widget.
- *
- * @param {Function} t {@link module:utils/locale~Locale#t Locale#t function} used to translate default fake selection's label.
- * @returns {Function}
- */
-export function modelToViewSelection( t ) {
-	return ( evt, data, consumable, conversionApi ) => {
-		const viewSelection = conversionApi.viewSelection;
-		const selectedElement = viewSelection.getSelectedElement();
-
-		if ( !selectedElement || !isImageWidget( selectedElement ) ) {
-			return;
-		}
-
-		let fakeSelectionLabel = t( 'image widget' );
-		const imgElement = selectedElement.getChild( 0 );
-		const altText = imgElement.getAttribute( 'alt' );
-
-		if ( altText ) {
-			fakeSelectionLabel = `${ altText } ${ fakeSelectionLabel }`;
-		}
-
-		viewSelection.setFake( true, { label: fakeSelectionLabel } );
-	};
-}
-
 /**
  * Creates image attribute converter for provided model conversion dispatchers.
  *

src/image/imageengine.js

@@ -10,7 +10,7 @@
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';
 import WidgetEngine from '../widget/widgetengine';
-import { viewToModelImage, modelToViewSelection, createImageAttributeConverter } from './converters';
+import { viewToModelImage, createImageAttributeConverter } from './converters';
 import { toImageWidget } from './utils';
 import ViewContainerElement from '@ckeditor/ckeditor5-engine/src/view/containerelement';
 import ViewEmptyElement from '@ckeditor/ckeditor5-engine/src/view/emptyelement';
@@ -39,6 +39,7 @@ export default class ImageEngine extends Plugin {
 		const schema = doc.schema;
 		const data = editor.data;
 		const editing = editor.editing;
+		const t = editor.t;
 
 		// Configure schema.
 		schema.registerItem( 'image' );
@@ -54,16 +55,13 @@ export default class ImageEngine extends Plugin {
 		// Build converter from model to view for editing pipeline.
 		buildModelConverter().for( editing.modelToView )
 			.fromElement( 'image' )
-			.toElement( () => toImageWidget( createImageViewElement() ) );
+			.toElement( () => toImageWidget( createImageViewElement(), t( 'image widget' ) ) );
 
 		createImageAttributeConverter( [ editing.modelToView, data.modelToView ], 'src' );
 		createImageAttributeConverter( [ editing.modelToView, data.modelToView ], 'alt' );
 
 		// Converter for figure element from view to model.
 		data.viewToModel.on( 'element:figure', viewToModelImage() );
-
-		// Creates fake selection label if selection is placed around image widget.
-		editing.modelToView.on( 'selection', modelToViewSelection( editor.t ), { priority: 'lowest' } );
 	}
 }
 

src/image/utils.js

@@ -15,15 +15,23 @@ const imageSymbol = Symbol( 'isImage' );
 /**
  * Converts given {@link module:engine/view/element~Element} to image widget:
  * * adds {@link module:engine/view/element~Element#setCustomProperty custom property} allowing to recognize image widget element,
- * * calls {@link module:image/widget/utils~widgetize widgetize}.
+ * * calls {@link module:image/widget/utils~widgetize widgetize} function with proper element's label creator.
  *
  * @param {module:engine/view/element~Element} viewElement
+ * @param {String} label Element's label. It will be concatenated with image's `alt` attribute if one is present.
  * @returns {module:engine/view/element~Element}
  */
-export function toImageWidget( viewElement ) {
+export function toImageWidget( viewElement, label ) {
 	viewElement.setCustomProperty( imageSymbol, true );
 
-	return widgetize( viewElement );
+	return widgetize( viewElement, { label: labelCreator } );
+
+	function labelCreator() {
+		const imgElement = viewElement.getChild( 0 );
+		const altText = imgElement.getAttribute( 'alt' );
+
+		return altText ? `${ altText } ${ label }` : label;
+	}
 }
 
 /**

src/widget/utils.js

@@ -8,6 +8,7 @@
  */
 
 const widgetSymbol = Symbol( 'isWidget' );
+const labelSymbol = Symbol( 'label' );
 
 /**
  * CSS class added to each widget element.
@@ -41,17 +42,53 @@ export function isWidget( element ) {
  * * adds custom property allowing to recognize widget elements by using {@link ~isWidget}.
  *
  * @param {module:engine/view/element~Element} element
+ * @param {Object} [options]
+ * @param {String|Function} [options.label] Element's label provided to {@link ~setLabel} function. It can be passed as
+ * a plain string or a function returning a string.
  * @returns {module:engine/view/element~Element} Returns same element.
  */
-export function widgetize( element ) {
+export function widgetize( element, options ) {
+	options = options || {};
 	element.setAttribute( 'contenteditable', false );
 	element.getFillerOffset = getFillerOffset;
 	element.addClass( WIDGET_CLASS_NAME );
 	element.setCustomProperty( widgetSymbol, true );
 
+	if ( options.label ) {
+		setLabel( element, options.label );
+	}
+
 	return element;
 }
 
+/**
+ * Sets label for given element.
+ * It can be passed as a plain string or a function returning a string. Function will be called each time label is retrieved by
+ * {@link ~getLabel}.
+ *
+ * @param {module:engine/view/element~Element} element
+ * @param {String|Function} labelOrCreator
+ */
+export function setLabel( element, labelOrCreator ) {
+	element.setCustomProperty( labelSymbol, labelOrCreator );
+}
+
+/**
+ * Returns label for provided element.
+ *
+ * @param {module:engine/view/element~Element} element
+ * @return {String}
+ */
+export function getLabel( element ) {
+	const labelCreator = element.getCustomProperty( labelSymbol );
+
+	if ( !labelCreator ) {
+		return '';
+	}
+
+	return typeof labelCreator == 'function' ? labelCreator() : labelCreator;
+}
+
 // Default filler offset function applied to all widget elements.
 //
 // @returns {null}

src/widget/widgetengine.js

@@ -8,7 +8,7 @@
  */
 
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
-import { WIDGET_SELECTED_CLASS_NAME, isWidget } from './utils';
+import { WIDGET_SELECTED_CLASS_NAME, isWidget, getLabel } from './utils';
 
 /**
  * The widget engine plugin.
@@ -42,7 +42,7 @@ export default class WidgetEngine extends Plugin {
 				return;
 			}
 
-			viewSelection.setFake( true );
+			viewSelection.setFake( true, { label: getLabel( selectedElement ) } );
 			selectedElement.addClass( WIDGET_SELECTED_CLASS_NAME );
 			previouslySelected = selectedElement;
 		}, { priority: 'low' } );

tests/image/converters.js

@@ -3,7 +3,7 @@
  * For licensing, see LICENSE.md.
  */
 
-import { viewToModelImage, modelToViewSelection, createImageAttributeConverter } from '../../src/image/converters';
+import { viewToModelImage, createImageAttributeConverter } from '../../src/image/converters';
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
 import { createImageViewElement } from '../../src/image/imageengine';
 import { toImageWidget } from '../../src/image/utils';
@@ -32,7 +32,6 @@ describe( 'Image converters', () => {
 					.fromElement( 'image' )
 					.toElement( () => toImageWidget( createImageViewElement() ) );
 
-				editor.editing.modelToView.on( 'selection', modelToViewSelection( editor.t ), { priority: 'lowest' } );
 				buildModelConverter().for( editor.editing.modelToView )
 					.fromElement( 'image' )
 					.toElement( () => toImageWidget( createImageViewElement() ) );
@@ -85,28 +84,6 @@ describe( 'Image converters', () => {
 		} );
 	} );
 
-	describe( 'modelToViewSelection', () => {
-		it( 'should convert selection over image to fake selection', () => {
-			setModelData( document, '[<image src=""></image>]' );
-
-			expect( viewDocument.selection.isFake ).to.be.true;
-			expect( viewDocument.selection.fakeSelectionLabel ).to.equal( 'image widget' );
-		} );
-
-		it( 'should convert fake selection label with alt', () => {
-			setModelData( document, '[<image src="" alt="foo bar"></image>]' );
-
-			expect( viewDocument.selection.isFake ).to.be.true;
-			expect( viewDocument.selection.fakeSelectionLabel ).to.equal( 'foo bar image widget' );
-		} );
-
-		it( 'should not convert fake selection if image is not selected', () => {
-			setModelData( document, '[]<image src="" alt="foo bar"></image>' );
-
-			expect( viewDocument.selection.isFake ).to.be.false;
-		} );
-	} );
-
 	describe( 'modelToViewAttributeConverter', () => {
 		it( 'should convert adding attribute to image', () => {
 			setModelData( document, '<image src=""></image>' );

tests/image/utils.js

@@ -6,20 +6,37 @@
 import ViewElement from '@ckeditor/ckeditor5-engine/src/view/element';
 import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
 import { toImageWidget, isImageWidget, isImage } from '../../src/image/utils';
-import { isWidget } from '../../src/widget/utils';
+import { isWidget, getLabel } from '../../src/widget/utils';
 
 describe( 'image widget utils', () => {
-	let element;
+	let element, image;
 
 	beforeEach( () => {
-		element = new ViewElement( 'div' );
-		toImageWidget( element );
+		image = new ViewElement( 'img' );
+		element = new ViewElement( 'figure', null, image );
+		toImageWidget( element, 'image widget' );
 	} );
 
 	describe( 'toImageWidget()', () => {
 		it( 'should be widgetized', () => {
 			expect( isWidget( element ) ).to.be.true;
 		} );
+
+		it( 'should set element\'s label', () => {
+			expect( getLabel( element ) ).to.equal( 'image widget' );
+		} );
+
+		it( 'should set element\'s label combined with alt attribute', () => {
+			image.setAttribute( 'alt', 'foo bar baz' );
+			expect( getLabel( element ) ).to.equal( 'foo bar baz image widget' );
+		} );
+
+		it( 'provided label creator should always return same label', () => {
+			image.setAttribute( 'alt', 'foo bar baz' );
+
+			expect( getLabel( element ) ).to.equal( 'foo bar baz image widget' );
+			expect( getLabel( element ) ).to.equal( 'foo bar baz image widget' );
+		} );
 	} );
 
 	describe( 'isImageWidget()', () => {

tests/widget/utils.js

@@ -4,7 +4,7 @@
  */
 
 import ViewElement from '@ckeditor/ckeditor5-engine/src/view/element';
-import { widgetize, isWidget, WIDGET_CLASS_NAME } from '../../src/widget/utils';
+import { widgetize, isWidget, WIDGET_CLASS_NAME, setLabel, getLabel } from '../../src/widget/utils';
 
 describe( 'widget utils', () => {
 	let element;
@@ -27,6 +27,20 @@ describe( 'widget utils', () => {
 		it( 'should add proper CSS class', () => {
 			expect( element.hasClass( WIDGET_CLASS_NAME ) ).to.be.true;
 		} );
+
+		it( 'should add element\'s label if one is provided', () => {
+			element = new ViewElement( 'div' );
+			widgetize( element, { label: 'foo bar baz label' } );
+
+			expect( getLabel( element ) ).to.equal( 'foo bar baz label' );
+		} );
+
+		it( 'should add element\'s label if one is provided as function', () => {
+			element = new ViewElement( 'div' );
+			widgetize( element, { label: () => 'foo bar baz label' } );
+
+			expect( getLabel( element ) ).to.equal( 'foo bar baz label' );
+		} );
 	} );
 
 	describe( 'isWidget()', () => {
@@ -38,4 +52,29 @@ describe( 'widget utils', () => {
 			expect( isWidget( new ViewElement( 'p' ) ) ).to.be.false;
 		} );
 	} );
+
+	describe( 'label utils', () => {
+		it( 'should allow to set label for element', () => {
+			const element = new ViewElement( 'p' );
+			setLabel( element, 'foo bar baz' );
+
+			expect( getLabel( element ) ).to.equal( 'foo bar baz' );
+		} );
+
+		it( 'should return empty string for elements without label', () => {
+			const element = new ViewElement( 'div' );
+
+			expect( getLabel( element ) ).to.equal( '' );
+		} );
+
+		it( 'should allow to use a function as label creator', () => {
+			const element = new ViewElement( 'p' );
+			let caption = 'foo';
+			setLabel( element, () => caption );
+
+			expect( getLabel( element ) ).to.equal( 'foo' );
+			caption = 'bar';
+			expect( getLabel( element ) ).to.equal( 'bar' );
+		} );
+	} );
 } );

tests/widget/widgetengine.js

@@ -31,7 +31,11 @@ describe( 'WidgetEngine', () => {
 
 				buildModelConverter().for( editor.editing.modelToView )
 					.fromElement( 'widget' )
-					.toElement( () => widgetize( new ViewContainer( 'div' ) ) );
+					.toElement( () => {
+						const element = widgetize( new ViewContainer( 'div' ), { label: 'element label' } );
+
+						return element;
+					} );
 
 				buildModelConverter().for( editor.editing.modelToView )
 					.fromElement( 'editable' )
@@ -52,6 +56,18 @@ describe( 'WidgetEngine', () => {
 		expect( viewDocument.selection.isFake ).to.be.true;
 	} );
 
+	it( 'should use element\'s label to set fake selection if one is provided', () => {
+		setModelData( document, '[<widget>foo bar</widget>]' );
+
+		expect( viewDocument.selection.fakeSelectionLabel ).to.equal( 'element label' );
+	} );
+
+	it( 'fake selection should be empty if widget is not selected', () => {
+		setModelData( document, '<widget>foo bar</widget>' );
+
+		expect( viewDocument.selection.fakeSelectionLabel ).to.equal( '' );
+	} );
+
 	it( 'should toggle selected class', () => {
 		setModelData( document, '[<widget>foo</widget>]' );