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

Allow to pass initial data to the editor constructor #38

Merged
merged 20 commits into from
Jul 3, 2018
Merged

Conversation

szymonkups
Copy link
Contributor

@szymonkups szymonkups commented May 15, 2018

Suggested merge commit message (convention)

Feature: Editor can be created with initial data passed to the constructor. Closes ckeditor/ckeditor5#2271.

BREAKING CHANGE: InlineEditor#element is now available as InlineEditor#sourceElement. See ckeditor/ckeditor5#2882.

Requires: ckeditor/ckeditor5-core#129

@szymonkups szymonkups requested a review from Reinmar May 15, 2018 14:48
@coveralls
Copy link

coveralls commented May 15, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 3f27c04 on t/37 into c148346 on master.

* import ...
*
* InlineEditor
* .create( '<p>Hello world!</p>, {
Copy link
Member

Choose a reason for hiding this comment

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

Missing '

@Reinmar
Copy link
Member

Reinmar commented May 17, 2018

I miss a slightly better explanation what you can find in editor.element. Plus, also updating the description of this property.

Apart from that, all LGTM.

Copy link
Member

@pomek pomek left a comment

Choose a reason for hiding this comment

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

I improved a little the manual test. Except that, everything looks and works fine.

@Reinmar
Copy link
Member

Reinmar commented Jun 6, 2018

…hUI interface (added editor.element property).
@@ -86,7 +97,7 @@ export default class InlineEditor extends Editor {
this.ui.destroy();

return super.destroy()
.then( () => setDataInElement( this.element, data ) );
.then( () => setDataInElement( this.sourceElement, data ) );
Copy link
Member

Choose a reason for hiding this comment

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

What if there's no source element?

Copy link
Member

Choose a reason for hiding this comment

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

Throws an ugly error. I'm on it.

@Reinmar Reinmar merged commit cfd8c53 into master Jul 3, 2018
@Reinmar Reinmar deleted the t/37 branch July 3, 2018 07: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.

Allow to set data in constructor instead of element
4 participants