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

t/3: Implemented a keystroke to display the toolbar in a collapsed selection #8

Merged
merged 4 commits into from
Jul 20, 2017

Conversation

oleq
Copy link
Member

@oleq oleq commented Jul 10, 2017

Suggested merge commit message (convention)

Other: Implemented a keystroke to display the toolbar in a collapsed selection. Closes ckeditor/ckeditor5#2344.


Requires ckeditor/ckeditor5-ui#270.

@Reinmar
Copy link
Member

Reinmar commented Jul 11, 2017

Tests?

@@ -69,7 +69,13 @@ export default class BalloonToolbarEditorUI {
origin: editor.editing.view,
originFocusTracker: this.focusTracker,
originKeystrokeHandler: editor.keystrokes,
toolbar: contextualToolbar.toolbarView
toolbar: contextualToolbar.toolbarView,
beforeFocus: () => {
Copy link
Member

Choose a reason for hiding this comment

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

beforeFocus() { ....

Copy link
Member

Choose a reason for hiding this comment

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

BTW, don't you think that beforeFocus callback doing show() is kinda odd? It's hard to read. I had to look into the code to figure out what's going on here.

Copy link
Member

@Reinmar Reinmar Jul 11, 2017

Choose a reason for hiding this comment

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

After a couple more minutes this looks reasonable... :D So I'm ok. But beforeFocus() { ... } stays.

@oleq
Copy link
Member Author

oleq commented Jul 12, 2017

Tests?

Well, this is what happens when you start your job one week and finish the next 😅

@Reinmar
Copy link
Member

Reinmar commented Jul 20, 2017

❤️ (maybe except that blur at the end, but that's not a priority now)

jul-20-2017 11-14-03

@Reinmar Reinmar merged commit c745e05 into master Jul 20, 2017
@Reinmar Reinmar deleted the t/3 branch July 20, 2017 09:15
@oleq
Copy link
Member Author

oleq commented Jul 20, 2017

AFAIR this blur is a problem we already know about ckeditor/ckeditor5#353.

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.

Implement a keystroke to display the toolbar in a collapsed selection
2 participants