-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
A bit confusing editor initialization. #2245
A bit confusing editor initialization. #2245
Comments
What do you propose to make it less confusing? Given the specific nature and the purpose of this editor, the API we created makes a lot of sense to me, TBH. |
@oleq Yeah but the very first user had problems with using it - that's why I'm reporting this. |
The API made a lot of sense for me too. But I'm afraid that we might've fixated on the purpose of this editor so much that we missed how its API might be confusing. I've heard from two guys today that they don't understand how to enable this editor. And this can be for two reasons:
Therefore, we should start from checking whether the documentation stresses the fact what happens when you initialise this editor. However, we should also think about making it work similarly to other editors. I've been discussing this with @wwalc and have some ideas, but, TBH, it'll be tricky. First of all – specifying a string as a data source is unnecessarily problematic in some cases. When rendering a website it's usually easier to put the content into some HTML element (div or textarea). So, we might add the ability to initialise this editor on an element, like all other editors: DecoupledEditor
.create( document.querySelector( '#data-source' ) )
... However, I haven't named this element So, it will still be a bit confusing. The above code will result in... hiding the WDYT, guys? |
I think that making them separate (editing element and data source element) will be confusing as well. I think that an expected thing is to have "editable area with an existing HTML content" where the "existing HTML content" was on page. But then thinking on how such editor might be used is: I have areas where I want to create editor: so those have to be defined. Then either I have existing content - let's say in that editable/editor area or from other source - let it be JS string. Then I'd like to provide this data to the editor upon creating or just right after. Looking how I believe it would be used i think that HTML data to load should be taken either from the editable area or passed as a parameter (or alternatively set later on to the editor). The third element Maybe some explicit errors would help. Ie in situation of loading data from existing editable container and passing a initial data at the same time the editor should throw? |
That's a good point too.
Should it set data back to the |
I get the feeling that it depends:
|
I agree with @jodator about confusing init. From my perspective - someone who doesn't make anything with CKEditor, besides using builds to create demos... All editors are initialized by This type of editor is different, the toolbar may be in a different place than editable area. When I'm aware of that, I think that the first parameter should be checked whether this is a string or Node element, if string then use it as the source of data, if Node then as editable area and its content as the data source. This behavior can be overwritten by What was the most confusing for me, is a fact that when I provide a string with editable content, then it is appended to the editableContainer, but it's not cleared before. My data is located after existed content. I don't see a case when this is useful, IMO container's content should be replaced by data from the first argument. |
Maybe we should remove both config options – editableContainer and toolbarContainer? There would be two ways to initialize the editor then: DecoupledEditor
.create( elementToBeConvertedToEditable ) // like in InlineEditor!
.then( editor => {
someContainer.appendChild( editor.ui.toolbar.element );
} );
DecoupledEditor
.create( dataString )
.then( editor => {
someContainer.appendChild( editor.ui.toolbar.element );
someOtherContainer.appendChild( editor.ui.editable.element );
} ); I think that this is very similar to other editors. WDYT? |
LGTM, plus dataString should replace |
It won't replace it in the code sample I showed above – the editable will be appended to |
Can I assume that everyone agrees with https://github.com/ckeditor/ckeditor5-editor-decoupled/issues/10#issuecomment-374545217? I'd like to implement it ASAP. |
@jodator WDYT? |
Other: Allowed the editable element to be passed into `DecoupledEditor.create()`. Removed `config.toolbarContainer` and `config.editableContainer`. Closes #10. Closes ckeditor/ckeditor5#912. BREAKING CHANGE: The config options `config.toolbarContainer` and `config.editableContainer` have been removed. Please refer to the `DecoupledEditor` class API documentation to learn about possible methods of bootstrapping the UI.
Other: Updated the build to the latest `DecoupledEditor` class API (see ckeditor/ckeditor5-editor-decoupled#10). BREAKING CHANGE: The config options `config.toolbarContainer` and `config.editableContainer` have been removed. Please refer to the `DecoupledEditor` class API documentation to learn about possible methods of bootstrapping the UI.
This is probably due to lack of docs or just that other
The confusing part might be with how to set data for editing in the
DecoupledEditor.create()
method as it kinda works differently then with other editors.Th consfusing part was that
editableContainer
cannot be used to init data to edit (despite the fact that there's a param for feeding the editor with data).Also the confusing part is that if there's anything in the
editableContainer
it will be around editable area but will not be editable.Now the confusion probably came from that other editors used some container as both editable container and to feed data to the editor.
The text was updated successfully, but these errors were encountered: