From 9f5424f85956a14b34a53e8044f273c56d4d8f0b Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 30 Jul 2019 13:59:24 +0200 Subject: [PATCH 01/10] Brought support for RTL content in the bindTwoStepCaretToAttribute() helper. --- src/utils/bindtwostepcarettoattribute.js | 31 +++++--- tests/utils/bindtwostepcarettoattribute.js | 85 +++++++++++++++++++++- 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/src/utils/bindtwostepcarettoattribute.js b/src/utils/bindtwostepcarettoattribute.js index 601c208ae..29f9bf97c 100644 --- a/src/utils/bindtwostepcarettoattribute.js +++ b/src/utils/bindtwostepcarettoattribute.js @@ -11,12 +11,15 @@ import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; import priorities from '@ckeditor/ckeditor5-utils/src/priorities'; /** - * This helper enabled the two-step caret (phantom) movement behavior for the given {@link module:engine/model/model~Model} + * This helper enables the two-step caret (phantom) movement behavior for the given {@link module:engine/model/model~Model} * attribute on arrow right () and left () key press. * * Thanks to this (phantom) caret movement the user is able to type before/after as well as at the * beginning/end of an attribute. * + * **Note:** This helper support right–to–left (Arabic, Hebrew, etc.) content by mirroring its behavior + * but for the sake of simplicity examples showcase only left–to–right use–cases. + * * # Forward movement * * ## "Entering" an attribute: @@ -83,8 +86,10 @@ import priorities from '@ckeditor/ckeditor5-utils/src/priorities'; * @param {module:utils/dom/emittermixin~Emitter} 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. + * Please refer to the {@link module:utils/locale~Locale editor locale} class to learn more. */ -export default function bindTwoStepCaretToAttribute( view, model, emitter, attribute ) { +export default function bindTwoStepCaretToAttribute( view, model, emitter, attribute, contentDirection ) { const twoStepCaretHandler = new TwoStepCaretHandler( model, emitter, attribute ); const modelSelection = model.document.selection; @@ -122,13 +127,21 @@ export default function bindTwoStepCaretToAttribute( view, model, emitter, attri const position = modelSelection.getFirstPosition(); let isMovementHandled; - if ( arrowRightPressed ) { - isMovementHandled = twoStepCaretHandler.handleForwardMovement( position, data ); + if ( contentDirection === 'rtl' ) { + if ( arrowRightPressed ) { + isMovementHandled = twoStepCaretHandler.handleBackwardMovement( position, data ); + } else { + isMovementHandled = twoStepCaretHandler.handleForwardMovement( position, data ); + } } else { - isMovementHandled = twoStepCaretHandler.handleBackwardMovement( position, data ); + if ( arrowRightPressed ) { + isMovementHandled = twoStepCaretHandler.handleForwardMovement( position, data ); + } else { + isMovementHandled = twoStepCaretHandler.handleBackwardMovement( position, data ); + } } - // Stop the keydown event if the two-step arent movement handled it. Avoid collisions + // Stop the keydown event if the two-step caret movement handled it. Avoid collisions // with other features which may also take over the caret movement (e.g. Widget). if ( isMovementHandled ) { evt.stop(); @@ -137,13 +150,13 @@ export default function bindTwoStepCaretToAttribute( view, model, emitter, attri } /** - * This is a private helper–class for {@link module:engine/utils/bindtwostepcarettoattribute}. + * This is a protected helper–class for {@link module:engine/utils/bindtwostepcarettoattribute}. * It handles the state of the 2-step caret movement for a single {@link module:engine/model/model~Model} * attribute upon the `keypress` in the {@link module:engine/view/view~View}. * - * @private + * @protected */ -class TwoStepCaretHandler { +export class TwoStepCaretHandler { /* * Creates two step handler instance. * diff --git a/tests/utils/bindtwostepcarettoattribute.js b/tests/utils/bindtwostepcarettoattribute.js index f2bdd2576..7912dcac6 100644 --- a/tests/utils/bindtwostepcarettoattribute.js +++ b/tests/utils/bindtwostepcarettoattribute.js @@ -9,8 +9,9 @@ import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtest import DomEmitterMixin from '@ckeditor/ckeditor5-utils/src/dom/emittermixin'; import DomEventData from '../../src/view/observer/domeventdata'; import EventInfo from '@ckeditor/ckeditor5-utils/src/eventinfo'; -import bindTwoStepCaretToAttribute from '../../src/utils/bindtwostepcarettoattribute'; +import bindTwoStepCaretToAttribute, { TwoStepCaretHandler } from '../../src/utils/bindtwostepcarettoattribute'; import Position from '../../src/model/position'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; import { setData } from '../../src/dev-utils/model'; @@ -18,6 +19,8 @@ describe( 'bindTwoStepCaretToAttribute()', () => { let editor, model, emitter, selection, view; let preventDefaultSpy, evtStopSpy; + testUtils.createSinonSandbox(); + beforeEach( () => { emitter = Object.create( DomEmitterMixin ); @@ -41,7 +44,7 @@ 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' ); + bindTwoStepCaretToAttribute( editor.editing.view, editor.model, emitter, 'a', 'ltr' ); } ); } ); @@ -550,7 +553,7 @@ describe( 'bindTwoStepCaretToAttribute()', () => { describe( 'multiple attributes', () => { beforeEach( () => { - bindTwoStepCaretToAttribute( editor.editing.view, editor.model, emitter, 'c' ); + bindTwoStepCaretToAttribute( editor.editing.view, editor.model, emitter, 'c', 'ltr' ); } ); it( 'should work with the two-step caret movement (moving right)', () => { @@ -743,6 +746,82 @@ describe( 'bindTwoStepCaretToAttribute()', () => { expect( getSelectionAttributesArray( selection ) ).to.have.members( [] ); } ); + describe.only( 'left–to–right and right–to–left content', () => { + it( 'should call methods associated with the keys (LTR content direction)', () => { + const forwardStub = testUtils.sinon.stub( TwoStepCaretHandler.prototype, 'handleForwardMovement' ); + const backwardStub = testUtils.sinon.stub( TwoStepCaretHandler.prototype, 'handleBackwardMovement' ); + + setData( model, '<$text>foo[]<$text a="true">bar' ); + + fireKeyDownEvent( { + keyCode: keyCodes.arrowright + } ); + + sinon.assert.calledOnce( forwardStub ); + sinon.assert.notCalled( backwardStub ); + + setData( model, '<$text>foo<$text a="true">[]bar' ); + + fireKeyDownEvent( { + keyCode: keyCodes.arrowleft + } ); + + sinon.assert.calledOnce( backwardStub ); + sinon.assert.calledOnce( forwardStub ); + } ); + + it( 'should use the opposite helper methods (RTL content direction)', () => { + const forwardStub = testUtils.sinon.stub( TwoStepCaretHandler.prototype, 'handleForwardMovement' ); + const backwardStub = testUtils.sinon.stub( TwoStepCaretHandler.prototype, 'handleBackwardMovement' ); + const emitter = Object.create( DomEmitterMixin ); + + let model; + + return VirtualTestEditor.create() + .then( newEditor => { + model = newEditor.model; + selection = model.document.selection; + view = newEditor.editing.view; + + newEditor.model.schema.extend( '$text', { + allowAttributes: [ 'a', 'b', 'c' ], + allowIn: '$root' + } ); + + model.schema.register( 'paragraph', { inheritAllFrom: '$block' } ); + newEditor.conversion.for( 'upcast' ).elementToAttribute( { view: 'a', model: 'a' } ); + newEditor.conversion.for( 'upcast' ).elementToAttribute( { view: 'b', model: 'b' } ); + 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' ); + + return newEditor; + } ) + .then( newEditor => { + setData( model, '<$text>foo[]<$text a="true">bar' ); + + fireKeyDownEvent( { + keyCode: keyCodes.arrowleft + } ); + + sinon.assert.calledOnce( forwardStub ); + sinon.assert.notCalled( backwardStub ); + + setData( model, '<$text>foo<$text a="true">[]bar' ); + + fireKeyDownEvent( { + keyCode: keyCodes.arrowright + } ); + + sinon.assert.calledOnce( backwardStub ); + sinon.assert.calledOnce( forwardStub ); + + return newEditor.destroy(); + } ); + } ); + } ); + const keyMap = { '→': 'arrowright', '←': 'arrowleft' From 7c20fab947e9689f075791d9cfb767c74626d912 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 30 Jul 2019 13:59:55 +0200 Subject: [PATCH 02/10] Tests: Added RTL content editor in the bindTwoStepCaretToAttribute() manual test. --- tests/manual/two-step-caret.html | 22 +++++++++++++++++++++- tests/manual/two-step-caret.js | 23 ++++++++++++++++++++--- tests/manual/two-step-caret.md | 7 +++++++ 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/tests/manual/two-step-caret.html b/tests/manual/two-step-caret.html index 46f8315e9..1af285f20 100644 --- a/tests/manual/two-step-caret.html +++ b/tests/manual/two-step-caret.html @@ -1,5 +1,25 @@ -
+

Left-to–right content

+ +

Foo bar biz

Foo barbiz buz?

Foo bar biz

+ +

Right–to–left content

+ +
+

 היא תכונה של

+

 זהה לזהשיוצג בתוצאה

+

וכדומה. תכונה זו מאפיינת

+
+ + diff --git a/tests/manual/two-step-caret.js b/tests/manual/two-step-caret.js index 5fef68471..a04cc8bfb 100644 --- a/tests/manual/two-step-caret.js +++ b/tests/manual/two-step-caret.js @@ -15,7 +15,7 @@ import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; import bindTwoStepCaretToAttribute from '../../src/utils/bindtwostepcarettoattribute'; ClassicEditor - .create( document.querySelector( '#editor' ), { + .create( document.querySelector( '#editor-ltr' ), { plugins: [ Essentials, Paragraph, Underline, Bold, Italic ], toolbar: [ 'undo', 'redo', '|', 'bold', 'underline', 'italic' ] } ) @@ -23,8 +23,25 @@ ClassicEditor const bold = editor.plugins.get( Italic ); const underline = editor.plugins.get( Underline ); - bindTwoStepCaretToAttribute( editor.editing.view, editor.model, bold, 'italic' ); - bindTwoStepCaretToAttribute( editor.editing.view, editor.model, underline, 'underline' ); + bindTwoStepCaretToAttribute( editor.editing.view, editor.model, bold, 'italic', 'ltr' ); + bindTwoStepCaretToAttribute( editor.editing.view, editor.model, underline, 'underline', 'ltr' ); + } ) + .catch( err => { + console.error( err.stack ); + } ); + +ClassicEditor + .create( document.querySelector( '#editor-rtl' ), { + contentLanguage: 'he', + plugins: [ Essentials, Paragraph, Underline, Bold, Italic ], + toolbar: [ 'undo', 'redo', '|', 'bold', 'underline', 'italic' ] + } ) + .then( editor => { + 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' ); } ) .catch( err => { console.error( err.stack ); diff --git a/tests/manual/two-step-caret.md b/tests/manual/two-step-caret.md index a2c79fcea..ef8fa44c5 100644 --- a/tests/manual/two-step-caret.md +++ b/tests/manual/two-step-caret.md @@ -47,3 +47,10 @@ ### Not bounded attribute Just make sure that two-steps caret movement is disabled for bold text from the third paragraph. + +### Right–to–left content + +**Tip**: Change the system keyboard to Hebrew before testing. + +Two-steps caret movement should also work when the content is right–to–left. Repeat all previous steps keeping in mind that the flow of the text is "reversed". + From 35c5abc84ae5d5fd616b312401d900bfd948849c Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 30 Jul 2019 14:00:10 +0200 Subject: [PATCH 03/10] Tests: Updated a ticket test to the latest bindTwoStepCaretToAttribute() API. --- tests/manual/tickets/1301/1.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/manual/tickets/1301/1.js b/tests/manual/tickets/1301/1.js index 7fb2da070..ff0b5e464 100644 --- a/tests/manual/tickets/1301/1.js +++ b/tests/manual/tickets/1301/1.js @@ -21,7 +21,7 @@ ClassicEditor .then( editor => { const bold = editor.plugins.get( Bold ); - bindTwoStepCaretToAttribute( editor.editing.view, editor.model, bold, 'bold' ); + bindTwoStepCaretToAttribute( editor.editing.view, editor.model, bold, 'bold', 'ltr' ); } ) .catch( err => { console.error( err.stack ); From f60b74541f7a75031030746f850d2626f9cd51f1 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 30 Jul 2019 14:06:42 +0200 Subject: [PATCH 04/10] Tests: Removed obsolete describe.only from bindTwoStepCaretToAttribute() tests. --- tests/utils/bindtwostepcarettoattribute.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils/bindtwostepcarettoattribute.js b/tests/utils/bindtwostepcarettoattribute.js index 7912dcac6..a411396e6 100644 --- a/tests/utils/bindtwostepcarettoattribute.js +++ b/tests/utils/bindtwostepcarettoattribute.js @@ -746,7 +746,7 @@ describe( 'bindTwoStepCaretToAttribute()', () => { expect( getSelectionAttributesArray( selection ) ).to.have.members( [] ); } ); - describe.only( 'left–to–right and right–to–left content', () => { + describe( 'left–to–right and right–to–left content', () => { it( 'should call methods associated with the keys (LTR content direction)', () => { const forwardStub = testUtils.sinon.stub( TwoStepCaretHandler.prototype, 'handleForwardMovement' ); const backwardStub = testUtils.sinon.stub( TwoStepCaretHandler.prototype, 'handleBackwardMovement' ); From cc9cf99357fcaf9ac07d91b7c7b22a42b4a761c3 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 8 Aug 2019 15:20:35 +0200 Subject: [PATCH 05/10] Changed the bindTwoStepCaretToAttribute() util arguments syntax. Code refactoring. --- src/utils/bindtwostepcarettoattribute.js | 27 +++++++----------- tests/manual/tickets/1301/1.js | 8 +++++- tests/manual/two-step-caret.js | 32 +++++++++++++++++++--- tests/utils/bindtwostepcarettoattribute.js | 24 ++++++++++++++-- 4 files changed, 66 insertions(+), 25 deletions(-) diff --git a/src/utils/bindtwostepcarettoattribute.js b/src/utils/bindtwostepcarettoattribute.js index 29f9bf97c..8085fd8d8 100644 --- 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{}rb{}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 index ff0b5e464..1522525f9 100644 --- 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 index a04cc8bfb..39ad16e3b 100644 --- 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 index a411396e6..24a2c53a7 100644 --- 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; } ) From a38343d83bf1a37e406a1f70e46baf08bc3d280c Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 9 Aug 2019 16:52:22 +0200 Subject: [PATCH 06/10] Used the latest EditorConfig#language syntax. --- tests/manual/two-step-caret.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/manual/two-step-caret.js b/tests/manual/two-step-caret.js index 39ad16e3b..9e76e67da 100644 --- a/tests/manual/two-step-caret.js +++ b/tests/manual/two-step-caret.js @@ -44,7 +44,9 @@ ClassicEditor ClassicEditor .create( document.querySelector( '#editor-rtl' ), { - contentLanguage: 'he', + language: { + content: 'he' + }, plugins: [ Essentials, Paragraph, Underline, Bold, Italic ], toolbar: [ 'undo', 'redo', '|', 'bold', 'underline', 'italic' ] } ) From 99b24585bd6ae9a970eb4b6ddd5f50506567015a Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 9 Aug 2019 17:04:05 +0200 Subject: [PATCH 07/10] Passed a Locale instance to the bindTwoStepCaretToAttribute() helper instead of a static content direction info. --- src/utils/bindtwostepcarettoattribute.js | 6 +++--- tests/manual/tickets/1301/1.js | 2 +- tests/manual/two-step-caret.js | 8 ++++---- tests/utils/bindtwostepcarettoattribute.js | 16 +++++++++++----- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/utils/bindtwostepcarettoattribute.js b/src/utils/bindtwostepcarettoattribute.js index 8085fd8d8..036582388 100644 --- a/src/utils/bindtwostepcarettoattribute.js +++ b/src/utils/bindtwostepcarettoattribute.js @@ -87,10 +87,9 @@ import priorities from '@ckeditor/ckeditor5-utils/src/priorities'; * @param {module:utils/dom/emittermixin~Emitter} options.emitter The emitter to which this behavior should be added * (e.g. a plugin instance). * @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. + * @param {module:utils/locale~Locale} locale The {@link module:core/editor/editor~Editor#locale} instance. */ -export default function bindTwoStepCaretToAttribute( { view, model, emitter, attribute, contentDirection } ) { +export default function bindTwoStepCaretToAttribute( { view, model, emitter, attribute, locale } ) { const twoStepCaretHandler = new TwoStepCaretHandler( model, emitter, attribute ); const modelSelection = model.document.selection; @@ -126,6 +125,7 @@ export default function bindTwoStepCaretToAttribute( { view, model, emitter, att } const position = modelSelection.getFirstPosition(); + const contentDirection = locale.contentLanguageDirection; let isMovementHandled; if ( ( contentDirection === 'ltr' && arrowRightPressed ) || ( contentDirection === 'rtl' && arrowLeftPressed ) ) { diff --git a/tests/manual/tickets/1301/1.js b/tests/manual/tickets/1301/1.js index 1522525f9..65434e236 100644 --- a/tests/manual/tickets/1301/1.js +++ b/tests/manual/tickets/1301/1.js @@ -26,7 +26,7 @@ ClassicEditor model: editor.model, emitter: bold, attribute: 'bold', - contentDirection: 'ltr' + locale: editor.locale } ); } ) .catch( err => { diff --git a/tests/manual/two-step-caret.js b/tests/manual/two-step-caret.js index 9e76e67da..0c351b379 100644 --- a/tests/manual/two-step-caret.js +++ b/tests/manual/two-step-caret.js @@ -28,14 +28,14 @@ ClassicEditor model: editor.model, emitter: bold, attribute: 'italic', - contentDirection: 'ltr' + locale: editor.locale } ); bindTwoStepCaretToAttribute( { view: editor.editing.view, model: editor.model, emitter: underline, attribute: 'underline', - contentDirection: 'ltr' + locale: editor.locale } ); } ) .catch( err => { @@ -59,14 +59,14 @@ ClassicEditor model: editor.model, emitter: bold, attribute: 'italic', - contentDirection: 'rtl' + locale: editor.locale } ); bindTwoStepCaretToAttribute( { view: editor.editing.view, model: editor.model, emitter: underline, attribute: 'underline', - contentDirection: 'rtl' + locale: editor.locale } ); } ) .catch( err => { diff --git a/tests/utils/bindtwostepcarettoattribute.js b/tests/utils/bindtwostepcarettoattribute.js index 24a2c53a7..55b7f66b1 100644 --- a/tests/utils/bindtwostepcarettoattribute.js +++ b/tests/utils/bindtwostepcarettoattribute.js @@ -16,7 +16,7 @@ import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; import { setData } from '../../src/dev-utils/model'; describe( 'bindTwoStepCaretToAttribute()', () => { - let editor, model, emitter, selection, view; + let editor, model, emitter, selection, view, locale; let preventDefaultSpy, evtStopSpy; testUtils.createSinonSandbox(); @@ -29,6 +29,7 @@ describe( 'bindTwoStepCaretToAttribute()', () => { model = editor.model; selection = model.document.selection; view = editor.editing.view; + locale = editor.locale; preventDefaultSpy = sinon.spy(); evtStopSpy = sinon.spy(); @@ -49,7 +50,7 @@ describe( 'bindTwoStepCaretToAttribute()', () => { model: editor.model, emitter, attribute: 'a', - contentDirection: 'ltr' + locale } ); } ); } ); @@ -564,7 +565,7 @@ describe( 'bindTwoStepCaretToAttribute()', () => { model: editor.model, emitter, attribute: 'c', - contentDirection: 'ltr' + locale } ); } ); @@ -789,7 +790,12 @@ describe( 'bindTwoStepCaretToAttribute()', () => { let model; - return VirtualTestEditor.create() + return VirtualTestEditor + .create( { + language: { + content: 'ar' + } + } ) .then( newEditor => { model = newEditor.model; selection = model.document.selection; @@ -811,7 +817,7 @@ describe( 'bindTwoStepCaretToAttribute()', () => { model: newEditor.model, emitter, attribute: 'a', - contentDirection: 'rtl' + locale: newEditor.locale } ); return newEditor; From bd8c42eef47ade61e3f9ff6f2b2e9d4f439ecea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 6 Aug 2019 11:28:04 +0200 Subject: [PATCH 08/10] Docs: Added example to clarify the case explained in the ticket. --- src/conversion/upcastdispatcher.js | 21 +++++++++++++++++++++ src/conversion/upcasthelpers.js | 4 ++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/conversion/upcastdispatcher.js b/src/conversion/upcastdispatcher.js index d9ed529f8..8d0aa8447 100644 --- a/src/conversion/upcastdispatcher.js +++ b/src/conversion/upcastdispatcher.js @@ -55,6 +55,27 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix'; * } * } ); * + * // Convert

's font-size style. + * // Note: You should use a low-priority observer in order to ensure that + * // it's executed after the element-to-element converter. + * editor.data.upcastDispatcher.on( 'element', ( evt, data, conversionApi ) => { + * const { consumable, schema, writer } = conversionApi; + * + * if ( !consumable.consume( data.viewItem, { style: 'font-size' } ) ) { + * return; + * } + * + * const fontSize = data.viewItem.getStyle( 'font-size' ); + * + * // Don't go for the model element after data.modelCursor because it might happen + * // that a single view element was converted to multiple model elements. Get all of them. + * for ( const item of data.modelRange.getItems( { shallow: true } ) ) { + * if ( schema.checkAttribute( item, 'fontSize' ) ) { + * writer.setAttribute( 'fontSize', fontSize, item ); + * } + * } + * }, { priority: 'low' } ); + * * // Convert all elements which have no custom converter into paragraph (autoparagraphing). * editor.data.upcastDispatcher.on( 'element', ( evt, data, conversionApi ) => { * // When element is already consumed by higher priority converters then do nothing. diff --git a/src/conversion/upcasthelpers.js b/src/conversion/upcasthelpers.js index 936be9376..a01022325 100644 --- a/src/conversion/upcasthelpers.js +++ b/src/conversion/upcasthelpers.js @@ -156,7 +156,7 @@ export default class UpcastHelpers extends ConversionHelpers { * @param {String|Object} config.model Model attribute key or an object with `key` and `value` properties, describing * the model attribute. `value` property may be set as a function that takes a view element and returns the value. * If `String` is given, the model attribute value will be set to `true`. - * @param {module:utils/priorities~PriorityString} [config.converterPriority='normal'] Converter priority. + * @param {module:utils/priorities~PriorityString} [config.converterPriority='low'] Converter priority. * @returns {module:engine/conversion/upcasthelpers~UpcastHelpers} */ elementToAttribute( config ) { @@ -422,7 +422,7 @@ function upcastElementToElement( config ) { // @param {String|Object} config.model Model attribute key or an object with `key` and `value` properties, describing // the model attribute. `value` property may be set as a function that takes a view element and returns the value. // If `String` is given, the model attribute value will be set to `true`. -// @param {module:utils/priorities~PriorityString} [config.converterPriority='normal'] Converter priority. +// @param {module:utils/priorities~PriorityString} [config.converterPriority='low'] Converter priority. // @returns {Function} Conversion helper. function upcastElementToAttribute( config ) { config = cloneDeep( config ); From 0f54c09106b4d5e31f7ebf6d7c3d5f0f72d810b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 6 Aug 2019 11:53:08 +0200 Subject: [PATCH 09/10] Fixed a typo. --- src/conversion/upcastdispatcher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conversion/upcastdispatcher.js b/src/conversion/upcastdispatcher.js index 8d0aa8447..82c95f390 100644 --- a/src/conversion/upcastdispatcher.js +++ b/src/conversion/upcastdispatcher.js @@ -58,7 +58,7 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix'; * // Convert

's font-size style. * // Note: You should use a low-priority observer in order to ensure that * // it's executed after the element-to-element converter. - * editor.data.upcastDispatcher.on( 'element', ( evt, data, conversionApi ) => { + * editor.data.upcastDispatcher.on( 'element:p', ( evt, data, conversionApi ) => { * const { consumable, schema, writer } = conversionApi; * * if ( !consumable.consume( data.viewItem, { style: 'font-size' } ) ) { From ded4d588dc6538b9496ef35cfb05ace9ad660d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Mon, 12 Aug 2019 11:36:33 +0200 Subject: [PATCH 10/10] Fixed a typo. --- src/utils/bindtwostepcarettoattribute.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/bindtwostepcarettoattribute.js b/src/utils/bindtwostepcarettoattribute.js index 036582388..e921305d8 100644 --- a/src/utils/bindtwostepcarettoattribute.js +++ b/src/utils/bindtwostepcarettoattribute.js @@ -87,7 +87,7 @@ import priorities from '@ckeditor/ckeditor5-utils/src/priorities'; * @param {module:utils/dom/emittermixin~Emitter} options.emitter The emitter to which this behavior should be added * (e.g. a plugin instance). * @param {String} options.attribute Attribute for which this behavior will be added. - * @param {module:utils/locale~Locale} locale The {@link module:core/editor/editor~Editor#locale} instance. + * @param {module:utils/locale~Locale} options.locale The {@link module:core/editor/editor~Editor#locale} instance. */ export default function bindTwoStepCaretToAttribute( { view, model, emitter, attribute, locale } ) { const twoStepCaretHandler = new TwoStepCaretHandler( model, emitter, attribute );