瀏覽代碼

Merge pull request #7440 from ckeditor/i/1016

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.
Maciej 5 年之前
父節點
當前提交
0bf66e47ee

+ 10 - 4
packages/ckeditor5-link/src/linkcommand.js

@@ -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.
@@ -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.

+ 68 - 2
packages/ckeditor5-link/src/linkediting.js

@@ -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';
@@ -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();
 	}
 
 	/**
@@ -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 );
+					}
+				} );
+			}
+		} );
+	}
 }

+ 8 - 8
packages/ckeditor5-link/tests/linkcommand.js

@@ -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', () => {
@@ -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', () => {
@@ -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', () => {
@@ -464,7 +464,7 @@ 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', () => {
@@ -472,7 +472,7 @@ describe( 'LinkCommand', () => {
 
 				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' );
 			} );
 		} );
 

+ 196 - 0
packages/ckeditor5-link/tests/linkediting.js

@@ -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 */
 
@@ -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>'
+			);
+		} );
+	} );
 } );

+ 21 - 13
packages/ckeditor5-link/tests/linkui.js

@@ -981,7 +981,7 @@ describe( 'LinkUI', () => {
 
 						expect( defaultProtocol ).to.equal( 'https://' );
 
-						editor.destroy();
+						return editor.destroy();
 					} );
 			} );
 
@@ -994,56 +994,64 @@ describe( 'LinkUI', () => {
 
 			it( 'should not add a protocol to the local links even when `config.link.defaultProtocol` configured', () => {
 				return createEditorWithDefaultProtocol( 'http://' ).then( ( { editor, formView } ) => {
+					const linkCommandSpy = sinon.spy( editor.commands.get( 'link' ), 'execute' );
 					formView.urlInputView.fieldView.value = '#test';
 					formView.fire( 'submit' );
 
-					expect( formView.urlInputView.fieldView.value ).to.equal( '#test' );
+					sinon.assert.calledWith( linkCommandSpy, '#test', sinon.match.any );
 
-					editor.destroy();
+					return editor.destroy();
 				} );
 			} );
 
 			it( 'should not add a protocol to the relative links even when `config.link.defaultProtocol` configured', () => {
 				return createEditorWithDefaultProtocol( 'http://' ).then( ( { editor, formView } ) => {
+					const linkCommandSpy = sinon.spy( editor.commands.get( 'link' ), 'execute' );
 					formView.urlInputView.fieldView.value = '/test.html';
 					formView.fire( 'submit' );
 
-					expect( formView.urlInputView.fieldView.value ).to.equal( '/test.html' );
+					sinon.assert.calledWith( linkCommandSpy, '/test.html', sinon.match.any );
 
-					editor.destroy();
+					return editor.destroy();
 				} );
 			} );
 
 			it( 'should not add a protocol when given provided within the value even when `config.link.defaultProtocol` configured', () => {
 				return createEditorWithDefaultProtocol( 'http://' ).then( ( { editor, formView } ) => {
+					const linkCommandSpy = sinon.spy( editor.commands.get( 'link' ), 'execute' );
 					formView.urlInputView.fieldView.value = 'http://example.com';
 					formView.fire( 'submit' );
 
 					expect( formView.urlInputView.fieldView.value ).to.equal( 'http://example.com' );
+					sinon.assert.calledWith( linkCommandSpy, 'http://example.com', sinon.match.any );
 
-					editor.destroy();
+					return editor.destroy();
 				} );
 			} );
 
 			it( 'should use the "http://" protocol when it\'s configured', () => {
 				return createEditorWithDefaultProtocol( 'http://' ).then( ( { editor, formView } ) => {
+					const linkCommandSpy = sinon.spy( editor.commands.get( 'link' ), 'execute' );
+
 					formView.urlInputView.fieldView.value = 'ckeditor.com';
 					formView.fire( 'submit' );
 
-					expect( formView.urlInputView.fieldView.value ).to.equal( 'http://ckeditor.com' );
+					sinon.assert.calledWith( linkCommandSpy, 'http://ckeditor.com', sinon.match.any );
 
-					editor.destroy();
+					return editor.destroy();
 				} );
 			} );
 
 			it( 'should use the "http://" protocol when it\'s configured and form input value contains "www."', () => {
 				return createEditorWithDefaultProtocol( 'http://' ).then( ( { editor, formView } ) => {
+					const linkCommandSpy = sinon.spy( editor.commands.get( 'link' ), 'execute' );
+
 					formView.urlInputView.fieldView.value = 'www.ckeditor.com';
 					formView.fire( 'submit' );
 
-					expect( formView.urlInputView.fieldView.value ).to.equal( 'http://www.ckeditor.com' );
+					sinon.assert.calledWith( linkCommandSpy, 'http://www.ckeditor.com', sinon.match.any );
 
-					editor.destroy();
+					return editor.destroy();
 				} );
 			} );
 
@@ -1058,7 +1066,7 @@ describe( 'LinkUI', () => {
 						'[<$text linkHref="http://ckeditor.com">ckeditor.com</$text>]'
 					);
 
-					editor.destroy();
+					return editor.destroy();
 				} );
 			} );
 
@@ -1074,7 +1082,7 @@ describe( 'LinkUI', () => {
 						'[<$text linkHref="mailto:email@example.com">email@example.com</$text>]'
 					);
 
-					editor.destroy();
+					return editor.destroy();
 				} );
 			} );
 
@@ -1086,7 +1094,7 @@ describe( 'LinkUI', () => {
 
 					expect( formView.urlInputView.fieldView.value ).to.equal( 'mailto:test@example.com' );
 
-					editor.destroy();
+					return editor.destroy();
 				} );
 			} );
 		} );

+ 5 - 2
packages/ckeditor5-link/tests/manual/link.md

@@ -7,7 +7,8 @@
 3. Check if balloon panel attached to the selection appeared.
 4. Fill in `Link URL` input in the panel.
 5. Click `Save` button.
-6. Check if selected text is converted into a link.
+6. Check if the selection is after the text that was converted into a link.
+7. Typing should not modify the link node.
 
 ### Insert new link
 
@@ -16,7 +17,8 @@
 3. Check if balloon panel attached to the selection appeared.
 4. Fill in `Link URL` input in the panel.
 5. Click `Save` button.
-6. Check if new link with anchor text the same as url value has been inserted and selected.
+6. Check if new link with anchor text the same as url value has been inserted.
+7. Check if the selection is after the text node. Typing should not modify the node.
 
 ### Edit link
 
@@ -25,6 +27,7 @@
 3. Change `Link URL` input value.
 4. Click `Save` button.
 5. Check if link href value has changed.
+6. Check if the selection is after the updated text node. Typing should not modify the node.
 
 ### Keyboard support
 

+ 7 - 0
packages/ckeditor5-mention/tests/mention-integration.js

@@ -304,8 +304,15 @@ describe( 'Mention feature - integration', () => {
 
 			return new Promise( resolve => setTimeout( resolve, 200 ) )
 				.then( () => {
+					const model = editor.model;
+
 					// Show link UI
 					editor.execute( 'link', '@' );
+					// The link is not being selected after inserting it. We need to put the selection manually. See #1016.
+					model.change( writer => {
+						writer.setSelection( writer.createRangeOn( model.document.getRoot().getChild( 0 ).getChild( 0 ) ) );
+					} );
+
 					editor.editing.view.document.fire( 'click' );
 
 					expect( panelView.isVisible ).to.be.true;