瀏覽代碼

Change triggerBy configuration API.

Maciej Gołaszewski 5 年之前
父節點
當前提交
4abbf4af06

+ 30 - 68
packages/ckeditor5-engine/src/conversion/downcastdispatcher.js

@@ -187,12 +187,10 @@ export default class DowncastDispatcher {
 	 *
 	 * @protected
 	 * @param {String} modelName Main model element name for which events will trigger reconversion.
-	 * @param {Array.<String>} events Array of inner events that would trigger conversion for this model.
+	 * @param {String} eventName Name of an event that would trigger conversion for given model element.
 	 */
-	mapRefreshTriggerEvent( modelName, events ) {
-		for ( const eventName of events ) {
-			this._refreshTriggerEventToElementNameMapping.set( eventName, modelName );
-		}
+	mapRefreshTriggerEvent( modelName, eventName ) {
+		this._refreshTriggerEventToElementNameMapping.set( eventName, modelName );
 	}
 
 	/**
@@ -305,18 +303,30 @@ export default class DowncastDispatcher {
 		const walkerValues = Array.from( range );
 		const topElementValue = walkerValues.shift();
 
-		this._reconvertElement( walkerValueToEventData( topElementValue ) );
+		const currentView = this.conversionApi.mapper.toViewElement( walkerValueToEventData( topElementValue ).item );
+
+		// Remove the old view but do not remove mapper mappings - those will be used to revive existing elements.
+		this.conversionApi.writer.remove( currentView );
+
+		this._convertInsertWithAttributes( walkerValueToEventData( topElementValue ) );
+
+		// Bring back removed child views on refreshing the parent view or convert insert for new elements.
+		const convertedViewElement = this.conversionApi.mapper.toViewElement( walkerValueToEventData( topElementValue ).item );
 
-		// All other values are top element's children - we need to check only those that are not handled by a "triggerBy".
-		// For instance if a "<slot>" insertion triggers reconversion, their events should be filtered out while <slot>'s children,
-		// like "<paragraph>", should be converted if they were newly inserted.
-		const eventsData = walkerValues.map( walkerValueToEventData )
-			.filter( eventData => !this._isRefreshTriggerEvent( getEventName( 'insert', eventData ), name ) );
+		for ( const value of Range._createIn( walkerValueToEventData( topElementValue ).item ) ) {
+			const { item } = value;
 
-		for ( const eventData of eventsData ) {
-			// convert only non-memoized elements, like "<paragraph>" inside newly inserted "<slot>".
-			if ( !elementHasViewMapping( eventData, this.conversionApi.mapper ) ) {
-				this._convertInsertWithAttributes( eventData );
+			const view = elementOrTextProxyToView( item, this.conversionApi.mapper );
+
+			if ( view ) {
+				if ( view.root !== convertedViewElement.root ) {
+					this.conversionApi.writer.move(
+						this.conversionApi.writer.createRangeOn( view ),
+						this.conversionApi.mapper.toViewPosition( Position._createAt( item.parent, item.startOffset ) )
+					);
+				}
+			} else {
+				this._convertInsertWithAttributes( walkerValueToEventData( value ) );
 			}
 		}
 
@@ -631,52 +641,6 @@ export default class DowncastDispatcher {
 	}
 
 	/**
-	 * Handles reconverting a model element that has an existing model-to-view mapping.
-	 *
-	 * It performs a shallow conversion for the element and its attributes. All children that already have a converted view
-	 * will not be converted again. Their existing view elements will be used instead.
-	 *
-	 * @private
-	 * @param {Object} data Event data.
-	 */
-	_reconvertElement( data ) {
-		const currentView = this.conversionApi.mapper.toViewElement( data.item );
-
-		const currentModelViewMapping = new Map();
-		const currentViewModelMapping = new Map();
-
-		for ( const { item } of Range._createIn( data.item ) ) {
-			const currentView = this.conversionApi.mapper.toViewElement( item );
-
-			if ( currentView ) {
-				currentModelViewMapping.set( item, currentView );
-				currentViewModelMapping.set( currentView, item );
-			}
-		}
-
-		// Remove the old view but do not remove mapper mappings - those will be used to revive existing elements.
-		this.conversionApi.writer.remove( currentView );
-
-		this._convertInsertWithAttributes( data );
-
-		// Bring back removed child views on refreshing the parent view.
-		const convertedViewElement = this.conversionApi.mapper.toViewElement( data.item );
-
-		for ( const { item } of Range._createIn( data.item ) ) {
-			const view = this.conversionApi.mapper.toViewElement( item );
-
-			if ( view ) {
-				if ( view.root !== convertedViewElement.root ) {
-					this.conversionApi.writer.move(
-						this.conversionApi.writer.createRangeOn( view ),
-						this.conversionApi.mapper.toViewPosition( Position._createAt( item.parent, item.startOffset ) )
-					);
-				}
-			}
-		}
-	}
-
-	/**
 	 * Checks if resulting change should trigger element reconversion.
 	 *
 	 * Those are defined by a `triggerBy` configuration for
@@ -861,16 +825,14 @@ function walkerValueToEventData( value ) {
 	};
 }
 
-function elementHasViewMapping( data, mapper ) {
-	if ( data.item.is( 'textProxy' ) ) {
-		const mappedPosition = mapper.toViewPosition( data.range.start );
+function elementOrTextProxyToView( item, mapper ) {
+	if ( item.is( 'textProxy' ) ) {
+		const mappedPosition = mapper.toViewPosition( Position._createAt( item.parent, item.startOffset ) );
 
-		return !!mappedPosition.parent.is( '$text' );
+		return mappedPosition.parent.is( '$text' ) ? mappedPosition.parent : null;
 	}
 
-	const viewElement = mapper.toViewElement( data.item );
-
-	return !!viewElement;
+	return mapper.toViewElement( item );
 }
 
 /**

+ 23 - 11
packages/ckeditor5-engine/src/conversion/downcasthelpers.js

@@ -66,12 +66,10 @@ export default class DowncastHelpers extends ConversionHelpers {
 	 *		editor.conversion.for( 'downcast' ).elementToElement( {
 	 *			model: 'complex',
 	 *			view: ( modelElement, conversionApi ) => createComplexViewFromModel( modelElement, conversionApi ),
-	 *			triggerBy: [
-	 *				'attribute:foo:complex',
-	 *				'attribute:bar:complex',
-	 *				'insert:slot',
-	 *				'remove:slot'
-	 *			]
+	 *			triggerBy: {
+	 *				attributes: [ 'foo', 'bar' ],
+	 *				children: [ 'slot' ]
+	 *			}
 	 *		} );
 	 *
 	 * See {@link module:engine/conversion/conversion~Conversion#for `conversion.for()`} to learn how to add a converter
@@ -86,8 +84,9 @@ export default class DowncastHelpers extends ConversionHelpers {
 	 * @param {module:engine/view/elementdefinition~ElementDefinition|Function} config.view A view element definition or a function
 	 * that takes the model element and {@link module:engine/conversion/downcastdispatcher~DowncastConversionApi downcast conversion API}
 	 * as parameters and returns a view container element.
-	 * @param {Array.<String>} [config.triggerBy] Events which will trigger element reconversion. Reconversion can be triggered by
-	 * attribute change (eg. `'attribute:foo:complex'` for the main element) or by adding or removing children (eg. `'insert:child'`).
+	 * @param {Object} [config.triggerBy] Re-conversion triggers. At least one trigger must be defined.
+	 * @param {Array.<String>} config.triggerBy.attributes Name of element's attributes which change will trigger element reconversion.
+	 * @param {Array.<String>} config.triggerBy.children Name of direct children that adding or removing will trigger element reconversion.
 	 * @returns {module:engine/conversion/downcasthelpers~DowncastHelpers}
 	 */
 	elementToElement( config ) {
@@ -1356,7 +1355,9 @@ function removeHighlight( highlightDescriptor ) {
 // @param {Object} config Conversion configuration.
 // @param {String} config.model
 // @param {module:engine/view/elementdefinition~ElementDefinition|Function} config.view
-// @param {Array.<String>} [config.triggerBy]
+// @param {Object} [config.triggerBy]
+// @param {Array.<String>} [config.triggerBy.attributes]
+// @param {Array.<String>} [config.triggerBy.children]
 // @returns {Function} Conversion helper.
 function downcastElementToElement( config ) {
 	config = cloneDeep( config );
@@ -1366,8 +1367,19 @@ function downcastElementToElement( config ) {
 	return dispatcher => {
 		dispatcher.on( 'insert:' + config.model, insertElement( config.view ), { priority: config.converterPriority || 'normal' } );
 
-		if ( Array.isArray( config.triggerBy ) ) {
-			dispatcher.mapRefreshTriggerEvent( config.model, config.triggerBy );
+		if ( config.triggerBy ) {
+			if ( Array.isArray( config.triggerBy.attributes ) ) {
+				for ( const attributeKey of config.triggerBy.attributes ) {
+					dispatcher.mapRefreshTriggerEvent( config.model, `attribute:${ attributeKey }:${ config.model }` );
+				}
+			}
+
+			if ( Array.isArray( config.triggerBy.children ) ) {
+				for ( const childName of config.triggerBy.children ) {
+					dispatcher.mapRefreshTriggerEvent( config.model, `insert:${ childName }` );
+					dispatcher.mapRefreshTriggerEvent( config.model, `remove:${ childName }` );
+				}
+			}
 		}
 	};
 }

+ 147 - 29
packages/ckeditor5-engine/tests/conversion/downcasthelpers.js

@@ -126,10 +126,9 @@ describe( 'DowncastHelpers', () => {
 						view: ( modelElement, { writer } ) => {
 							return writer.createContainerElement( 'div', getViewAttributes( modelElement ) );
 						},
-						triggerBy: [
-							'attribute:toStyle:simpleBlock',
-							'attribute:toClass:simpleBlock'
-						]
+						triggerBy: {
+							attributes: [ 'toStyle', 'toClass' ]
+						}
 					} );
 				} );
 
@@ -221,10 +220,9 @@ describe( 'DowncastHelpers', () => {
 						view: ( modelElement, { writer } ) => {
 							return writer.createContainerElement( 'div', getViewAttributes( modelElement ) );
 						},
-						triggerBy: [
-							'attribute:toStyle:simpleBlock',
-							'attribute:toClass:simpleBlock'
-						]
+						triggerBy: {
+							attributes: [ 'toStyle', 'toClass' ]
+						}
 					} );
 
 					model.schema.register( 'paragraph', {
@@ -324,6 +322,132 @@ describe( 'DowncastHelpers', () => {
 				} );
 			} );
 
+			describe( 'with simple block view structure (with children - reconvert on child add)', () => {
+				beforeEach( () => {
+					model.schema.register( 'simpleBlock', {
+						allowIn: '$root'
+					} );
+					downcastHelpers.elementToElement( {
+						model: 'simpleBlock',
+						view: 'div',
+						triggerBy: {
+							children: [ 'paragraph' ]
+						}
+					} );
+
+					model.schema.register( 'paragraph', {
+						inheritAllFrom: '$block',
+						allowIn: 'simpleBlock'
+					} );
+					downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
+				} );
+
+				it( 'should convert on insert', () => {
+					model.change( writer => {
+						const simpleBlock = writer.createElement( 'simpleBlock' );
+						const paragraph = writer.createElement( 'paragraph' );
+
+						writer.insert( simpleBlock, modelRoot, 0 );
+						writer.insert( paragraph, simpleBlock, 0 );
+						writer.insertText( 'foo', paragraph, 0 );
+					} );
+
+					expectResult( '<div><p>foo</p></div>' );
+				} );
+
+				it( 'should convert on adding a child (at the beginning)', () => {
+					setModelData( model, '<simpleBlock toStyle="display:block"><paragraph>foo</paragraph></simpleBlock>' );
+
+					const [ viewBefore, paraBefore, textBefore ] = getNodes();
+
+					model.change( writer => {
+						const paragraph = writer.createElement( 'paragraph' );
+						const text = writer.createText( 'bar' );
+
+						writer.insert( paragraph, modelRoot.getChild( 0 ), 0 );
+						writer.insert( text, paragraph, 0 );
+					} );
+
+					const [ viewAfter, /* insertedPara */, /* insertedText */, paraAfter, textAfter ] = getNodes();
+
+					expectResult( '<div><p>bar</p><p>foo</p></div>' );
+
+					expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore );
+					expect( paraAfter, 'para' ).to.equal( paraBefore );
+					expect( textAfter, 'text' ).to.equal( textBefore );
+				} );
+
+				it( 'should convert on adding a child (in the middle)', () => {
+					setModelData( model,
+						'<simpleBlock toStyle="display:block">' +
+							'<paragraph>foo</paragraph><paragraph>bar</paragraph>' +
+						'</simpleBlock>' );
+
+					const [ viewBefore, paraFooBefore, textFooBefore, paraBarBefore, textBarBefore ] = getNodes();
+
+					model.change( writer => {
+						const paragraph = writer.createElement( 'paragraph' );
+						const text = writer.createText( 'baz' );
+
+						writer.insert( paragraph, modelRoot.getChild( 0 ), 1 );
+						writer.insert( text, paragraph, 0 );
+					} );
+
+					const [ viewAfter,
+						paraFooAfter, textFooAfter, /* insertedPara */, /* insertedText */, paraBarAfter, textBarAfter
+					] = getNodes();
+
+					expectResult( '<div><p>foo</p><p>baz</p><p>bar</p></div>' );
+
+					expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore );
+					expect( paraFooAfter, 'para foo' ).to.equal( paraFooBefore );
+					expect( textFooAfter, 'text foo' ).to.equal( textFooBefore );
+					expect( paraBarAfter, 'para bar' ).to.equal( paraBarBefore );
+					expect( textBarAfter, 'text bar' ).to.equal( textBarBefore );
+				} );
+
+				it( 'should convert on adding a child (at the end)', () => {
+					setModelData( model, '<simpleBlock toStyle="display:block"><paragraph>foo</paragraph></simpleBlock>' );
+
+					const [ viewBefore, paraBefore, textBefore ] = getNodes();
+
+					model.change( writer => {
+						const paragraph = writer.createElement( 'paragraph' );
+						const text = writer.createText( 'bar' );
+
+						writer.insert( paragraph, modelRoot.getChild( 0 ), 1 );
+						writer.insert( text, paragraph, 0 );
+					} );
+
+					const [ viewAfter, paraAfter, textAfter ] = getNodes();
+
+					expectResult( '<div><p>foo</p><p>bar</p></div>' );
+
+					expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore );
+					expect( paraAfter, 'para' ).to.equal( paraBefore );
+					expect( textAfter, 'text' ).to.equal( textBefore );
+				} );
+
+				it( 'should convert on removing a child', () => {
+					setModelData( model,
+						'<simpleBlock><paragraph>foo</paragraph><paragraph>bar</paragraph></simpleBlock>' );
+
+					const [ viewBefore, paraBefore, textBefore ] = getNodes();
+
+					model.change( writer => {
+						writer.remove( modelRoot.getNodeByPath( [ 0, 1 ] ) );
+					} );
+
+					const [ viewAfter, paraAfter, textAfter ] = getNodes();
+
+					expectResult( '<div><p>foo</p></div>' );
+
+					expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore );
+					expect( paraAfter, 'para' ).to.equal( paraBefore );
+					expect( textAfter, 'text' ).to.equal( textBefore );
+				} );
+			} );
+
 			describe( 'with complex view structure - no children allowed', () => {
 				beforeEach( () => {
 					model.schema.register( 'complex', {
@@ -340,10 +464,9 @@ describe( 'DowncastHelpers', () => {
 
 							return outer;
 						},
-						triggerBy: [
-							'attribute:toStyle:complex',
-							'attribute:toClass:complex'
-						]
+						triggerBy: {
+							attributes: [ 'toStyle', 'toClass' ]
+						}
 					} );
 				} );
 
@@ -438,10 +561,9 @@ describe( 'DowncastHelpers', () => {
 
 							return outer;
 						},
-						triggerBy: [
-							'attribute:toStyle:complex',
-							'attribute:toClass:complex'
-						]
+						triggerBy: {
+							attributes: [ 'toStyle', 'toClass' ]
+						}
 					} );
 
 					model.schema.register( 'paragraph', {
@@ -572,13 +694,10 @@ describe( 'DowncastHelpers', () => {
 
 							return outer;
 						},
-						triggerBy: [
-							'attribute:classForMain:complex',
-							'attribute:classForWrap:complex',
-							'attribute:attributeToElement:complex',
-							'insert:slot',
-							'remove:slot'
-						]
+						triggerBy: {
+							attributes: [ 'classForMain', 'classForWrap', 'attributeToElement' ],
+							children: [ 'slot' ]
+						}
 					} );
 
 					model.schema.register( 'slot', {
@@ -935,12 +1054,11 @@ describe( 'DowncastHelpers', () => {
 
 							return outer;
 						},
-						triggerBy: [
-							'attribute:classForMain:complex',
-							'attribute:classForWrap:complex',
-							'attribute:attributeToElement:complex'
-							// Contrary to the previous test - do not act on slot insert/remove: 'insert:slot', 'remove:slot'.
-						]
+						triggerBy: {
+							attributes: [ 'classForMain', 'classForWrap', 'attributeToElement' ]
+							// Contrary to the previous test - do not act on child changes.
+							// children: [ 'slot' ]
+						}
 					} );
 					downcastHelpers.elementToElement( {
 						model: 'slot',