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

Commit

Permalink
Merge pull request #147 from ckeditor/t/ckeditor5/447
Browse files Browse the repository at this point in the history
Fix: The Link keystrokes should properly integrate with the List feature. Improved balloon positioning. Closes #146.
  • Loading branch information
Reinmar committed Aug 22, 2017
2 parents 87885e2 + 24c5e0e commit 4d808b7
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 8 deletions.
12 changes: 6 additions & 6 deletions src/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,11 @@ export default class Link extends Plugin {
this.formView.focus();
cancel();
}
}, {
// Use the high priority because the link UI navigation is more important
// than other feature's actions, e.g. list indentation.
// https://github.com/ckeditor/ckeditor5-link/issues/146
priority: 'high'
} );

// Close the panel on the Esc key press when the editable has focus and the balloon is visible.
Expand Down Expand Up @@ -278,14 +283,9 @@ export default class Link extends Plugin {
// * there was no link element in the first place, i.e. creating a new link
else {
// If still in a link element, simply update the position of the balloon.
if ( renderSelectedLink ) {
this._balloon.updatePosition();
}
// If there was no link, upon #render, the balloon must be moved
// to the new position in the editing view (a new native DOM range).
else {
this._balloon.updatePosition( this._getBalloonPositionData() );
}
this._balloon.updatePosition( this._getBalloonPositionData() );
}
} );

Expand Down
15 changes: 14 additions & 1 deletion tests/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,9 @@ describe( 'Link', () => {
viewDocument.render();

sinon.assert.calledOnce( spy );
sinon.assert.calledWithExactly( spy );
sinon.assert.calledWithExactly( spy, {
target: viewDocument.domConverter.mapViewToDom( root.getChild( 0 ).getChild( 0 ) )
} );
} );

// https://github.com/ckeditor/ckeditor5-link/issues/113
Expand Down Expand Up @@ -481,6 +483,11 @@ describe( 'Link', () => {
stopPropagation: sinon.spy()
};

const normalPriorityTabCallbackSpy = sinon.spy();
const highestPriorityTabCallbackSpy = sinon.spy();
editor.keystrokes.set( 'Tab', normalPriorityTabCallbackSpy );
editor.keystrokes.set( 'Tab', highestPriorityTabCallbackSpy, { priority: 'highest' } );

// Balloon is invisible, form not focused.
formView.focusTracker.isFocused = false;

Expand All @@ -490,6 +497,8 @@ describe( 'Link', () => {
sinon.assert.notCalled( keyEvtData.preventDefault );
sinon.assert.notCalled( keyEvtData.stopPropagation );
sinon.assert.notCalled( spy );
sinon.assert.calledOnce( normalPriorityTabCallbackSpy );
sinon.assert.calledOnce( highestPriorityTabCallbackSpy );

// Balloon is visible, form focused.
linkFeature._showPanel( true );
Expand All @@ -499,6 +508,8 @@ describe( 'Link', () => {
sinon.assert.notCalled( keyEvtData.preventDefault );
sinon.assert.notCalled( keyEvtData.stopPropagation );
sinon.assert.notCalled( spy );
sinon.assert.calledTwice( normalPriorityTabCallbackSpy );
sinon.assert.calledTwice( highestPriorityTabCallbackSpy );

// Balloon is still visible, form not focused.
formView.focusTracker.isFocused = false;
Expand All @@ -507,6 +518,8 @@ describe( 'Link', () => {
sinon.assert.calledOnce( keyEvtData.preventDefault );
sinon.assert.calledOnce( keyEvtData.stopPropagation );
sinon.assert.calledOnce( spy );
sinon.assert.calledTwice( normalPriorityTabCallbackSpy );
sinon.assert.calledThrice( highestPriorityTabCallbackSpy );
} );

it( 'should hide the #_balloon after Esc key press (from editor) and not focus the editable', () => {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/linkformview.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ describe( 'LinkFormView', () => {
} );

describe( 'activates keyboard navigation for the toolbar', () => {
it( 'so "tab" the next focusable item', () => {
it( 'so "tab" focuses the next focusable item', () => {
const keyEvtData = {
keyCode: keyCodes.tab,
preventDefault: sinon.spy(),
Expand Down

0 comments on commit 4d808b7

Please sign in to comment.