From acf80a4c531c690127315c94520f162007c9e908 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 20 Apr 2017 17:12:55 +0200 Subject: [PATCH] Fix: Link balloon should hide unlink button when creating a link. Closes #53. --- src/link.js | 35 +++++++++++++++++++++++------------ tests/link.js | 17 +++++++++++++++++ 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/link.js b/src/link.js index 9c910be..6ef380d 100644 --- a/src/link.js +++ b/src/link.js @@ -186,7 +186,7 @@ export default class Link extends Plugin { // Keep panel open until selection will be inside the same link element. this.listenTo( viewDocument, 'click', () => { const viewSelection = viewDocument.selection; - const parentLink = getPositionParentLink( viewSelection.getFirstPosition() ); + const parentLink = this._getSelectedLinkElement(); // When collapsed selection is inside link element (link element is clicked). if ( viewSelection.isCollapsed && parentLink ) { @@ -199,7 +199,7 @@ export default class Link extends Plugin { // Start listen to view document changes and close the panel when selection will be moved // out of the actual link element. this.listenTo( viewDocument, 'render', () => { - const currentParentLink = getPositionParentLink( viewSelection.getFirstPosition() ); + const currentParentLink = this._getSelectedLinkElement(); if ( !viewSelection.isCollapsed || parentLink !== currentParentLink ) { this._hidePanel(); @@ -244,6 +244,9 @@ export default class Link extends Plugin { * @return {Promise} A promise resolved when the {@link #formView} {@link module:ui/view~View#init} is done. */ _showPanel( focusInput ) { + // https://github.com/ckeditor/ckeditor5-link/issues/53 + this.formView.unlinkButtonView.isVisible = !!this._getSelectedLinkElement(); + if ( this._balloon.hasView( this.formView ) ) { // Check if formView should be focused and focus it if is visible. if ( focusInput && this._balloon.visibleView === this.formView ) { @@ -296,7 +299,7 @@ export default class Link extends Plugin { */ _getBalloonPositionData() { const viewDocument = this.editor.editing.view; - const targetLink = getPositionParentLink( viewDocument.selection.getFirstPosition() ); + const targetLink = this._getSelectedLinkElement(); const target = targetLink ? // When selection is inside link element, then attach panel to this element. @@ -310,14 +313,22 @@ export default class Link extends Plugin { limiter: viewDocument.domConverter.getCorrespondingDomElement( viewDocument.selection.editableElement ) }; } -} -// Try to find if one of the position parent ancestors is a LinkElement, -// if yes return this element. -// -// @private -// @param {engine.view.Position} position -// @returns {module:link/linkelement~LinkElement|null} -function getPositionParentLink( position ) { - return position.parent.getAncestors().find( ( ancestor ) => ancestor instanceof LinkElement ); + /** + * Returns the {@link module:link/linkelement~LinkElement} at the first + * {@link module:engine/model/position~Position} of + * {@link module:engine/view/document~Document editing view's} selection or `null` + * if there's none. + * + * @private + * @returns {module:link/linkelement~LinkElement|null} + */ + _getSelectedLinkElement() { + return this.editor.editing.view + .selection + .getFirstPosition() + .parent + .getAncestors() + .find( ancestor => ancestor instanceof LinkElement ); + } } diff --git a/tests/link.js b/tests/link.js index 068d415..5b657f7 100644 --- a/tests/link.js +++ b/tests/link.js @@ -196,6 +196,23 @@ describe( 'Link', () => { sinon.assert.calledOnce( spy ); } ); } ); + + // https://github.com/ckeditor/ckeditor5-link/issues/53 + it( 'should set formView.unlinkButtonView#isVisible depending on the selection in a link or not', () => { + setModelData( editor.document, 'f[]oo' ); + + return linkFeature._showPanel() + .then( () => { + expect( formView.unlinkButtonView.isVisible ).to.be.false; + + setModelData( editor.document, '<$text linkHref="url">f[]oo' ); + + return linkFeature._showPanel() + .then( () => { + expect( formView.unlinkButtonView.isVisible ).to.be.true; + } ); + } ); + } ); } ); describe( '_hidePanel()', () => {