Jelajahi Sumber

Changed: Moved some rename conversion logic from ModelConversionDispatcher to converter.

Szymon Cofalik 9 tahun lalu
induk
melakukan
a952797ecd

+ 24 - 6
packages/ckeditor5-engine/src/conversion/model-to-view-converters.js

@@ -4,6 +4,7 @@
  */
 
 import ModelRange from '../model/range.js';
+import ModelPosition from '../model/position.js';
 import ModelElement from '../model/element.js';
 
 import ViewElement from '../view/element.js';
@@ -322,12 +323,13 @@ export function remove() {
  *
  *		modelDispatcher.on( 'rename', rename() );
  *
- * This converter enables default behavior of rename conversion implemented in {@link engine.conversion.ModelConversionDispatcher}.
- * It exposes fake model element `fakeElement` and provides proper model-view bindings between original and fake model elements
- * and their view counterparts.
+ * This converter re-uses converters added for `insert`, `move` and `remove` change types.
  *
  * @external engine.conversion.modelToView
  * @function engine.conversion.modelToView.rename
+ * @fires engine.conversion.ModelConversionDispatcher#event:insert
+ * @fires engine.conversion.ModelConversionDispatcher#event:move
+ * @fires engine.conversion.ModelConversionDispatcher#event:remove
  * @returns {Function}
  */
 export function rename() {
@@ -335,10 +337,9 @@ export function rename() {
 		if ( consumable.test( data.element, 'rename' ) ) {
 			// Create fake element in model, needed for conversion purposes.
 			const fakeElement = new ModelElement( data.oldName, data.element.getAttributes() );
-			// Append the fake element to artificial document fragment to enable making range on it.
+			// Append the fake element to model document to enable making range on it.
 			data.element.parent.insertChildren( data.element.index, fakeElement );
 
-			// Rebind view element to the fake element.
 			// Check what was bound to renamed element.
 			const oldViewElement = conversionApi.mapper.toViewElement( data.element );
 			// Unbind renamed element.
@@ -346,7 +347,24 @@ export function rename() {
 			// Bind view element to the fake element.
 			conversionApi.mapper.bindElements( fakeElement, oldViewElement );
 
-			data.fakeElement = fakeElement;
+			// The range that includes only the renamed element. Will be used to insert an empty element in the view.
+			const insertRange = ModelRange.createFromParentsAndOffsets( data.element.parent, data.element.startOffset, data.element, 0 );
+
+			// Move source position and range of moved nodes. Will be used to move nodes from original view element to renamed one.
+			const moveSourcePosition = ModelPosition.createAt( fakeElement, 0 );
+			const moveRange = ModelRange.createIn( data.element );
+
+			// Remove range containing the fake element. Will be used to remove original view element from the view.
+			const removeRange = ModelRange.createOn( fakeElement );
+
+			// Start the "real" conversion. Use already defined converters by firing insertion, move and remove
+			// conversion on correct ranges / positions.
+			conversionApi.dispatcher.convertInsertion( insertRange );
+			conversionApi.dispatcher.convertMove( moveSourcePosition, moveRange );
+			conversionApi.dispatcher.convertRemove( removeRange.start, removeRange );
+
+			// Cleanup.
+			fakeElement.remove();
 		}
 	};
 }

+ 2 - 43
packages/ckeditor5-engine/src/conversion/modelconversiondispatcher.js

@@ -114,7 +114,7 @@ export default class ModelConversionDispatcher {
 		 *
 		 * @member {Object} engine.conversion.ModelConversionDispatcher#conversionApi
 		 */
-		this.conversionApi = extend( {}, conversionApi );
+		this.conversionApi = extend( { dispatcher: this }, conversionApi );
 	}
 
 	/**
@@ -271,21 +271,9 @@ export default class ModelConversionDispatcher {
 	}
 
 	/**
-	 * Fires events needed to handle rename conversion.
-	 *
-	 * Renaming is handled by:
-	 * * firing insertion conversion - for a new element (renamed clone of old element),
-	 * * firing move conversion - moving children of old element to new element,
-	 * * firing remove conversion - removing old element.
-	 *
-	 * This way two goals are achieved:
-	 * * no need to provide additional converters for renaming,
-	 * * contents of renamed element do not have to be re-rendered.
+	 * Fires rename event with data based on passed values.
 	 *
 	 * @fires engine.conversion.ModelConversionDispatcher#event:rename
-	 * @fires engine.conversion.ModelConversionDispatcher#event:insert
-	 * @fires engine.conversion.ModelConversionDispatcher#event:move
-	 * @fires engine.conversion.ModelConversionDispatcher#event:remove
 	 * @param {engine.view.Element} element Renamed element.
 	 * @param {String} oldName Name of the renamed element before it was renamed.
 	 */
@@ -293,25 +281,8 @@ export default class ModelConversionDispatcher {
 		const consumable = new Consumable();
 		consumable.add( element, 'rename' );
 
-		// Callbacks added to `rename` event should either provide totally custom handling of rename conversion
-		// or create "fake element" in model that is needed to fire default rename conversion.
 		const data = { element, oldName };
 		this.fire( 'rename:' + element.name + ':' + oldName, data, consumable, this.conversionApi );
-
-		// Default callback should provide `data.fakeElement` model element that is needed for firing conversion.
-		if ( data.fakeElement ) {
-			const insertRange = Range.createFromParentsAndOffsets( element.parent, element.startOffset, element, 0 );
-			const moveSourcePosition = Position.createAt( data.fakeElement, 0 );
-			const moveRange = Range.createIn( data.element );
-			const removeRange = Range.createOn( data.fakeElement );
-
-			this.convertInsertion( insertRange );
-			this.convertMove( moveSourcePosition, moveRange );
-			this.convertRemove( removeRange.start, removeRange );
-
-			// Cleanup.
-			data.fakeElement.remove();
-		}
 	}
 
 	/**
@@ -477,18 +448,6 @@ export default class ModelConversionDispatcher {
 	/**
 	 * Fired for renamed element.
 	 *
-	 * **Note:** rename event is a specific event because it requires to have a specific callback. The callback should either
-	 * consume value from `consumable` (and provide all renaming functionality) or add `output` object to `data` object
-	 * with following values:
-	 * * `insertRange` - range in model containing renamed element only (without children),
-	 * * `moveRange` - range in model containing renamed element children only (without renamed element),
-	 * * `moveSourcePosition` - range in model in empty, fake element,
-	 * * `removeRange` - range in model containing the fake element.
-	 *
-	 * For model to view conversion, {@link engine.conversion.modelToView.rename default rename converter} handles
-	 * creating fake element, filling `data` with values and providing correct binding between elements. Use that
-	 * callback as the default callback for renaming.
-	 *
 	 * @event engine.conversion.ModelConversionDispatcher.rename
 	 * @param {Object} data Additional information about the change.
 	 * @param {engine.model.Element} data.element Renamed element.

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

@@ -504,20 +504,6 @@ describe( 'rename', () => {
 		converters.remove.restore();
 	} );
 
-	it( 'should unbind renamed element from view, add fake element to model and bind it with proper view element', () => {
-		const viewElement = viewRoot.getChild( 0 );
-
-		dispatcher.on( 'rename', ( evt, data, consumable, conversionApi ) => {
-			expect( data.fakeElement ).to.be.instanceof( ModelElement );
-			expect( data.fakeElement.parent ).to.equal( data.element.parent );
-
-			expect( conversionApi.mapper.toViewElement( data.element ) ).to.be.undefined;
-			expect( conversionApi.mapper.toViewElement( data.fakeElement ) ).to.equal( viewElement );
-		} );
-
-		dispatcher.convertRename( element, oldName );
-	} );
-
 	it( 'should enable default rename conversion, that uses already registered callbacks', () => {
 		expect( viewRoot.getChild( 0 ).name ).to.equal( 'oldName' );
 		dispatcher.convertRename( element, oldName );

+ 0 - 25
packages/ckeditor5-engine/tests/conversion/modelconversiondispatcher.js

@@ -7,7 +7,6 @@
 
 import ModelConversionDispatcher from '/ckeditor5/engine/conversion/modelconversiondispatcher.js';
 import ModelDocument from '/ckeditor5/engine/model/document.js';
-import ModelDocumentFragment from '/ckeditor5/engine/model/documentfragment.js';
 import ModelText from '/ckeditor5/engine/model/text.js';
 import ModelElement from '/ckeditor5/engine/model/element.js';
 import ModelRange from '/ckeditor5/engine/model/range.js';
@@ -341,30 +340,6 @@ describe( 'ModelConversionDispatcher', () => {
 
 			dispatcher.convertRename( element, oldName );
 		} );
-
-		it( 'should fire insert, move and remove conversion if fake model element was provided in data object', () => {
-			sinon.spy( dispatcher, 'convertInsertion' );
-			sinon.spy( dispatcher, 'convertMove' );
-			sinon.spy( dispatcher, 'convertRemove' );
-
-			dispatcher.on( 'rename', ( evt, data ) => {
-				const fakeElement = new ModelElement( 'oldName' );
-				const fakeParent = new ModelDocumentFragment();
-				fakeParent.appendChildren( fakeElement );
-
-				data.fakeElement = fakeElement;
-			} );
-
-			const element = new ModelElement( 'newName' );
-			const parent = new ModelDocumentFragment();
-			parent.appendChildren( element );
-
-			dispatcher.convertRename( element, 'oldName' );
-
-			expect( dispatcher.convertInsertion.calledOnce ).to.be.true;
-			expect( dispatcher.convertMove.calledOnce ).to.be.true;
-			expect( dispatcher.convertRemove.calledOnce ).to.be.true;
-		} );
 	} );
 
 	describe( 'convertAttribute', () => {