Преглед изворни кода

Split marker conversion to separately convert texts and elements inside.

Szymon Kupś пре 8 година
родитељ
комит
7920832107

+ 9 - 3
packages/ckeditor5-engine/src/conversion/buildmodelconverter.js

@@ -15,7 +15,8 @@ import {
 	removeUIElement,
 	wrapItem,
 	unwrapItem,
-	markerToVirtualSelection
+	convertTextsInsideMarker,
+	convertElementsInsideMarker
 } from './model-to-view-converters';
 
 import { convertSelectionAttribute, convertSelectionMarker } from './model-selection-to-view-converters';
@@ -322,8 +323,13 @@ class ModelConverterBuilder {
 		}
 
 		for ( const dispatcher of this._dispatchers ) {
-			dispatcher.on( 'addMarker:' + this._from.name, markerToVirtualSelection( selectionDescriptor ), { priority } );
-			dispatcher.on( 'removeMarker:' + this._from.name, markerToVirtualSelection( selectionDescriptor ), { priority } );
+			// Separate converters for converting texts and elements inside marker's range.
+			dispatcher.on( 'addMarker:' + this._from.name, convertTextsInsideMarker( selectionDescriptor ), { priority } );
+			dispatcher.on( 'addMarker:' + this._from.name, convertElementsInsideMarker( selectionDescriptor ), { priority } );
+
+			dispatcher.on( 'removeMarker:' + this._from.name, convertTextsInsideMarker( selectionDescriptor ), { priority } );
+			dispatcher.on( 'removeMarker:' + this._from.name, convertElementsInsideMarker( selectionDescriptor ), { priority } );
+
 			dispatcher.on( 'selectionMarker:' + this._from.name, convertSelectionMarker( selectionDescriptor ), { priority } );
 		}
 	}

+ 72 - 57
packages/ckeditor5-engine/src/conversion/model-to-view-converters.js

@@ -265,11 +265,9 @@ export function removeAttribute( attributeCreator ) {
  *
  * @param {module:engine/view/element~Element|Function} elementCreator View element, or function returning a view element, which will
  * be used for wrapping.
- * @param {Function} [nameToConsumable] Optional function used to map conversion event name to consumable name. By default it
- * uses {engine/conversion/model-to-view-converters~eventNameToConsumableType} function.
  * @returns {Function} Set/change attribute converter.
  */
-export function wrapItem( elementCreator, nameToConsumable = eventNameToConsumableType ) {
+export function wrapItem( elementCreator ) {
 	return ( evt, data, consumable, conversionApi ) => {
 		const viewElement = ( elementCreator instanceof ViewElement ) ?
 			elementCreator.clone( true ) :
@@ -279,7 +277,7 @@ export function wrapItem( elementCreator, nameToConsumable = eventNameToConsumab
 			return;
 		}
 
-		if ( !consumable.consume( data.item, nameToConsumable( evt.name ) ) ) {
+		if ( !consumable.consume( data.item, eventNameToConsumableType( evt.name ) ) ) {
 			return;
 		}
 
@@ -317,11 +315,9 @@ export function wrapItem( elementCreator, nameToConsumable = eventNameToConsumab
  * @see module:engine/conversion/model-to-view-converters~wrapItem
  * @param {module:engine/view/element~Element|Function} elementCreator View element, or function returning a view element, which will
  * be used for unwrapping.
- * @param {Function} [nameToConsumable] Optional function used to map conversion event name to consumable name. By default it
- * uses {engine/conversion/model-to-view-converters~eventNameToConsumableType} function.
  * @returns {Function} Remove attribute converter.
  */
-export function unwrapItem( elementCreator, nameToConsumable = eventNameToConsumableType ) {
+export function unwrapItem( elementCreator ) {
 	return ( evt, data, consumable, conversionApi ) => {
 		const viewElement = ( elementCreator instanceof ViewElement ) ?
 			elementCreator.clone( true ) :
@@ -331,7 +327,7 @@ export function unwrapItem( elementCreator, nameToConsumable = eventNameToConsum
 			return;
 		}
 
-		if ( !consumable.consume( data.item, nameToConsumable( evt.name ) ) ) {
+		if ( !consumable.consume( data.item, eventNameToConsumableType( evt.name ) ) ) {
 			return;
 		}
 
@@ -397,79 +393,78 @@ export function remove() {
 }
 
 /**
- * Function factory, crates marker to virtual selection converter.
- * For more information see {@link module:engine/conversion/buildmodelconverter~ModelConverterBuilder#toVirtualSelection}.
+ * Function factory, creates converter that converts all texts inside marker's range. Converter wraps each text with
+ * {@link module:engine/view/attributeelement~AttributeElement} created from provided descriptor.
+ * See {@link engine/conversion/model-to-view-converters~virtualSelectionDescriptorToAttributeElement}.
  *
  * @param {module:engine/conversion/buildmodelconverter~VirtualSelectionDescriptor|Function} selectionDescriptor
- * @return {Function} Marker to virtual selection converter.
+ * @return {Function}
  */
-export function markerToVirtualSelection( selectionDescriptor ) {
-	return ( evt, data, consumable, conversionApi ) =>	{
+export function convertTextsInsideMarker( selectionDescriptor ) {
+	return ( evt, data, consumable, conversionApi ) => {
 		const descriptor = typeof selectionDescriptor == 'function' ?
 			selectionDescriptor( data, consumable, conversionApi ) :
 			selectionDescriptor;
 
-		if ( !descriptor ) {
+		const modelItem = data.item;
+
+		if ( !descriptor || data.markerRange.isCollapsed || !modelItem.is( 'textProxy' ) ) {
 			return;
 		}
 
-		// Do not convert if marker's range is collapsed.
-		if ( data.markerRange.isCollapsed ) {
+		if ( !consumable.consume( modelItem, evt.name ) ) {
 			return;
 		}
 
-		const addMarker = evt.name.split( ':' )[ 0 ] == 'addMarker';
-		const modelItem = data.item;
-
-		if ( modelItem.is( 'textProxy' ) ) {
-			const viewElement = virtualSelectionDescriptorToAttributeElement( descriptor );
-			const converter = addMarker ?
-				wrapItem( viewElement, eventName => eventName ) :
-				unwrapItem( viewElement, eventName => eventName );
+		const viewElement = virtualSelectionDescriptorToAttributeElement( descriptor );
+		const viewRange = conversionApi.mapper.toViewRange( data.range );
 
-			converter( evt, data, consumable, conversionApi );
+		if ( evt.name.split( ':' )[ 0 ] == 'addMarker' ) {
+			viewWriter.wrap( viewRange, viewElement );
+		} else {
+			viewWriter.unwrap( viewRange, viewElement );
 		}
+	};
+}
 
-		if ( modelItem.is( 'element' ) ) {
-			const consumableType = evt.name;
-			const viewElement = conversionApi.mapper.toViewElement( modelItem );
-
-			if ( !consumable.consume( modelItem, consumableType ) ) {
-				return;
-			}
-
-			const selectionHandlingMethod = addMarker ? 'setVirtualSelection' : 'removeVirtualSelection';
+/**
+ * Function factory, creates converter that converts all elements inside marker's range. Converter checks if element has
+ * functions stored under `setVirtualSelection` and `removeVirtualSelection` custom properties and calls them passing
+ * {@link module:engine/conversion/buildmodelconverter~VirtualSelectionDescriptor. In such case converter will consume
+ * all element's children, assuming that they were handled by element itself.
+ *
+ * @param {module:engine/conversion/buildmodelconverter~VirtualSelectionDescriptor|Function} selectionDescriptor
+ * @return {Function}
+ */
+export function convertElementsInsideMarker( selectionDescriptor ) {
+	return ( evt, data, consumable, conversionApi ) => {
+		const descriptor = typeof selectionDescriptor == 'function' ?
+			selectionDescriptor( data, consumable, conversionApi ) :
+			selectionDescriptor;
 
-			if ( viewElement && viewElement.getCustomProperty( selectionHandlingMethod ) ) {
-				// Virtual selection will be handled by parent element - consume all children.
-				for ( const value of ModelRange.createIn( modelItem ) ) {
-					consumable.consume( value.item, consumableType );
-				}
+		const modelItem = data.item;
 
-				viewElement.getCustomProperty( selectionHandlingMethod )( viewElement, descriptor );
-			}
+		if ( !descriptor || data.markerRange.isCollapsed || !modelItem.is( 'element' ) ) {
+			return;
 		}
-	};
-}
 
-// Helper function that shifts given view `position` in a way that returned position is after `howMany` characters compared
-// to the original `position`.
-// Because in view there might be view ui elements splitting text nodes, we cannot simply use `ViewPosition#getShiftedBy()`.
-function _shiftViewPositionByCharacters( position, howMany ) {
-	// Create a walker that will walk the view tree starting from given position and walking characters one-by-one.
-	const walker = new ViewTreeWalker( { startPosition: position, singleCharacters: true } );
-	// We will count visited characters and return the position after `howMany` characters.
-	let charactersFound = 0;
+		if ( !consumable.consume( data.item, evt.name ) ) {
+			return;
+		}
 
-	for ( const value of walker ) {
-		if ( value.type == 'text' ) {
-			charactersFound++;
+		const viewElement = conversionApi.mapper.toViewElement( modelItem );
+		const addMarker = evt.name.split( ':' )[ 0 ] == 'addMarker';
+		const selectionHandlingMethod = addMarker ? 'setVirtualSelection' : 'removeVirtualSelection';
 
-			if ( charactersFound == howMany ) {
-				return walker.position;
+		if ( viewElement && viewElement.getCustomProperty( selectionHandlingMethod ) ) {
+			// Virtual selection will be handled by parent element - consume all children.
+			for ( const value of ModelRange.createIn( modelItem ) ) {
+				consumable.consume( value.item, evt.name );
 			}
+
+			viewElement.getCustomProperty( selectionHandlingMethod )( viewElement, descriptor );
 		}
-	}
+	};
 }
 
 /**
@@ -558,3 +553,23 @@ export function virtualSelectionDescriptorToAttributeElement( descriptor ) {
 
 	return attributeElement;
 }
+
+// Helper function that shifts given view `position` in a way that returned position is after `howMany` characters compared
+// to the original `position`.
+// Because in view there might be view ui elements splitting text nodes, we cannot simply use `ViewPosition#getShiftedBy()`.
+function _shiftViewPositionByCharacters( position, howMany ) {
+	// Create a walker that will walk the view tree starting from given position and walking characters one-by-one.
+	const walker = new ViewTreeWalker( { startPosition: position, singleCharacters: true } );
+	// We will count visited characters and return the position after `howMany` characters.
+	let charactersFound = 0;
+
+	for ( const value of walker ) {
+		if ( value.type == 'text' ) {
+			charactersFound++;
+
+			if ( charactersFound == howMany ) {
+				return walker.position;
+			}
+		}
+	}
+}

+ 7 - 4
packages/ckeditor5-engine/tests/conversion/model-selection-to-view-converters.js

@@ -28,7 +28,8 @@ import {
 	insertElement,
 	insertText,
 	wrapItem,
-	markerToVirtualSelection
+	convertTextsInsideMarker,
+	convertElementsInsideMarker
 } from '../../src/conversion/model-to-view-converters';
 
 import { stringify as stringifyView } from '../../src/dev-utils/view';
@@ -58,8 +59,10 @@ describe( 'model-selection-to-view-converters', () => {
 		dispatcher.on( 'insert:$text', insertText() );
 		dispatcher.on( 'addAttribute:bold', wrapItem( new ViewAttributeElement( 'strong' ) ) );
 
-		dispatcher.on( 'addMarker:marker', markerToVirtualSelection( virtualSelectionDescriptor ) );
-		dispatcher.on( 'removeMarker:marker', markerToVirtualSelection( virtualSelectionDescriptor ) );
+		dispatcher.on( 'addMarker:marker', convertTextsInsideMarker( virtualSelectionDescriptor ) );
+		dispatcher.on( 'addMarker:marker', convertElementsInsideMarker( virtualSelectionDescriptor ) );
+		dispatcher.on( 'removeMarker:marker', convertTextsInsideMarker( virtualSelectionDescriptor ) );
+		dispatcher.on( 'removeMarker:marker', convertElementsInsideMarker( virtualSelectionDescriptor ) );
 
 		// Default selection converters.
 		dispatcher.on( 'selection', clearAttributes(), { priority: 'low' } );
@@ -228,7 +231,7 @@ describe( 'model-selection-to-view-converters', () => {
 				// Convert model to view.
 				dispatcher.convertInsertion( ModelRange.createIn( modelRoot ) );
 				dispatcher.convertMarker( 'addMarker', marker.name, marker.getRange() );
-                //
+
 				const markers = Array.from( modelDoc.markers.getMarkersAtPosition( modelSelection.getFirstPosition() ) );
 				dispatcher.convertSelection( modelSelection, markers );
 

+ 112 - 14
packages/ckeditor5-engine/tests/conversion/model-to-view-converters.js

@@ -28,7 +28,8 @@ import {
 	unwrapItem,
 	remove,
 	removeUIElement,
-	markerToVirtualSelection,
+	convertTextsInsideMarker,
+	convertElementsInsideMarker,
 	virtualSelectionDescriptorToAttributeElement
 } from '../../src/conversion/model-to-view-converters';
 
@@ -81,7 +82,7 @@ describe( 'model-to-view-converters', () => {
 		return result;
 	}
 
-	describe( 'markerToVirtualSelection()', () => {
+	describe( 'convertTextsInsideMarker()', () => {
 		let modelElement1, modelElement2, markerRange;
 		const virtualSelectionDescriptor = {
 			class: 'marker-class',
@@ -104,8 +105,8 @@ describe( 'model-to-view-converters', () => {
 		} );
 
 		it( 'should wrap and unwrap text nodes', () => {
-			dispatcher.on( 'addMarker:marker', markerToVirtualSelection( virtualSelectionDescriptor ) );
-			dispatcher.on( 'removeMarker:marker', markerToVirtualSelection( virtualSelectionDescriptor ) );
+			dispatcher.on( 'addMarker:marker', convertTextsInsideMarker( virtualSelectionDescriptor ) );
+			dispatcher.on( 'removeMarker:marker', convertTextsInsideMarker( virtualSelectionDescriptor ) );
 			dispatcher.convertInsertion( markerRange );
 
 			modelDoc.markers.set( 'marker', markerRange );
@@ -127,10 +128,80 @@ describe( 'model-to-view-converters', () => {
 			expect( viewToString( viewRoot ) ).to.equal( '<div><p>foo</p><p>bar</p></div>' );
 		} );
 
+		it( 'should not convert marker on elements already consumed', () => {
+			const newDescriptor = { class: 'override-class' };
+
+			dispatcher.on( 'addMarker:marker', convertTextsInsideMarker( virtualSelectionDescriptor ) );
+			dispatcher.on( 'addMarker:marker', convertTextsInsideMarker( newDescriptor ), { priority: 'high' } );
+			dispatcher.on( 'removeMarker:marker', convertTextsInsideMarker( virtualSelectionDescriptor ) );
+			dispatcher.on( 'removeMarker:marker', convertTextsInsideMarker( newDescriptor ), { priority: 'high' } );
+			dispatcher.convertInsertion( markerRange );
+
+			modelDoc.markers.set( 'marker', markerRange );
+			dispatcher.convertMarker( 'addMarker', 'marker', markerRange );
+
+			expect( viewToString( viewRoot ) ).to.equal(
+				'<div>' +
+					'<p>' +
+						'<span class="override-class">foo</span>' +
+					'</p>' +
+					'<p>' +
+						'<span class="override-class">bar</span>' +
+					'</p>' +
+				'</div>'
+			);
+
+			dispatcher.convertMarker( 'removeMarker', 'marker', markerRange );
+
+			expect( viewToString( viewRoot ) ).to.equal( '<div><p>foo</p><p>bar</p></div>' );
+		} );
+
+		it( 'should do nothing if descriptor is not provided', () => {
+			dispatcher.on( 'addMarker:marker', convertTextsInsideMarker( () => null ) );
+			dispatcher.on( 'removeMarker:marker', convertTextsInsideMarker( () => null ) );
+
+			dispatcher.convertInsertion( markerRange );
+
+			modelDoc.markers.set( 'marker', markerRange );
+			dispatcher.convertMarker( 'addMarker', 'marker', markerRange );
+
+			expect( viewToString( viewRoot ) ).to.equal( '<div><p>foo</p><p>bar</p></div>' );
+			dispatcher.convertMarker( 'removeMarker', 'marker', markerRange );
+			expect( viewToString( viewRoot ) ).to.equal( '<div><p>foo</p><p>bar</p></div>' );
+		} );
+	} );
+
+	describe( 'convertElementsInsideMarker()', () => {
+		let modelElement1, modelElement2, markerRange;
+		const virtualSelectionDescriptor = {
+			class: 'marker-class',
+			priority: 7,
+			attributes: { title: 'title' }
+		};
+
+		beforeEach( () => {
+			const modelText1 = new ModelText( 'foo' );
+			modelElement1 = new ModelElement( 'paragraph', null, modelText1 );
+			const modelText2 = new ModelText( 'bar' );
+			modelElement2 = new ModelElement( 'paragraph', null, modelText2 );
+
+			modelRoot.appendChildren( modelElement1 );
+			modelRoot.appendChildren( modelElement2 );
+			dispatcher.on( 'insert:paragraph', insertElement( () => new ViewContainerElement( 'p' ) ) );
+			dispatcher.on( 'insert:$text', insertText() );
+
+			markerRange = ModelRange.createIn( modelRoot );
+		} );
+
 		it( 'should use setVirtualSelection and removeVirtualSelection on elements and not convert children nodes', () => {
-			dispatcher.on( 'addMarker:marker', markerToVirtualSelection( virtualSelectionDescriptor ) );
-			dispatcher.on( 'removeMarker:marker', markerToVirtualSelection( virtualSelectionDescriptor ) );
-			dispatcher.on( 'insert:paragraph', insertElement( () => {
+			dispatcher.on( 'addMarker:marker', convertElementsInsideMarker( virtualSelectionDescriptor ) );
+			dispatcher.on( 'removeMarker:marker', convertElementsInsideMarker( virtualSelectionDescriptor ) );
+			dispatcher.on( 'insert:paragraph', insertElement( data => {
+				// Use special converter only for first paragraph.
+				if ( data.item == modelElement2 ) {
+					return;
+				}
+
 				const viewContainer = new ViewContainerElement( 'p' );
 
 				viewContainer.setCustomProperty( 'setVirtualSelection', ( element, descriptor ) => {
@@ -157,7 +228,7 @@ describe( 'model-to-view-converters', () => {
 					'<p class="virtual-selection-own-class">' +
 						'foo' +
 					'</p>' +
-					'<p class="virtual-selection-own-class">' +
+					'<p>' +
 						'bar' +
 					'</p>' +
 				'</div>'
@@ -171,11 +242,11 @@ describe( 'model-to-view-converters', () => {
 		it( 'should not convert marker on elements already consumed', () => {
 			const newDescriptor = { class: 'override-class' };
 
-			dispatcher.on( 'addMarker:marker', markerToVirtualSelection( virtualSelectionDescriptor ) );
-			dispatcher.on( 'removeMarker:marker', markerToVirtualSelection( virtualSelectionDescriptor ) );
+			dispatcher.on( 'addMarker:marker', convertElementsInsideMarker( virtualSelectionDescriptor ) );
+			dispatcher.on( 'removeMarker:marker', convertElementsInsideMarker( virtualSelectionDescriptor ) );
 
-			dispatcher.on( 'addMarker:marker', markerToVirtualSelection( newDescriptor ), { priority: 'high' } );
-			dispatcher.on( 'removeMarker:marker', markerToVirtualSelection( newDescriptor ), { priority: 'high' } );
+			dispatcher.on( 'addMarker:marker', convertElementsInsideMarker( newDescriptor ), { priority: 'high' } );
+			dispatcher.on( 'removeMarker:marker', convertElementsInsideMarker( newDescriptor ), { priority: 'high' } );
 
 			dispatcher.on( 'insert:paragraph', insertElement( () => {
 				const element = new ViewContainerElement( 'p' );
@@ -206,8 +277,8 @@ describe( 'model-to-view-converters', () => {
 		} );
 
 		it( 'should do nothing if descriptor is not provided', () => {
-			dispatcher.on( 'addMarker:marker', markerToVirtualSelection( () => null ) );
-			dispatcher.on( 'removeMarker:marker', markerToVirtualSelection( () => null ) );
+			dispatcher.on( 'addMarker:marker', convertElementsInsideMarker( () => null ) );
+			dispatcher.on( 'removeMarker:marker', convertElementsInsideMarker( () => null ) );
 
 			dispatcher.convertInsertion( markerRange );
 
@@ -810,6 +881,33 @@ describe( 'model-to-view-converters', () => {
 
 				expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p></div>' );
 			} );
+
+			it( 'should be possible to overwrite', () => {
+				const viewUi = new ViewUIElement( 'span', { 'class': 'marker' } );
+
+				sinon.spy( dispatcher, 'fire' );
+
+				dispatcher.on( 'addMarker:marker', insertUIElement( viewUi ) );
+				dispatcher.on( 'removeMarker:marker', removeUIElement( viewUi ) );
+
+				dispatcher.on( 'addMarker:marker', ( evt, data, consumable ) => {
+					consumable.consume( data.item, 'addMarker:marker' );
+				}, { priority: 'high' } );
+
+				dispatcher.on( 'removeMarker:marker', ( evt, data, consumable ) => {
+					consumable.consume( data.item, 'removeMarker:marker' );
+				}, { priority: 'high' } );
+
+				dispatcher.convertMarker( 'addMarker', 'marker', range );
+
+				expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p></div>' );
+				expect( dispatcher.fire.calledWith( 'addMarker:marker' ) );
+
+				dispatcher.convertMarker( 'removeMarker', 'marker', range );
+
+				expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p></div>' );
+				expect( dispatcher.fire.calledWith( 'removeMarker:marker' ) );
+			} );
 		} );
 	} );