Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contextualToolbar -> balloonToolbar? #550

Closed
wwalc opened this issue Sep 7, 2017 · 1 comment
Closed

contextualToolbar -> balloonToolbar? #550

wwalc opened this issue Sep 7, 2017 · 1 comment
Assignees
Labels
package:editor-balloon package:editor-inline package:ui type:improvement This issue reports a possible enhancement of an existing feature.
Milestone

Comments

@wwalc
Copy link
Member

wwalc commented Sep 7, 2017

I edited my initial, longer report as I realised I was wrong regarding the lack of possibility of setting the toolbar of balloon editor via config.toolbar


A detail, but I wonder whether config.contextualToolbar shouldn't be renamed to config.balloonToolbar (same with the class name - ContextualToolbar). I think at some point we came to a conclusion that the toolbar isn't actually contextual.

@Reinmar
Copy link
Member

Reinmar commented Sep 8, 2017

I edited my initial, longer report as I realised I was wrong regarding the lack of possibility of setting the toolbar of balloon editor via config.toolbar

Yes, this is something which was tricky for us. The BalloonEditor uses ContextualToolbar plugin to implement the toolbar. That's because that functionality was only exposed in form of a plugin which can only be configured through the config option.

To straighten this up we'd need to split the plugin to ContextualToolbarController (ekhm for the "controller") and ContextualToolbar which only glues a couple of things together. This is in line with #425.

After such refactoring, this will be the state of things:

  • BalloonEditor will use ContextualToolbarController and will not have to silently load the plugin and fill config.contextualToolbar with what's in config.toolbar.

  • There will still be two options in EditorConfigtoolbar for the "editor toolbar" configuration and contextualToolbar for the plugin configuration. This is still a bit problematic, but it's hard to resolve. Perhaps we should've had defined plugin configs completely separately from editor config, but that was far beyond our perspective inherited from CKE4.

    However, one thing we can do here now is to extract config.contextualToolbar to a separate interface, just like with config.image and config.heading. This will, hopefully, better indicate that this is separate plugin's config.


Regarding renaming... definitely. I've just noticed that ContextualToolbar uses BalloonPanel and is used in BalloonEditor :D. The Ctx prefix made sense only when it was related to the editor name.

@Reinmar Reinmar added candidate:1.0.0 package:ui type:improvement This issue reports a possible enhancement of an existing feature. labels Sep 8, 2017
@Reinmar Reinmar mentioned this issue Oct 20, 2017
5 tasks
@oleq oleq added this to the iteration 15 milestone Feb 7, 2018
Reinmar added a commit to ckeditor/ckeditor5-core that referenced this issue Feb 21, 2018
Internal: Aligned code after ranaming ContextualToolbar to BallonToolbar. See ckeditor/ckeditor5#550.
Reinmar added a commit to ckeditor/ckeditor5-editor-balloon that referenced this issue Feb 21, 2018
Internal: Aligned code after ranaming ContextualToolbar to BallonToolbar. See ckeditor/ckeditor5#550.
Reinmar added a commit to ckeditor/ckeditor5-image that referenced this issue Feb 21, 2018
Internal: Aligned code after ranaming ContextualToolbar to BallonToolbar. See ckeditor/ckeditor5#550.
Reinmar added a commit that referenced this issue Feb 21, 2018
Docs: Aligned docs after ranaming ContextualToolbar to BallonToolbar. Closes #550.
@Reinmar Reinmar modified the milestones: iteration 15, iteration 14 Feb 22, 2018
@Reinmar Reinmar mentioned this issue Oct 9, 2019
18 tasks
mlewand pushed a commit that referenced this issue May 1, 2020
Internal: Cleaned up the FormHeaderView docs and code. Closes #6507.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:editor-balloon package:editor-inline package:ui type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

No branches or pull requests

5 participants