Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #1769 from ckeditor/t/ckeditor5/1151
Browse files Browse the repository at this point in the history
Feature: Brought support for RTL content in the `bindTwoStepCaretToAttribute()` helper. See ckeditor/ckeditor5#1151.

BREAKING CHANGE: The `bindTwoStepCaretToAttribute()` helper arguments syntax has changed (replaced by an object). Please refer to the helper documentation to learn more.
  • Loading branch information
Reinmar authored Aug 12, 2019
2 parents 941c000 + ded4d58 commit d57ff5a
Show file tree
Hide file tree
Showing 6 changed files with 205 additions and 20 deletions.
28 changes: 17 additions & 11 deletions src/utils/bindtwostepcarettoattribute.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (<kbd>→</kbd>) and left (<kbd>←</kbd>) 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:
Expand Down Expand Up @@ -78,13 +81,15 @@ 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} options.attribute Attribute for which this behavior will be added.
* @param {module:utils/locale~Locale} options.locale The {@link module:core/editor/editor~Editor#locale} instance.
*/
export default function bindTwoStepCaretToAttribute( view, model, emitter, attribute ) {
export default function bindTwoStepCaretToAttribute( { view, model, emitter, attribute, locale } ) {
const twoStepCaretHandler = new TwoStepCaretHandler( model, emitter, attribute );
const modelSelection = model.document.selection;

Expand Down Expand Up @@ -120,15 +125,16 @@ export default function bindTwoStepCaretToAttribute( view, model, emitter, attri
}

const position = modelSelection.getFirstPosition();
const contentDirection = locale.contentLanguageDirection;
let isMovementHandled;

if ( arrowRightPressed ) {
if ( ( contentDirection === 'ltr' && arrowRightPressed ) || ( contentDirection === 'rtl' && arrowLeftPressed ) ) {
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();
Expand All @@ -137,13 +143,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.
*
Expand Down
8 changes: 7 additions & 1 deletion tests/manual/tickets/1301/1.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,13 @@ ClassicEditor
.then( editor => {
const bold = editor.plugins.get( Bold );

bindTwoStepCaretToAttribute( editor.editing.view, editor.model, bold, 'bold' );
bindTwoStepCaretToAttribute( {
view: editor.editing.view,
model: editor.model,
emitter: bold,
attribute: 'bold',
locale: editor.locale
} );
} )
.catch( err => {
console.error( err.stack );
Expand Down
22 changes: 21 additions & 1 deletion tests/manual/two-step-caret.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,25 @@
<div id="editor">
<h2>Left-to–right content</h2>

<div id="editor-ltr">
<p>Foo <u>bar</u> biz</p>
<p>Foo <u>bar</u><i>biz</i> buz?</p>
<p>Foo <b>bar</b> biz</p>
</div>

<h2>Right–to–left content</h2>

<div id="editor-rtl">
<p>&nbsp;היא <u>תכונה</u> של</p>
<p>&nbsp;זהה <i>לזה</i><u>שיוצג</u> בתוצאה</p>
<p>וכדומה. <strong>תכונה</strong> זו מאפיינת</p>
</div>

<style>
u {
background: rgba(255,0,0,.3);
}

i {
background: rgba(0,255,0,.3);
}
</style>
49 changes: 46 additions & 3 deletions tests/manual/two-step-caret.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,59 @@ 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' ]
} )
.then( editor => {
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( {
view: editor.editing.view,
model: editor.model,
emitter: bold,
attribute: 'italic',
locale: editor.locale
} );
bindTwoStepCaretToAttribute( {
view: editor.editing.view,
model: editor.model,
emitter: underline,
attribute: 'underline',
locale: editor.locale
} );
} )
.catch( err => {
console.error( err.stack );
} );

ClassicEditor
.create( document.querySelector( '#editor-rtl' ), {
language: {
content: '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( {
view: editor.editing.view,
model: editor.model,
emitter: bold,
attribute: 'italic',
locale: editor.locale
} );
bindTwoStepCaretToAttribute( {
view: editor.editing.view,
model: editor.model,
emitter: underline,
attribute: 'underline',
locale: editor.locale
} );
} )
.catch( err => {
console.error( err.stack );
Expand Down
7 changes: 7 additions & 0 deletions tests/manual/two-step-caret.md
Original file line number Diff line number Diff line change
Expand Up @@ -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".

111 changes: 107 additions & 4 deletions tests/utils/bindtwostepcarettoattribute.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,18 @@ 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';

describe( 'bindTwoStepCaretToAttribute()', () => {
let editor, model, emitter, selection, view;
let editor, model, emitter, selection, view, locale;
let preventDefaultSpy, evtStopSpy;

testUtils.createSinonSandbox();

beforeEach( () => {
emitter = Object.create( DomEmitterMixin );

Expand All @@ -26,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();
Expand All @@ -41,7 +45,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' );
bindTwoStepCaretToAttribute( {
view: editor.editing.view,
model: editor.model,
emitter,
attribute: 'a',
locale
} );
} );
} );

Expand Down Expand Up @@ -550,7 +560,13 @@ describe( 'bindTwoStepCaretToAttribute()', () => {

describe( 'multiple attributes', () => {
beforeEach( () => {
bindTwoStepCaretToAttribute( editor.editing.view, editor.model, emitter, 'c' );
bindTwoStepCaretToAttribute( {
view: editor.editing.view,
model: editor.model,
emitter,
attribute: 'c',
locale
} );
} );

it( 'should work with the two-step caret movement (moving right)', () => {
Expand Down Expand Up @@ -743,6 +759,93 @@ describe( 'bindTwoStepCaretToAttribute()', () => {
expect( getSelectionAttributesArray( selection ) ).to.have.members( [] );
} );

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' );

setData( model, '<$text>foo[]</$text><$text a="true">bar</$text>' );

fireKeyDownEvent( {
keyCode: keyCodes.arrowright
} );

sinon.assert.calledOnce( forwardStub );
sinon.assert.notCalled( backwardStub );

setData( model, '<$text>foo</$text><$text a="true">[]bar</$text>' );

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( {
language: {
content: 'ar'
}
} )
.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( {
view: newEditor.editing.view,
model: newEditor.model,
emitter,
attribute: 'a',
locale: newEditor.locale
} );

return newEditor;
} )
.then( newEditor => {
setData( model, '<$text>foo[]</$text><$text a="true">bar</$text>' );

fireKeyDownEvent( {
keyCode: keyCodes.arrowleft
} );

sinon.assert.calledOnce( forwardStub );
sinon.assert.notCalled( backwardStub );

setData( model, '<$text>foo</$text><$text a="true">[]bar</$text>' );

fireKeyDownEvent( {
keyCode: keyCodes.arrowright
} );

sinon.assert.calledOnce( backwardStub );
sinon.assert.calledOnce( forwardStub );

return newEditor.destroy();
} );
} );
} );

const keyMap = {
'→': 'arrowright',
'←': 'arrowleft'
Expand Down

0 comments on commit d57ff5a

Please sign in to comment.