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

Improved focus management test. #96

Merged
merged 3 commits into from
Apr 14, 2017
Merged

Improved focus management test. #96

merged 3 commits into from
Apr 14, 2017

Conversation

oskarwrobel
Copy link
Contributor

@oskarwrobel oskarwrobel commented Apr 7, 2017

Suggested merge commit message (convention)

Fix: Improved focus management test. Closes ckeditor/ckeditor5#4779.


Needs: ckeditor/ckeditor5-ui#194

@oskarwrobel oskarwrobel requested a review from oleq April 7, 2017 13:35
@oleq
Copy link
Member

oleq commented Apr 12, 2017

The editor toolbar remains now, but the focus does not go into the URL input:
kapture 2017-04-12 at 17 36 26

@oskarwrobel
Copy link
Contributor Author

oskarwrobel commented Apr 12, 2017

  1. Bug is already on master.

  2. It's really easy to fix it:

this._balloon.add( {
	view: this.formView,
	position: this._getBalloonPositionData()
} );

if ( focusInput ) {
	this.formView.urlInputView.select();
}

vs

this._balloon.add( {
	view: this.formView,
	position: this._getBalloonPositionData()
} ).then( () => {
	if ( focusInput ) {
		this.formView.urlInputView.select();
	}
} );
  1. There is a problem (related to: https://github.com/ckeditor/ckeditor5-ui/issues/203) with unit tests. Test finishes before promise.then() is called.

expect( selectUrlInputSpy.calledOnce ).to.true;
// Focus of url input is called async after internal promise resolve and we are
// not able to return this promise.
return wait().then( () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an ugly hack, I don't like it but I didn't figure out anything less complicated.

@oskarwrobel
Copy link
Contributor Author

I'm refactoring Link tests so let's wait with review till I finish.

@oleq oleq merged commit 472713e into master Apr 14, 2017
@oleq oleq deleted the t/95 branch April 14, 2017 12:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken focus management in editor-inline
2 participants