Explorar o código

Put data object instead of selection in the selection event. We may need to add mode data in the future (for instance treeView instance).

Piotr Jasiun %!s(int64=9) %!d(string=hai) anos
pai
achega
f525b24afd

+ 12 - 6
packages/ckeditor5-engine/src/treecontroller/model-selection-to-view-converters.js

@@ -27,7 +27,9 @@ import ViewRange from '../treeview/range.js';
  * @returns {Function} Selection converter.
  */
 export function convertRangeSelection() {
-	return ( evt, selection, consumable, conversionApi ) => {
+	return ( evt, data, consumable, conversionApi ) => {
+		const selection = data.selection;
+
 		if ( selection.isCollapsed ) {
 			return;
 		}
@@ -68,7 +70,9 @@ export function convertRangeSelection() {
  * @returns {Function} Selection converter.
  */
 export function convertCollapsedSelection() {
-	return ( evt, selection, consumable, conversionApi ) => {
+	return ( evt, data, consumable, conversionApi ) => {
+		const selection = data.selection;
+
 		if ( !selection.isCollapsed ) {
 			return;
 		}
@@ -136,11 +140,13 @@ export function convertCollapsedSelection() {
  */
 export function convertSelectionAttribute( elementCreator ) {
 	return ( evt, data, consumable, conversionApi ) => {
-		if ( !data.selection.isCollapsed ) {
+		const selection = data.selection;
+
+		if ( !selection.isCollapsed ) {
 			return;
 		}
 
-		if ( !consumable.consume( data.selection, 'selectionAttribute:' + data.key ) ) {
+		if ( !consumable.consume( selection, 'selectionAttribute:' + data.key ) ) {
 			return;
 		}
 
@@ -150,11 +156,11 @@ export function convertSelectionAttribute( elementCreator ) {
 		for ( let range of ranges ) {
 			const viewElement = elementCreator instanceof ViewElement ?
 				elementCreator.clone( true ) :
-				elementCreator( data.value, data.selection, consumable, conversionApi );
+				elementCreator( data.value, selection, consumable, conversionApi );
 
 			const viewPosition = conversionApi.writer.wrapPosition( range.start, viewElement );
 
-			conversionApi.viewSelection.addRange( new ViewRange( viewPosition, viewPosition ), data.selection.isBackward );
+			conversionApi.viewSelection.addRange( new ViewRange( viewPosition, viewPosition ), selection.isBackward );
 		}
 	};
 }

+ 6 - 6
packages/ckeditor5-engine/src/treecontroller/modelconversiondispatcher.js

@@ -256,15 +256,15 @@ export default class ModelConversionDispatcher {
 	 */
 	convertSelection( selection ) {
 		const consumable = this._createSelectionConsumable( selection );
+		const data = {
+			selection: selection
+		};
 
-		this.fire( 'selection', selection, consumable, this.conversionApi );
+		this.fire( 'selection', data, consumable, this.conversionApi );
 
 		for ( let attr of selection.getAttributes() ) {
-			const data = {
-				selection: selection,
-				key: attr[ 0 ],
-				value: attr[ 1 ]
-			};
+			data.key = attr[ 0 ];
+			data.value = attr[ 1 ];
 
 			// Do not fire event if the attribute has been consumed.
 			if ( consumable.test( selection, 'selectionAttribute:' + data.key ) ) {

+ 3 - 3
packages/ckeditor5-engine/tests/treecontroller/model-converter-builder.js

@@ -197,7 +197,7 @@ describe( 'Model converter builder', () => {
 		it( 'selection conversion', () => {
 			// This test requires collapsed range selection converter (breaking attributes), clearing view selection
 			// and clearing "artifacts" (empty nodes) before new conversion happens.
-			dispatcher.on( 'selection', ( evt, selection, consumable, conversionApi ) => {
+			dispatcher.on( 'selection', ( evt, data, consumable, conversionApi ) => {
 				// Clear artifacts.
 				for ( let range of conversionApi.viewSelection.getRanges() ) {
 					const parentNode = range.start.parent;
@@ -208,11 +208,11 @@ describe( 'Model converter builder', () => {
 				}
 
 				// Break attributes.
-				const viewPosition = conversionApi.mapper.toViewPosition( selection.getFirstPosition() );
+				const viewPosition = conversionApi.mapper.toViewPosition( data.selection.getFirstPosition() );
 				const brokenPosition = conversionApi.writer.breakAttributes( viewPosition );
 
 				// Set the new range as the only range of selection (clear old ranges).
-				conversionApi.viewSelection.setRanges( [ new ViewRange( brokenPosition, brokenPosition ) ], selection.isBackward );
+				conversionApi.viewSelection.setRanges( [ new ViewRange( brokenPosition, brokenPosition ) ], data.selection.isBackward );
 			} );
 
 			// Model converter builder should add selection converter.

+ 7 - 5
packages/ckeditor5-engine/tests/treecontroller/model-selection-to-view-converters.js

@@ -132,8 +132,8 @@ describe( 'default converters', () => {
 		it( 'consumes consumable values properly', () => {
 			// Add callback that will fire before default ones.
 			// This should prevent default callback doing anything.
-			dispatcher.on( 'selection', ( evt, selection, consumable ) => {
-				expect( consumable.consume( selection, 'selection' ) ).to.be.true;
+			dispatcher.on( 'selection', ( evt, data, consumable ) => {
+				expect( consumable.consume( data.selection, 'selection' ) ).to.be.true;
 			}, null, 0 );
 
 			// Similar test case as the first in this suite.
@@ -183,8 +183,8 @@ describe( 'default converters', () => {
 		it( 'consumes consumable values properly', () => {
 			// Add callbacks that will fire before default ones.
 			// This should prevent default callbacks doing anything.
-			dispatcher.on( 'selection', ( evt, selection, consumable ) => {
-				expect( consumable.consume( selection, 'selection' ) ).to.be.true;
+			dispatcher.on( 'selection', ( evt, data, consumable ) => {
+				expect( consumable.consume( data.selection, 'selection' ) ).to.be.true;
 			}, null, 0 );
 
 			dispatcher.on( 'selectionAttribute:bold', ( evt, data, consumable ) => {
@@ -306,7 +306,9 @@ describe( 'table cell selection converter', () => {
 		dispatcher.on( 'insert:td', tableConverter );
 
 		// Special converter for table cells.
-		dispatcher.on( 'selection', ( evt, selection, consumable, conversionApi ) => {
+		dispatcher.on( 'selection', ( evt, data, consumable, conversionApi ) => {
+			const selection = data.selection;
+
 			if ( !consumable.test( selection, 'selection' ) || selection.isCollapsed ) {
 				return;
 			}

+ 10 - 7
packages/ckeditor5-engine/tests/treecontroller/modelconversiondispatcher.js

@@ -370,7 +370,10 @@ describe( 'ModelConversionDispatcher', () => {
 
 			dispatcher.convertSelection( doc.selection );
 
-			expect( dispatcher.fire.calledWith( 'selection', sinon.match.instanceOf( doc.selection.constructor ) ) ).to.be.true;
+			expect( dispatcher.fire.calledWith(
+				'selection',
+				{ selection: sinon.match.instanceOf( doc.selection.constructor ) }
+			) ).to.be.true;
 		} );
 
 		it( 'should prepare correct list of consumable values', () => {
@@ -380,10 +383,10 @@ describe( 'ModelConversionDispatcher', () => {
 					.setAttr( 'italic', true, ModelRange.createFromParentsAndOffsets( root, 4, root, 5 ) );
 			} );
 
-			dispatcher.on( 'selection', ( evt, selection, consumable ) => {
-				expect( consumable.test( selection, 'selection' ) ).to.be.true;
-				expect( consumable.test( selection, 'selectionAttribute:bold' ) ).to.be.true;
-				expect( consumable.test( selection, 'selectionAttribute:italic' ) ).to.be.null;
+			dispatcher.on( 'selection', ( evt, data, consumable ) => {
+				expect( consumable.test( data.selection, 'selection' ) ).to.be.true;
+				expect( consumable.test( data.selection, 'selectionAttribute:bold' ) ).to.be.true;
+				expect( consumable.test( data.selection, 'selectionAttribute:italic' ) ).to.be.null;
 			} );
 
 			dispatcher.convertSelection( doc.selection );
@@ -407,8 +410,8 @@ describe( 'ModelConversionDispatcher', () => {
 		it( 'should not fire attributes events if attribute has been consumed', () => {
 			sinon.spy( dispatcher, 'fire' );
 
-			dispatcher.on( 'selection', ( evt, selection, consumable ) => {
-				consumable.consume( selection, 'selectionAttribute:bold' );
+			dispatcher.on( 'selection', ( evt, data, consumable ) => {
+				consumable.consume( data.selection, 'selectionAttribute:bold' );
 			} );
 
 			doc.enqueueChanges( () => {