8
0
Pārlūkot izejas kodu

Refactored check for refresh trigger event mapping.

Maciej Gołaszewski 5 gadi atpakaļ
vecāks
revīzija
eba356cee3

+ 25 - 11
packages/ckeditor5-engine/src/conversion/downcastdispatcher.js

@@ -295,14 +295,27 @@ export default class DowncastDispatcher {
 		// Create a list of things that can be consumed, consisting of nodes and their attributes.
 		this.conversionApi.consumable = this._createInsertConsumable( range );
 
-		const values = Array.from( range );
-		const topElementValue = values.shift();
+		// The first tree walker value will be for the element marked to be refreshed.
+		// For instance, in the below model structure it will be "<complex>" element:
+		// <complex>
+		//     <slot><paragraph>foo</paragraph></slot>
+		//     <slot><paragraph>bar</paragraph></slot>
+		// </complex>
+		const walkerValues = Array.from( range );
+		const topElementValue = walkerValues.shift();
 
 		this._reconvertElement( rangeIteratorValueToEventData( topElementValue ) );
 
-		for ( const data of values.map( rangeIteratorValueToEventData ) ) {
-			if ( !this._isRefreshTriggerEvent( data ) && !elementWasMemoized( data, this.conversionApi.mapper ) ) {
-				this._convertInsertWithAttributes( data );
+		// 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( rangeIteratorValueToEventData )
+			.filter( eventData => !this._isRefreshTriggerEvent( getEventName( 'insert', eventData ), name ) );
+
+		for ( const eventData of eventsData ) {
+			// convert only non-memoized elements, like "<paragraph>" inside newly inserted "<slot>".
+			if ( !elementWasMemoized( eventData, this.conversionApi.mapper ) ) {
+				this._convertInsertWithAttributes( eventData );
 			}
 		}
 
@@ -607,6 +620,7 @@ export default class DowncastDispatcher {
 			.filter( ( { type } ) => type === 'attribute' || type === 'insert' || type === 'remove' )
 			.map( entry => {
 				const element = getElementFromChange( entry );
+
 				let eventName;
 
 				if ( entry.type === 'attribute' ) {
@@ -620,7 +634,7 @@ export default class DowncastDispatcher {
 					eventName = `${ entry.type }:${ entry.name }`;
 				}
 
-				if ( this._refreshTriggerEventToElementNameMapping.has( eventName ) ) {
+				if ( this._isRefreshTriggerEvent( eventName, element.name ) ) {
 					return element;
 				}
 			} )
@@ -681,13 +695,13 @@ export default class DowncastDispatcher {
 	 * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement `elementToElement()`} conversion helper.
 	 *
 	 * @private
-	 * @param {Object} data Event data.
+	 * @param {String} eventName Event name to check.
+	 * @param {String} elementName Element name to check.
 	 * @returns {Boolean}
 	 */
-	_isRefreshTriggerEvent( data ) {
-		const expectedEventName = getEventName( 'insert', data );
-
-		return this._refreshTriggerEventToElementNameMapping.has( expectedEventName );
+	_isRefreshTriggerEvent( eventName, elementName ) {
+		return this._refreshTriggerEventToElementNameMapping.has( eventName ) &&
+			this._refreshTriggerEventToElementNameMapping.get( eventName ) === elementName;
 	}
 
 	/**

+ 53 - 8
packages/ckeditor5-engine/tests/conversion/downcasthelpers.js

@@ -610,11 +610,57 @@ describe( 'DowncastHelpers', () => {
 					);
 				} );
 
+				it( 'should not trigger refresh  on adding a slot to an element without triggerBy conversion', () => {
+					model.schema.register( 'other', {
+						allowIn: '$root'
+					} );
+					model.schema.extend( 'slot', {
+						allowIn: 'other'
+					} );
+					downcastHelpers.elementToElement( {
+						model: 'other',
+						view: {
+							name: 'div',
+							classes: 'other'
+						}
+					} );
+					downcastHelpers.elementToElement( {
+						model: 'slot',
+						view: {
+							name: 'div',
+							classes: 'slot'
+						}
+					} );
+
+					setModelData( model,
+						'<other>' +
+							'<slot><paragraph>foo</paragraph></slot>' +
+							'<slot><paragraph>bar</paragraph></slot>' +
+						'</other>'
+					);
+					const otherView = viewRoot.getChild( 0 );
+
+					model.change( writer => {
+						insertBazSlot( writer, modelRoot );
+					} );
+
+					expectResult(
+						'<div class="other">' +
+							'<div class="slot"><p>foo</p></div>' +
+							'<div class="slot"><p>bar</p></div>' +
+							'<div class="slot"><p>baz</p></div>' +
+						'</div>'
+					);
+					const otherViewAfter = viewRoot.getChild( 0 );
+
+					expect( otherView, 'the view should not be refreshed' ).to.equal( otherViewAfter );
+				} );
+
 				describe( 'memoization', () => {
 					it( 'should create new element on re-converting element', () => {
 						setModelData( model, '<complex>' +
-							'<slot><paragraph>foo</paragraph></slot>' +
-							'<slot><paragraph>bar</paragraph></slot>' +
+								'<slot><paragraph>foo</paragraph></slot>' +
+								'<slot><paragraph>bar</paragraph></slot>' +
 							'</complex>'
 						);
 
@@ -626,13 +672,13 @@ describe( 'DowncastHelpers', () => {
 
 						const viewAfterReRender = viewRoot.getChild( 0 );
 
-						expect( viewAfterReRender ).to.not.equal( complexView );
+						expect( viewAfterReRender, 'the view should be refreshed' ).to.not.equal( complexView );
 					} );
 
 					it( 'should not re-create slot\'s child elements on re-converting main element (attribute changed)', () => {
 						setModelData( model, '<complex>' +
-							'<slot><paragraph>foo</paragraph></slot>' +
-							'<slot><paragraph>bar</paragraph></slot>' +
+								'<slot><paragraph>foo</paragraph></slot>' +
+								'<slot><paragraph>bar</paragraph></slot>' +
 							'</complex>'
 						);
 
@@ -653,8 +699,8 @@ describe( 'DowncastHelpers', () => {
 
 					it( 'should not re-create slot\'s child elements on re-converting main element (slot added)', () => {
 						setModelData( model, '<complex>' +
-							'<slot><paragraph>foo</paragraph></slot>' +
-							'<slot><paragraph>bar</paragraph></slot>' +
+								'<slot><paragraph>foo</paragraph></slot>' +
+								'<slot><paragraph>bar</paragraph></slot>' +
 							'</complex>'
 						);
 
@@ -781,7 +827,6 @@ describe( 'DowncastHelpers', () => {
 					expectResult( '<div class="complex-slots"><div class="slots"></div></div>' );
 				} );
 
-				// TODO: add memoization check - as this is need.
 				it( 'should convert on attribute set (main element)', () => {
 					setModelData( model, '<complex></complex>' );