Skip to content

Commit

Permalink
Merge pull request #7440 from ckeditor/i/1016
Browse files Browse the repository at this point in the history
Other (link): The selection after inserting a link will land after the inserted element. Thanks to that a user will be able to type directly after the link without extending the link element. Closes #1016.

Other (link): After clicking at the beginning or end of the link element, the selection will land before/after the clicked element. Thanks to that a user will be able to typing before or after the link element as normal text without extending the link. See #1016.
  • Loading branch information
jodator authored Jun 17, 2020
2 parents 9f0efa8 + 20fab62 commit 0bf66e4
Show file tree
Hide file tree
Showing 7 changed files with 315 additions and 29 deletions.
14 changes: 10 additions & 4 deletions packages/ckeditor5-link/src/linkcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ export default class LinkCommand extends Command {
writer.removeAttribute( item, linkRange );
} );

// Create new range wrapping changed link.
writer.setSelection( linkRange );
// Put the selection at the end of the updated link.
writer.setSelection( writer.createPositionAfter( linkRange.end.nodeBefore ) );
}
// If not then insert text node with `linkHref` attribute in place of caret.
// However, since selection in collapsed, attribute value will be used as data for text node.
Expand All @@ -191,9 +191,15 @@ export default class LinkCommand extends Command {

model.insertContent( node, position );

// Create new range wrapping created node.
writer.setSelection( writer.createRangeOn( node ) );
// Put the selection at the end of the inserted link.
writer.setSelection( writer.createPositionAfter( node ) );
}

// Remove the `linkHref` attribute and all link decorators from the selection.
// It stops adding a new content into the link element.
[ 'linkHref', ...truthyManualDecorators, ...falsyManualDecorators ].forEach( item => {
writer.removeSelectionAttribute( item );
} );
} else {
// If selection has non-collapsed ranges, we change attribute on nodes inside those ranges
// omitting nodes where the `linkHref` attribute is disallowed.
Expand Down
70 changes: 68 additions & 2 deletions packages/ckeditor5-link/src/linkediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
*/

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 LinkCommand from './linkcommand';
import UnlinkCommand from './unlinkcommand';
import { createLinkElement, ensureSafeUrl, getLocalizedDecorators, normalizeDecorators } from './utils';
import AutomaticDecorators from './utils/automaticdecorators';
import ManualDecorator from './utils/manualdecorator';
import bindTwoStepCaretToAttribute from '@ckeditor/ckeditor5-engine/src/utils/bindtwostepcarettoattribute';
import findLinkRange from './findlinkrange';
import { createLinkElement, ensureSafeUrl, getLocalizedDecorators, normalizeDecorators } from './utils';

import '../theme/link.css';

const HIGHLIGHT_CLASS = 'ck-link_selected';
Expand Down Expand Up @@ -104,6 +106,9 @@ export default class LinkEditing extends Plugin {

// Change the attributes of the selection in certain situations after the link was inserted into the document.
this._enableInsertContentSelectionAttributesFixer();

// Handle a click at the beginning/end of a link element.
this._enableClickingAfterLink();
}

/**
Expand Down Expand Up @@ -347,4 +352,65 @@ export default class LinkEditing extends Plugin {
} );
}, { priority: 'low' } );
}

/**
* Starts listening to {@link module:engine/view/document~Document#event:mousedown} and
* {@link module:engine/view/document~Document#event:selectionChange} and puts the selection before/after a link node
* if clicked at the beginning/ending of the link.
*
* The purpose of this action is to allow typing around the link node directly after a click.
*
* See https://github.com/ckeditor/ckeditor5/issues/1016.
*
* @private
*/
_enableClickingAfterLink() {
const editor = this.editor;

editor.editing.view.addObserver( MouseObserver );

let clicked = false;

// Detect the click.
this.listenTo( editor.editing.view.document, 'mousedown', () => {
clicked = true;
} );

// When the selection has changed...
this.listenTo( editor.editing.view.document, 'selectionChange', () => {
if ( !clicked ) {
return;
}

// ...and it was caused by the click...
clicked = false;

const selection = editor.model.document.selection;

// ...and no text is selected...
if ( !selection.isCollapsed ) {
return;
}

// ...and clicked text is the link...
if ( !selection.hasAttribute( 'linkHref' ) ) {
return;
}

const position = selection.getFirstPosition();
const linkRange = findLinkRange( position, selection.getAttribute( 'linkHref' ), editor.model );

// ...check whether clicked start/end boundary of the link.
// If so, remove the `linkHref` attribute.
if ( position.isTouching( linkRange.start ) || position.isTouching( linkRange.end ) ) {
editor.model.change( writer => {
writer.removeSelectionAttribute( 'linkHref' );

for ( const manualDecorator of editor.commands.get( 'link' ).manualDecorators ) {
writer.removeSelectionAttribute( manualDecorator.id );
}
} );
}
} );
}
}
16 changes: 8 additions & 8 deletions packages/ckeditor5-link/tests/linkcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,12 +351,12 @@ describe( 'LinkCommand', () => {
} );

describe( 'collapsed selection', () => {
it( 'should insert text with `linkHref` attribute, text data equal to href and select new link', () => {
it( 'should insert text with `linkHref` attribute, text data equal to href and put the selection after the new link', () => {
setData( model, 'foo[]bar' );

command.execute( 'url' );

expect( getData( model ) ).to.equal( 'foo[<$text linkHref="url">url</$text>]bar' );
expect( getData( model ) ).to.equal( 'foo<$text linkHref="url">url</$text>[]bar' );
} );

it( 'should insert text with `linkHref` attribute, and selection attributes', () => {
Expand All @@ -367,16 +367,16 @@ describe( 'LinkCommand', () => {
command.execute( 'url' );

expect( getData( model ) ).to.equal(
'<$text bold="true">foo</$text>[<$text bold="true" linkHref="url">url</$text>]<$text bold="true">bar</$text>'
'<$text bold="true">foo</$text><$text bold="true" linkHref="url">url</$text><$text bold="true">[]bar</$text>'
);
} );

it( 'should update `linkHref` attribute and select whole link when selection is inside text with `linkHref` attribute', () => {
it( 'should update `linkHref` attribute (text with `linkHref` attribute) and put the selection after the node', () => {
setData( model, '<$text linkHref="other url">foo[]bar</$text>' );

command.execute( 'url' );

expect( getData( model ) ).to.equal( '[<$text linkHref="url">foobar</$text>]' );
expect( getData( model ) ).to.equal( '<$text linkHref="url">foobar</$text>[]' );
} );

it( 'should not insert text with `linkHref` attribute when is not allowed in parent', () => {
Expand Down Expand Up @@ -455,7 +455,7 @@ describe( 'LinkCommand', () => {
command.execute( 'url', { linkIsFoo: true, linkIsBar: true, linkIsSth: true } );

expect( getData( model ) ).to
.equal( 'foo[<$text linkHref="url" linkIsBar="true" linkIsFoo="true" linkIsSth="true">url</$text>]bar' );
.equal( 'foo<$text linkHref="url" linkIsBar="true" linkIsFoo="true" linkIsSth="true">url</$text>[]bar' );
} );

it( 'should add additional attributes to link when link is modified', () => {
Expand All @@ -464,15 +464,15 @@ describe( 'LinkCommand', () => {
command.execute( 'url', { linkIsFoo: true, linkIsBar: true, linkIsSth: true } );

expect( getData( model ) ).to
.equal( 'f[<$text linkHref="url" linkIsBar="true" linkIsFoo="true" linkIsSth="true">ooba</$text>]r' );
.equal( 'f<$text linkHref="url" linkIsBar="true" linkIsFoo="true" linkIsSth="true">ooba</$text>[]r' );
} );

it( 'should remove additional attributes to link if those are falsy', () => {
setData( model, 'foo<$text linkHref="url" linkIsBar="true" linkIsFoo="true">u[]rl</$text>bar' );

command.execute( 'url', { linkIsFoo: false, linkIsBar: false } );

expect( getData( model ) ).to.equal( 'foo[<$text linkHref="url">url</$text>]bar' );
expect( getData( model ) ).to.equal( 'foo<$text linkHref="url">url</$text>[]bar' );
} );
} );

Expand Down
196 changes: 196 additions & 0 deletions packages/ckeditor5-link/tests/linkediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { getData as getModelData, setData as setModelData } from '@ckeditor/cked
import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
import { isLinkElement } from '../src/utils';
import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
import Typing from '@ckeditor/ckeditor5-typing/src/typing';
import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting';

/* global document */

Expand Down Expand Up @@ -851,4 +853,198 @@ describe( 'LinkEditing', () => {
} );
} );
} );

// https://github.com/ckeditor/ckeditor5/issues/1016
describe( 'typing around the link after a click', () => {
let editor;

beforeEach( async () => {
editor = await ClassicTestEditor.create( element, {
plugins: [ Paragraph, LinkEditing, Enter, Typing, BoldEditing ],
link: {
decorators: {
isFoo: {
mode: 'manual',
label: 'Foo',
attributes: {
class: 'foo'
}
},
isBar: {
mode: 'manual',
label: 'Bar',
attributes: {
target: '_blank'
}
}
}
}
} );

model = editor.model;
view = editor.editing.view;
} );

afterEach( async () => {
await editor.destroy();
} );

it( 'should insert content after the link', () => {
setModelData( model, '<paragraph><$text linkHref="url">Bar[]</$text></paragraph>' );

editor.editing.view.document.fire( 'mousedown' );
editor.editing.view.document.fire( 'selectionChange', {
newSelection: view.document.selection
} );

expect( getModelData( model ) ).to.equal( '<paragraph><$text linkHref="url">Bar</$text>[]</paragraph>' );

editor.execute( 'input', { text: 'Foo' } );

expect( getModelData( model ) ).to.equal( '<paragraph><$text linkHref="url">Bar</$text>Foo[]</paragraph>' );
} );

it( 'should insert content before the link', () => {
setModelData( model, '<paragraph><$text linkHref="url">[]Bar</$text></paragraph>' );

editor.editing.view.document.fire( 'mousedown' );
editor.editing.view.document.fire( 'selectionChange', {
newSelection: view.document.selection
} );

expect( getModelData( model ) ).to.equal( '<paragraph>[]<$text linkHref="url">Bar</$text></paragraph>' );

editor.execute( 'input', { text: 'Foo' } );

expect( getModelData( model ) ).to.equal( '<paragraph>Foo[]<$text linkHref="url">Bar</$text></paragraph>' );
} );

it( 'should insert content to the link if clicked inside it', () => {
setModelData( model, '<paragraph><$text linkHref="url">B[]ar</$text></paragraph>' );

editor.editing.view.document.fire( 'mousedown' );
editor.editing.view.document.fire( 'selectionChange', {
newSelection: view.document.selection
} );

expect( getModelData( model ) ).to.equal( '<paragraph><$text linkHref="url">B[]ar</$text></paragraph>' );

editor.execute( 'input', { text: 'ar. B' } );

expect( getModelData( model ) ).to.equal( '<paragraph><$text linkHref="url">Bar. B[]ar</$text></paragraph>' );
} );

it( 'should insert content between two links (selection at the end of the first link)', () => {
setModelData( model, '<paragraph><$text linkHref="foo">Foo[]</$text><$text linkHref="bar">Bar</$text></paragraph>' );

editor.editing.view.document.fire( 'mousedown' );
editor.editing.view.document.fire( 'selectionChange', {
newSelection: view.document.selection
} );

expect( getModelData( model ) ).to.equal(
'<paragraph><$text linkHref="foo">Foo</$text>[]<$text linkHref="bar">Bar</$text></paragraph>'
);

editor.execute( 'input', { text: 'Foo' } );

expect( getModelData( model ) ).to.equal(
'<paragraph><$text linkHref="foo">Foo</$text>Foo[]<$text linkHref="bar">Bar</$text></paragraph>'
);
} );

it( 'should insert content between two links (selection at the beginning of the second link)', () => {
setModelData( model, '<paragraph><$text linkHref="foo">Foo</$text><$text linkHref="bar">[]Bar</$text></paragraph>' );

editor.editing.view.document.fire( 'mousedown' );
editor.editing.view.document.fire( 'selectionChange', {
newSelection: view.document.selection
} );

expect( getModelData( model ) ).to.equal(
'<paragraph><$text linkHref="foo">Foo</$text>[]<$text linkHref="bar">Bar</$text></paragraph>'
);

editor.execute( 'input', { text: 'Foo' } );

expect( getModelData( model ) ).to.equal(
'<paragraph><$text linkHref="foo">Foo</$text>Foo[]<$text linkHref="bar">Bar</$text></paragraph>'
);
} );

it( 'should not touch other attributes than `linkHref`', () => {
setModelData( model, '<paragraph><$text bold="true" linkHref="url">Bar[]</$text></paragraph>' );

editor.editing.view.document.fire( 'mousedown' );
editor.editing.view.document.fire( 'selectionChange', {
newSelection: view.document.selection
} );

expect( getModelData( model ) ).to.equal(
'<paragraph><$text bold="true" linkHref="url">Bar</$text><$text bold="true">[]</$text></paragraph>'
);

editor.execute( 'input', { text: 'Foo' } );

expect( getModelData( model ) ).to.equal(
'<paragraph><$text bold="true" linkHref="url">Bar</$text><$text bold="true">Foo[]</$text></paragraph>'
);
} );

it( 'should do nothing if the text was not clicked', () => {
setModelData( model, '<paragraph><$text linkHref="url">Bar[]</$text></paragraph>' );

editor.editing.view.document.fire( 'selectionChange', {
newSelection: view.document.selection
} );

expect( getModelData( model ) ).to.equal( '<paragraph><$text linkHref="url">Bar[]</$text></paragraph>' );
} );

it( 'should do nothing if the selection is not collapsed after the click', () => {
setModelData( model, '<paragraph>[<$text linkHref="url">Bar</$text>]</paragraph>' );

editor.editing.view.document.fire( 'mousedown' );
editor.editing.view.document.fire( 'selectionChange', {
newSelection: view.document.selection
} );

expect( getModelData( model ) ).to.equal( '<paragraph>[<$text linkHref="url">Bar</$text>]</paragraph>' );
} );

it( 'should do nothing if the text is not a link', () => {
setModelData( model, '<paragraph><$text bold="true">Bar[]</$text></paragraph>' );

editor.editing.view.document.fire( 'mousedown' );
editor.editing.view.document.fire( 'selectionChange', {
newSelection: view.document.selection
} );

expect( getModelData( model ) ).to.equal( '<paragraph><$text bold="true">Bar[]</$text></paragraph>' );
} );

it( 'should remove manual decorators', () => {
model.schema.extend( '$text', {
allowIn: '$root',
allowAttributes: [ 'linkIsFoo', 'linkIsBar' ]
} );

setModelData( model, '<paragraph><$text linkIsFoo="true" linkIsBar="true" linkHref="url">Bar[]</$text></paragraph>' );

editor.editing.view.document.fire( 'mousedown' );
editor.editing.view.document.fire( 'selectionChange', {
newSelection: view.document.selection
} );

expect( getModelData( model ) ).to.equal(
'<paragraph><$text linkHref="url" linkIsBar="true" linkIsFoo="true">Bar</$text>[]</paragraph>'
);

editor.execute( 'input', { text: 'Foo' } );

expect( getModelData( model ) ).to.equal(
'<paragraph><$text linkHref="url" linkIsBar="true" linkIsFoo="true">Bar</$text>Foo[]</paragraph>'
);
} );
} );
} );
Loading

0 comments on commit 0bf66e4

Please sign in to comment.