Browse Source

Merge pull request #216 from ckeditor/t/1501

Fix: Improved the focus management when removing the link form from the DOM. Closes ckeditor/ckeditor5#1501.
Aleksander Nowodzinski 6 years ago
parent
commit
ece7f55031
2 changed files with 30 additions and 3 deletions
  1. 8 3
      packages/ckeditor5-link/src/linkui.js
  2. 22 0
      packages/ckeditor5-link/tests/linkui.js

+ 8 - 3
packages/ckeditor5-link/src/linkui.js

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

+ 22 - 0
packages/ckeditor5-link/tests/linkui.js

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