Skip to content

Commit

Permalink
Merge pull request #7506 from ckeditor/i/7444-2SCM-refactor
Browse files Browse the repository at this point in the history 
Other (engine): The engine's util `bindTwoStepCaretToAttribute()` was removed. See #7444.
Feature (typing): Introduced `TwoStepCaretMovement` plugin. See #7444.

MINOR BREAKING CHANGE (engine): The `bindTwoStepCaretToAttribute()` utility function has been removed. Use `editor.plugins.get( TwoStepCaretMovement ).registerAttribute()` instead.
  • Loading branch information
@jodator
jodator authored Jul 1, 2020
2 parents 565569a + 48f5c95 commit d40bd58
Show file tree
Hide file tree
Showing 11 changed files with 157 additions and 155 deletions.
2 changes: 1 addition & 1 deletion packages/ckeditor5-link/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"@ckeditor/ckeditor5-core": "^20.0.0",
"@ckeditor/ckeditor5-engine": "^20.0.0",
"@ckeditor/ckeditor5-image": "^20.0.0",
"@ckeditor/ckeditor5-typing": "^20.0.0",
"@ckeditor/ckeditor5-ui": "^20.0.0",
"@ckeditor/ckeditor5-utils": "^20.0.0",
"lodash-es": "^4.17.15"
Expand All @@ -26,7 +27,6 @@
"@ckeditor/ckeditor5-enter": "^20.0.0",
"@ckeditor/ckeditor5-paragraph": "^20.0.0",
"@ckeditor/ckeditor5-theme-lark": "^20.0.0",
"@ckeditor/ckeditor5-typing": "^20.0.0",
"@ckeditor/ckeditor5-undo": "^20.0.0"
},
"engines": {
Expand Down
19 changes: 10 additions & 9 deletions packages/ckeditor5-link/src/linkediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import MouseObserver from '@ckeditor/ckeditor5-engine/src/view/observer/mouseobserver';
import bindTwoStepCaretToAttribute from '@ckeditor/ckeditor5-engine/src/utils/bindtwostepcarettoattribute';
import TwoStepCaretMovement from '@ckeditor/ckeditor5-typing/src/twostepcaretmovement';
import LinkCommand from './linkcommand';
import UnlinkCommand from './unlinkcommand';
import AutomaticDecorators from './utils/automaticdecorators';
Expand Down Expand Up @@ -40,6 +40,13 @@ export default class LinkEditing extends Plugin {
return 'LinkEditing';
}

/**
* @inheritDoc
*/
static get requires() {
return [ TwoStepCaretMovement ];
}

/**
* @inheritDoc
*/
Expand All @@ -56,7 +63,6 @@ export default class LinkEditing extends Plugin {
*/
init() {
const editor = this.editor;
const locale = editor.locale;

// Allow link attribute on all inline nodes.
editor.model.schema.extend( '$text', { allowAttributes: 'linkHref' } );
Expand Down Expand Up @@ -93,13 +99,8 @@ export default class LinkEditing extends Plugin {
this._enableManualDecorators( linkDecorators.filter( item => item.mode === DECORATOR_MANUAL ) );

// Enable two-step caret movement for `linkHref` attribute.
bindTwoStepCaretToAttribute( {
view: editor.editing.view,
model: editor.model,
emitter: this,
attribute: 'linkHref',
locale
} );
const twoStepCaretMovementPlugin = editor.plugins.get( TwoStepCaretMovement );
twoStepCaretMovementPlugin.registerAttribute( 'linkHref' );

// Setup highlight over selected link.
this._setupLinkHighlight();
Expand Down
27 changes: 18 additions & 9 deletions packages/ckeditor5-link/tests/linkediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,17 @@ describe( 'LinkEditing', () => {
expect( model.schema.checkAttribute( [ '$block' ], 'linkHref' ) ).to.be.false;
} );

// Let's check only the minimum to not duplicate `bindTwoStepCaretToAttribute()` tests.
// Let's check only the minimum to not duplicate `TwoStepCaretMovement` tests.
// Testing minimum is better than testing using spies that might give false positive results.
describe( 'two-step caret movement', () => {
it( 'should be bound to th `linkHref` attribute (LTR)', () => {
it( 'should be bound to the `linkHref` attribute (LTR)', () => {
const selection = editor.model.document.selection;

// Put selection before the link element.
setModelData( editor.model, '<paragraph>foo[]<$text linkHref="url">b</$text>ar</paragraph>' );

// The selection's gravity is not overridden because selection landed here not because of `keydown`.
expect( editor.model.document.selection.isGravityOverridden ).to.false;
// The selection's gravity should read attributes from the left.
expect( selection.hasAttribute( 'linkHref' ), 'hasAttribute( \'linkHref\' )' ).to.be.false;

// So let's simulate the `keydown` event.
editor.editing.view.document.fire( 'keydown', {
Expand All @@ -92,10 +94,13 @@ describe( 'LinkEditing', () => {
domTarget: document.body
} );

expect( editor.model.document.selection.isGravityOverridden ).to.true;
expect( getModelData( model ) ).to.equal( '<paragraph>foo<$text linkHref="url">[]b</$text>ar</paragraph>' );
// Selection should get the attributes from the right.
expect( selection.hasAttribute( 'linkHref' ), 'hasAttribute( \'linkHref\' )' ).to.be.true;
expect( selection.getAttribute( 'linkHref' ), 'linkHref attribute' ).to.equal( 'url' );
} );

it( 'should be bound to th `linkHref` attribute (RTL)', async () => {
it( 'should be bound to the `linkHref` attribute (RTL)', async () => {
const editor = await ClassicTestEditor.create( element, {
plugins: [ Paragraph, LinkEditing, Enter ],
language: {
Expand All @@ -105,12 +110,13 @@ describe( 'LinkEditing', () => {

model = editor.model;
view = editor.editing.view;
const selection = editor.model.document.selection;

// Put selection before the link element.
setModelData( editor.model, '<paragraph>foo[]<$text linkHref="url">b</$text>ar</paragraph>' );

// The selection's gravity is not overridden because selection landed here not because of `keydown`.
expect( editor.model.document.selection.isGravityOverridden ).to.false;
// The selection's gravity should read attributes from the left.
expect( selection.hasAttribute( 'linkHref' ), 'hasAttribute( \'linkHref\' )' ).to.be.false;

// So let's simulate the `keydown` event.
editor.editing.view.document.fire( 'keydown', {
Expand All @@ -119,7 +125,10 @@ describe( 'LinkEditing', () => {
domTarget: document.body
} );

expect( editor.model.document.selection.isGravityOverridden ).to.true;
expect( getModelData( model ) ).to.equal( '<paragraph>foo<$text linkHref="url">[]b</$text>ar</paragraph>' );
// Selection should get the attributes from the right.
expect( selection.hasAttribute( 'linkHref' ), 'hasAttribute( \'linkHref\' )' ).to.be.true;
expect( selection.getAttribute( 'linkHref' ), 'linkHref attribute' ).to.equal( 'url' );

await editor.destroy();
} );
Expand Down
160 changes: 101 additions & 59 deletions .../src/utils/bindtwostepcarettoattribute.js → ...ditor5-typing/src/twostepcaretmovement.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,30 @@
*/

/**
* @module engine/utils/bindtwostepcarettoattribute
* @module typing/twostepcaretmovement
*/

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';

import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
import priorities from '@ckeditor/ckeditor5-utils/src/priorities';

/**
* 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.
* This plugin enables the two-step caret (phantom) movement behavior for
* {@link module:typing/twostepcaretmovement~TwoStepCaretMovement#registerAttribute registered attributes}
* 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
* **Note:** This plugin 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:
*
* When this behavior is enabled for the `a` attribute and the selection is right before it
* When this plugin is enabled and registered for the `a` attribute and the selection is right before it
* (at the attribute boundary), pressing the right arrow key will not move the selection but update its
* attributes accordingly:
*
Expand Down Expand Up @@ -80,70 +83,109 @@ import priorities from '@ckeditor/ckeditor5-utils/src/priorities';
* <kbd>←</kbd>
*
* <$text a="true">ba{}r</$text>b{}az
*
* @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} 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, locale } ) {
const twoStepCaretHandler = new TwoStepCaretHandler( model, emitter, attribute );
const modelSelection = model.document.selection;

// Listen to keyboard events and handle the caret movement according to the 2-step caret logic.
//
// Note: This listener has the "high+1" priority:
// * "high" because of the filler logic implemented in the renderer which also engages on #keydown.
// When the gravity is overridden the attributes of the (model) selection attributes are reset.
// It may end up with the filler kicking in and breaking the selection.
// * "+1" because we would like to avoid collisions with other features (like Widgets), which
// take over the keydown events with the "high" priority. Two-step caret movement takes precedence
// over Widgets in that matter.
//
// Find out more in https://github.com/ckeditor/ckeditor5-engine/issues/1301.
emitter.listenTo( view.document, 'keydown', ( evt, data ) => {
// This implementation works only for collapsed selection.
if ( !modelSelection.isCollapsed ) {
return;
}
export default class TwoStepCaretMovement extends Plugin {
/**
* @inheritDoc
*/
static get pluginName() {
return 'TwoStepCaretMovement';
}

// When user tries to expand the selection or jump over the whole word or to the beginning/end then
// two-steps movement is not necessary.
if ( data.shiftKey || data.altKey || data.ctrlKey ) {
return;
}
/**
* @inheritDoc
*/
constructor( editor ) {
super( editor );

const arrowRightPressed = data.keyCode == keyCodes.arrowright;
const arrowLeftPressed = data.keyCode == keyCodes.arrowleft;
/**
* A map of handlers for each attribute.
*
* @protected
* @property {module:typing/twostepcaretmovement~TwoStepCaretMovement}
*/
this._handlers = new Map();
}

// When neither left or right arrow has been pressed then do noting.
if ( !arrowRightPressed && !arrowLeftPressed ) {
return;
}
/**
* @inheritDoc
*/
init() {
const editor = this.editor;
const model = editor.model;
const view = editor.editing.view;
const locale = editor.locale;

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

if ( ( contentDirection === 'ltr' && arrowRightPressed ) || ( contentDirection === 'rtl' && arrowLeftPressed ) ) {
isMovementHandled = twoStepCaretHandler.handleForwardMovement( position, data );
} else {
isMovementHandled = twoStepCaretHandler.handleBackwardMovement( position, data );
}
// Listen to keyboard events and handle the caret movement according to the 2-step caret logic.
//
// Note: This listener has the "high+1" priority:
// * "high" because of the filler logic implemented in the renderer which also engages on #keydown.
// When the gravity is overridden the attributes of the (model) selection attributes are reset.
// It may end up with the filler kicking in and breaking the selection.
// * "+1" because we would like to avoid collisions with other features (like Widgets), which
// take over the keydown events with the "high" priority. Two-step caret movement takes precedence
// over Widgets in that matter.
//
// Find out more in https://github.com/ckeditor/ckeditor5-engine/issues/1301.
this.listenTo( view.document, 'keydown', ( evt, data ) => {
// This implementation works only for collapsed selection.
if ( !modelSelection.isCollapsed ) {
return;
}

// 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();
}
}, { priority: priorities.get( 'high' ) + 1 } );
// When user tries to expand the selection or jump over the whole word or to the beginning/end then
// two-steps movement is not necessary.
if ( data.shiftKey || data.altKey || data.ctrlKey ) {
return;
}

const arrowRightPressed = data.keyCode == keyCodes.arrowright;
const arrowLeftPressed = data.keyCode == keyCodes.arrowleft;

// When neither left or right arrow has been pressed then do noting.
if ( !arrowRightPressed && !arrowLeftPressed ) {
return;
}

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

if ( ( contentDirection === 'ltr' && arrowRightPressed ) || ( contentDirection === 'rtl' && arrowLeftPressed ) ) {
for ( const [ , handler ] of this._handlers ) {
isMovementHandled = isMovementHandled || handler.handleForwardMovement( position, data );
}
} else {
for ( const [ , handler ] of this._handlers ) {
isMovementHandled = isMovementHandled || handler.handleBackwardMovement( position, data );
}
}

// 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();
}
}, { priority: priorities.get( 'high' ) + 1 } );
}

/**
* Registers a given attribute for the two-step caret movement.
*
* @param {String} attribute Name of the attribute to handle.
*/
registerAttribute( attribute ) {
this._handlers.set(
attribute,
new TwoStepCaretHandler( this.editor.model, this, attribute )
);
}
}

/**
* This is a protected helper–class for {@link module:engine/utils/bindtwostepcarettoattribute}.
* This is a protected helper–class for {@link module:typing/twostepcaretmovement}.
* 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}.
*
Expand Down
0 ...5-engine/tests/manual/tickets/1301/1.html → ...ckeditor5-typing/tests/manual/1301/1.html
View file
File renamed without changes.
14 changes: 4 additions & 10 deletions ...or5-engine/tests/manual/tickets/1301/1.js → ...s/ckeditor5-typing/tests/manual/1301/1.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,17 @@ import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic';

import bindTwoStepCaretToAttribute from '../../../../src/utils/bindtwostepcarettoattribute';
import TwoStepCaretMovement from '../../../src/twostepcaretmovement';

ClassicEditor
.create( document.querySelector( '#editor' ), {
plugins: [ Essentials, Paragraph, Bold, Italic ],
plugins: [ Essentials, Paragraph, Bold, Italic, TwoStepCaretMovement ],
toolbar: [ 'undo', 'redo', '|', 'bold', 'italic' ]
} )
.then( editor => {
const bold = editor.plugins.get( Bold );
const twoStepCaretMovement = editor.plugins.get( TwoStepCaretMovement );

bindTwoStepCaretToAttribute( {
view: editor.editing.view,
model: editor.model,
emitter: bold,
attribute: 'bold',
locale: editor.locale
} );
twoStepCaretMovement.registerAttribute( 'bold' );
} )
.catch( err => {
console.error( err.stack );
Expand Down
2 changes: 1 addition & 1 deletion ...or5-engine/tests/manual/tickets/1301/1.md → ...s/ckeditor5-typing/tests/manual/1301/1.md
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
## Two-steps caret movement [#1301](https://github.com/ckeditor/ckeditor5-engine/issues/1301)
## Two-steps caret movement [ckeditor5-engine#1301](https://github.com/ckeditor/ckeditor5-engine/issues/1301)

1. Put the selection at the end of the content.
2. Press the <kbd>←</kbd> key 3 times.
Expand Down
0 ...5-engine/tests/manual/two-step-caret.html → ...5-typing/tests/manual/two-step-caret.html
View file
File renamed without changes.
Loading

0 comments on commit d40bd58

Please sign in to comment.