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

t/23: The editing UI should show up when the selection encloses a link #122

Merged
merged 1 commit into from
May 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 37 additions & 11 deletions src/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import ClickObserver from '@ckeditor/ckeditor5-engine/src/view/observer/clickobserver';
import Range from '@ckeditor/ckeditor5-engine/src/view/range';
import LinkEngine from './linkengine';
import LinkElement from './linkelement';
import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon';
Expand Down Expand Up @@ -185,11 +186,9 @@ export default class Link extends Plugin {
// Handle click on view document and show panel when selection is placed inside the link element.
// Keep panel open until selection will be inside the same link element.
this.listenTo( viewDocument, 'click', () => {
const viewSelection = viewDocument.selection;
const parentLink = this._getSelectedLinkElement();

// When collapsed selection is inside link element (link element is clicked).
if ( viewSelection.isCollapsed && parentLink ) {
if ( parentLink ) {
// Then show panel but keep focus inside editor editable.
this._showPanel();
}
Expand Down Expand Up @@ -336,20 +335,47 @@ export default class Link extends Plugin {
}

/**
* Returns the {@link module:link/linkelement~LinkElement} at the first
* {@link module:engine/model/position~Position} of
* Returns the {@link module:link/linkelement~LinkElement} under
* {@link module:engine/view/document~Document editing view's} selection or `null`
* if there's none.
*
* **Note**: For non–collapsed selection the `LinkElement` is only returned when **fully**
* selected and the **only** element within the selection boundaries.
*
* @private
* @returns {module:link/linkelement~LinkElement|null}
*/
_getSelectedLinkElement() {
return this.editor.editing.view
.selection
.getFirstPosition()
.parent
.getAncestors()
.find( ancestor => ancestor instanceof LinkElement );
const selection = this.editor.editing.view.selection;

if ( selection.isCollapsed ) {
return findLinkElementAncestor( selection.getFirstPosition() );
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would follow this rule http://eslint.org/docs/rules/no-else-return but it's only a preference.

// The range for fully selected link is usually anchored in adjacent text nodes.
// Trim it to get closer to the actual LinkElement.
const range = selection.getFirstRange().getTrimmed();
const startLink = findLinkElementAncestor( range.start );
const endLink = findLinkElementAncestor( range.end );

if ( !startLink || startLink != endLink ) {
return null;
}

// Check if the LinkElement is fully selected.
if ( Range.createIn( startLink ).getTrimmed().isEqual( range ) ) {
return startLink;
} else {
return null;
}
}
}
}

// Returns a `LinkElement` if there's one among the ancestors of the provided `Position`.
//
// @private
// @param {module:engine/view/position~Position} View position to analyze.
// @returns {module:link/linkelement~LinkElement|null} LinkElement at the position or null.
function findLinkElementAncestor( position ) {
return position.getAncestors().find( ancestor => ancestor instanceof LinkElement );
}
60 changes: 47 additions & 13 deletions tests/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -582,43 +582,77 @@ describe( 'Link', () => {
} );

describe( 'clicking on editable', () => {
let observer;
let observer, spy;

beforeEach( () => {
observer = editor.editing.view.getObserver( ClickObserver );
} );
editor.document.schema.allow( { name: '$text', inside: '$root' } );

it( 'should open with not selected formView when collapsed selection is inside link element', () => {
// Method is stubbed because it returns internal promise which can't be returned in test.
const spy = testUtils.sinon.stub( linkFeature, '_showPanel', () => {} );
spy = testUtils.sinon.stub( linkFeature, '_showPanel', () => {} );
} );

editor.document.schema.allow( { name: '$text', inside: '$root' } );
it( 'should open with not selected formView when collapsed selection is inside link element', () => {
setModelData( editor.document, '<$text linkHref="url">fo[]o</$text>' );

observer.fire( 'click', { target: document.body } );
sinon.assert.calledWithExactly( spy );
} );

it( 'should open when selection exclusively encloses a LinkElement (#1)', () => {
setModelData( editor.document, '[<$text linkHref="url">foo</$text>]' );

observer.fire( 'click', { target: {} } );
sinon.assert.calledWithExactly( spy );
} );

it( 'should not open when selection is not inside link element', () => {
const showSpy = testUtils.sinon.stub( linkFeature, '_showPanel' );
it( 'should open when selection exclusively encloses a LinkElement (#2)', () => {
setModelData( editor.document, '<$text linkHref="url">[foo]</$text>' );

observer.fire( 'click', { target: {} } );
sinon.assert.calledWithExactly( spy );
} );

it( 'should not open when selection is not inside link element', () => {
setModelData( editor.document, '[]' );

observer.fire( 'click', { target: {} } );
sinon.assert.notCalled( spy );
} );

it( 'should not open when selection is non-collapsed and doesn\'t enclose a LinkElement (#1)', () => {
setModelData( editor.document, '<$text linkHref="url">f[o]o</$text>' );

sinon.assert.notCalled( showSpy );
observer.fire( 'click', { target: {} } );
sinon.assert.notCalled( spy );
} );

it( 'should not open when selection is non-collapsed', () => {
const showSpy = testUtils.sinon.stub( linkFeature, '_showPanel' );
it( 'should not open when selection is non-collapsed and doesn\'t enclose a LinkElement (#2)', () => {
setModelData( editor.document, '<$text linkHref="url">[fo]o</$text>' );

editor.document.schema.allow( { name: '$text', inside: '$root' } );
setModelData( editor.document, '<$text linkHref="url">f[o]o</$text>' );
observer.fire( 'click', { target: {} } );
sinon.assert.notCalled( spy );
} );

it( 'should not open when selection is non-collapsed and doesn\'t enclose a LinkElement (#3)', () => {
setModelData( editor.document, '<$text linkHref="url">f[oo]</$text>' );

observer.fire( 'click', { target: {} } );
sinon.assert.notCalled( spy );
} );

it( 'should not open when selection is non-collapsed and doesn\'t enclose a LinkElement (#4)', () => {
setModelData( editor.document, 'ba[r<$text linkHref="url">foo]</$text>' );

observer.fire( 'click', { target: {} } );
sinon.assert.notCalled( spy );
} );

sinon.assert.notCalled( showSpy );
it( 'should not open when selection is non-collapsed and doesn\'t enclose a LinkElement (#5)', () => {
setModelData( editor.document, 'ba[r<$text linkHref="url">foo</$text>]' );

observer.fire( 'click', { target: {} } );
sinon.assert.notCalled( spy );
} );
} );
} );
Expand Down