Changed the bindTwoStepCaretToAttribute() util arguments syntax. Code…

… refactoring.
ckeditor · Aug 8, 2019 · cc9cf99 · cc9cf99
1 parent 1821fd6
commit cc9cf99
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 25 deletions.
diff --git a/src/utils/bindtwostepcarettoattribute.js b/src/utils/bindtwostepcarettoattribute.js
@@ -81,15 +81,16 @@ import priorities from '@ckeditor/ckeditor5-utils/src/priorities';
  *
  *   		<$text a="true">ba{}r</$text>b{}az
  *
- * @param {module:engine/view/view~View} view View controller instance.
- * @param {module:engine/model/model~Model} model Data model instance.
- * @param {module:utils/dom/emittermixin~Emitter} emitter The emitter to which this behavior should be added
+ * @param {Object} options Helper options.
+ * @param {module:engine/view/view~View} options.view View controller instance.
+ * @param {module:engine/model/model~Model} options.model Data model instance.
+ * @param {module:utils/dom/emittermixin~Emitter} options.emitter The emitter to which this behavior should be added
  * (e.g. a plugin instance).
- * @param {String} attribute Attribute for which this behavior will be added.
- * @param {String} contentDirection Either "ltr" or "rtl" depending on the editor content direction.
+ * @param {String} options.attribute Attribute for which this behavior will be added.
+ * @param {String} options.contentDirection Either "ltr" or "rtl" depending on the editor content direction.
  * Please refer to the {@link module:utils/locale~Locale editor locale} class to learn more.
  */
-export default function bindTwoStepCaretToAttribute( view, model, emitter, attribute, contentDirection ) {
+export default function bindTwoStepCaretToAttribute( { view, model, emitter, attribute, contentDirection } ) {
 	const twoStepCaretHandler = new TwoStepCaretHandler( model, emitter, attribute );
 	const modelSelection = model.document.selection;
 
@@ -127,18 +128,10 @@ export default function bindTwoStepCaretToAttribute( view, model, emitter, attri
 		const position = modelSelection.getFirstPosition();
 		let isMovementHandled;
 
-		if ( contentDirection === 'rtl' ) {
-			if ( arrowRightPressed ) {
-				isMovementHandled = twoStepCaretHandler.handleBackwardMovement( position, data );
-			} else {
-				isMovementHandled = twoStepCaretHandler.handleForwardMovement( position, data );
-			}
+		if ( ( contentDirection === 'ltr' && arrowRightPressed ) || ( contentDirection === 'rtl' && arrowLeftPressed ) ) {
+			isMovementHandled = twoStepCaretHandler.handleForwardMovement( position, data );
 		} else {
-			if ( arrowRightPressed ) {
-				isMovementHandled = twoStepCaretHandler.handleForwardMovement( position, data );
-			} else {
-				isMovementHandled = twoStepCaretHandler.handleBackwardMovement( position, data );
-			}
+			isMovementHandled = twoStepCaretHandler.handleBackwardMovement( position, data );
 		}
 
 		// Stop the keydown event if the two-step caret movement handled it. Avoid collisions

diff --git a/tests/manual/tickets/1301/1.js b/tests/manual/tickets/1301/1.js
@@ -21,7 +21,13 @@ ClassicEditor
 	.then( editor => {
 		const bold = editor.plugins.get( Bold );
 
-		bindTwoStepCaretToAttribute( editor.editing.view, editor.model, bold, 'bold', 'ltr' );
+		bindTwoStepCaretToAttribute( {
+			view: editor.editing.view,
+			model: editor.model,
+			emitter: bold,
+			attribute: 'bold',
+			contentDirection: 'ltr'
+		} );
 	} )
 	.catch( err => {
 		console.error( err.stack );

diff --git a/tests/manual/two-step-caret.js b/tests/manual/two-step-caret.js
@@ -23,8 +23,20 @@ ClassicEditor
 		const bold = editor.plugins.get( Italic );
 		const underline = editor.plugins.get( Underline );
 
-		bindTwoStepCaretToAttribute( editor.editing.view, editor.model, bold, 'italic', 'ltr' );
-		bindTwoStepCaretToAttribute( editor.editing.view, editor.model, underline, 'underline', 'ltr' );
+		bindTwoStepCaretToAttribute( {
+			view: editor.editing.view,
+			model: editor.model,
+			emitter: bold,
+			attribute: 'italic',
+			contentDirection: 'ltr'
+		} );
+		bindTwoStepCaretToAttribute( {
+			view: editor.editing.view,
+			model: editor.model,
+			emitter: underline,
+			attribute: 'underline',
+			contentDirection: 'ltr'
+		} );
 	} )
 	.catch( err => {
 		console.error( err.stack );
@@ -40,8 +52,20 @@ ClassicEditor
 		const bold = editor.plugins.get( Italic );
 		const underline = editor.plugins.get( Underline );
 
-		bindTwoStepCaretToAttribute( editor.editing.view, editor.model, bold, 'italic', 'rtl' );
-		bindTwoStepCaretToAttribute( editor.editing.view, editor.model, underline, 'underline', 'rtl' );
+		bindTwoStepCaretToAttribute( {
+			view: editor.editing.view,
+			model: editor.model,
+			emitter: bold,
+			attribute: 'italic',
+			contentDirection: 'rtl'
+		} );
+		bindTwoStepCaretToAttribute( {
+			view: editor.editing.view,
+			model: editor.model,
+			emitter: underline,
+			attribute: 'underline',
+			contentDirection: 'rtl'
+		} );
 	} )
 	.catch( err => {
 		console.error( err.stack );

diff --git a/tests/utils/bindtwostepcarettoattribute.js b/tests/utils/bindtwostepcarettoattribute.js
@@ -44,7 +44,13 @@ describe( 'bindTwoStepCaretToAttribute()', () => {
 			editor.conversion.for( 'upcast' ).elementToAttribute( { view: 'c', model: 'c' } );
 			editor.conversion.elementToElement( { model: 'paragraph', view: 'p' } );
 
-			bindTwoStepCaretToAttribute( editor.editing.view, editor.model, emitter, 'a', 'ltr' );
+			bindTwoStepCaretToAttribute( {
+				view: editor.editing.view,
+				model: editor.model,
+				emitter,
+				attribute: 'a',
+				contentDirection: 'ltr'
+			} );
 		} );
 	} );
 
@@ -553,7 +559,13 @@ describe( 'bindTwoStepCaretToAttribute()', () => {
 
 	describe( 'multiple attributes', () => {
 		beforeEach( () => {
-			bindTwoStepCaretToAttribute( editor.editing.view, editor.model, emitter, 'c', 'ltr' );
+			bindTwoStepCaretToAttribute( {
+				view: editor.editing.view,
+				model: editor.model,
+				emitter,
+				attribute: 'c',
+				contentDirection: 'ltr'
+			} );
 		} );
 
 		it( 'should work with the two-step caret movement (moving right)', () => {
@@ -794,7 +806,13 @@ describe( 'bindTwoStepCaretToAttribute()', () => {
 					newEditor.conversion.for( 'upcast' ).elementToAttribute( { view: 'c', model: 'c' } );
 					newEditor.conversion.elementToElement( { model: 'paragraph', view: 'p' } );
 
-					bindTwoStepCaretToAttribute( newEditor.editing.view, newEditor.model, emitter, 'a', 'rtl' );
+					bindTwoStepCaretToAttribute( {
+						view: newEditor.editing.view,
+						model: newEditor.model,
+						emitter,
+						attribute: 'a',
+						contentDirection: 'rtl'
+					} );
 
 					return newEditor;
 				} )