t/1: The first implementation of the balloon toolbar editor #2
Conversation
src/balloontoolbar.js
Outdated
| */ | ||
|
|
||
| /** | ||
| * @module editor-balloon-toolbar/contextual |
There was a problem hiding this comment.
Shouldn't it be editor-balloon-toolbar/balloontoolbareditor ?
src/balloontoolbar.js
Outdated
| } | ||
|
|
||
| /** | ||
| * Creates an balloon toolbar editor instance. |
src/balloontoolbar.js
Outdated
|
|
||
| /** | ||
| * The balloon toolbar editor. Uses an inline editable and a toolbar based | ||
| * on the {@link ui/toolbar/contextual/contextualtoolbar~ContextualToolbar}. |
There was a problem hiding this comment.
module:ui/toolbar/contextual/contextualtoolbar~ContextualToolbar
| super( element, config ); | ||
|
|
||
| this.config.get( 'plugins' ).push( ContextualToolbar ); | ||
| this.config.define( 'contextualToolbar', this.config.get( 'toolbar' ) ); |
There was a problem hiding this comment.
I am wondering if this will not cause any issues, because it will (correct me if I am wrong) copy reference to same object, so further changes to toolbar configuration will also affect contextualToolbar. Or maybe this is desired behavior?
There was a problem hiding this comment.
Well... this is quite a story. At the very beginning, this editor type expected developers to configure it like this:
BalloonToolbarEditor.create( editorElement, {
plugins: [ ContextualToolbar ],
contextualToolbar: [ ... ]
} );but in a F2F talk with @Reinmar we found out this is not a very straightforward since editor-inline and editor-classic don't require a) any special plugins b) custom config names (toolbar vs contextualToolbar).
So we decided to hack things a little bit to load ContextualToolbar automatically and create a "bridge" between cfg.toolbar and cfg.contextualToolbar to keep the API between the creators as similar as possible.
so further changes to toolbar configuration will also affect contextualToolbar
Well, I suppose it doesn't matter. Once you decide to use BalloonToolbarEditor, it takes the full control over ContextualToolbar. I don't see how ContextualToolbar could remain useful for other features once seized by the BalloonToolbarEditor 🤔 .
|
|
||
| @import '~@ckeditor/ckeditor5-theme-lark/theme/theme.scss'; | ||
|
|
||
| // TODO move to make a common style with editor-classic. |
There was a problem hiding this comment.
Is it for the future reference?
There was a problem hiding this comment.
I created an issue for this. We'll fix it as a follow-up.
Other: Optimized the bundle size (~10%) by enabling webpack's `ModuleConcatenationPlugin` plugin. Read more ckeditor/ckeditor5#475.
Suggested merge commit message (convention)
Feature: The first implementation of the balloon toolbar editor. Closes ckeditor/ckeditor5#2337.
Requires