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 #216 from ckeditor/t/1501
Browse files Browse the repository at this point in the history
Fix: Improved the focus management when removing the link form from the DOM. Closes ckeditor/ckeditor5#1501.
  • Loading branch information
oleq authored Feb 18, 2019
2 parents 22b0b17 + d080fa5 commit 9dd756c
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
11 changes: 8 additions & 3 deletions src/linkui.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,10 @@ export default class LinkUI extends Plugin {
*/
_removeFormView() {
if ( this._isFormInPanel ) {
// Blur the input element before removing it from DOM to prevent issues in some browsers.
// See https://github.com/ckeditor/ckeditor5/issues/1501.
this.formView.saveButtonView.focus();

this._balloon.remove( this.formView );

// Because the form has an input which has focus, the focus must be brought back
Expand Down Expand Up @@ -372,14 +376,15 @@ export default class LinkUI extends Plugin {

this.stopListening( editor.ui, 'update' );

// Make sure the focus always gets back to the editable _before_ removing the focused form view.
// Doing otherwise causes issues in some browsers. See https://github.com/ckeditor/ckeditor5-link/issues/193.
editor.editing.view.focus();

// Remove form first because it's on top of the stack.
this._removeFormView();

// Then remove the actions view because it's beneath the form.
this._balloon.remove( this.actionsView );

// Make sure the focus always gets back to the editable.
editor.editing.view.focus();
}

/**
Expand Down
22 changes: 22 additions & 0 deletions tests/linkui.js
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,16 @@ describe( 'LinkUI', () => {
sinon.assert.calledTwice( spy );
} );

// https://github.com/ckeditor/ckeditor5-link/issues/193
it( 'should focus the `editable` before before removing elements from the balloon', () => {
const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' );
const removeSpy = testUtils.sinon.spy( balloon, 'remove' );

linkUIFeature._hideUI();

expect( focusSpy.calledBefore( removeSpy ) ).to.equal( true );
} );

it( 'should not throw an error when views are not in the `balloon`', () => {
linkUIFeature._hideUI();

Expand Down Expand Up @@ -823,6 +833,18 @@ describe( 'LinkUI', () => {
expect( balloon.visibleView ).to.equal( actionsView );
expect( focusEditableSpy.calledOnce ).to.be.true;
} );

// https://github.com/ckeditor/ckeditor5/issues/1501
it( 'should blur url input element before hiding the view', () => {
linkUIFeature._showUI();

const focusSpy = testUtils.sinon.spy( formView.saveButtonView, 'focus' );
const removeSpy = testUtils.sinon.spy( balloon, 'remove' );

formView.fire( 'cancel' );

expect( focusSpy.calledBefore( removeSpy ) ).to.equal( true );
} );
} );
} );
} );

0 comments on commit 9dd756c

Please sign in to comment.