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.

Sign up for GitHub

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

Jump to bottom

T/47 #48

Merged
merged 7 commits into from
Apr 19, 2019
Merged

T/47 #48

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a422e09
Change the way of data initialize.
elszczepano Apr 18, 2019
9b557ca
Remove unnecessary variable.
elszczepano Apr 18, 2019
1ce9387
Restore and fix test for check initial data. Check if setData() wasn'…
elszczepano Apr 18, 2019
49b8f6d
Fix code style
elszczepano Apr 19, 2019
e507980
Add link to the related issue
elszczepano Apr 19, 2019
e1ee311
Merge branch 'master' into t/47
oleq Apr 19, 2019
9c9a6ec
Code refactoring and code style.
oleq Apr 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/ckeditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ export default {
name: 'ckeditor',

render( createElement ) {
return createElement( this.tagName );
return createElement(this.tagName, {
pjasiun marked this conversation as resolved.
Show resolved Hide resolved
domProps: {
innerHTML: this.value || ''
Copy link
Member

Choose a reason for hiding this comment

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

Does this work when tagName is textarea?

Copy link
Member

Choose a reason for hiding this comment

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

I'm worried because setting the value of an input like this works only once (the second one will be ignored). In theory, we do it only once but still, this is not the right way IMO.

Copy link
Member

Choose a reason for hiding this comment

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

Also... does this whole innerHTML thing work when a content with/without entities is passed as a value?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've built a sample with tagName set to textarea. The editor seems to work properly, no error occurs on init and on destroy.

}
});
},

props: {
Expand Down Expand Up @@ -49,9 +53,6 @@ export default {
// Save the reference to the instance for further use.
this.instance = editor;

// Set the initial data of the editor.
editor.setData( this.value );

// Set initial disabled state.
editor.isReadOnly = this.disabled;

Expand Down
2 changes: 2 additions & 0 deletions tests/_utils/mockeditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export default class MockEditor {
this.element = el;
this.config = config;
this.data = '';
this.setDataCounter = 0;

this.model = {
document: new ModelDocument()
Expand All @@ -39,6 +40,7 @@ export default class MockEditor {
}

setData( data ) {
this.setDataCounter++;
this.data = data;
}

Expand Down
9 changes: 5 additions & 4 deletions tests/ckeditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,15 @@ describe( 'CKEditor Component', () => {
expect( vm.value ).to.equal( '' );
} );

it( 'should set the initial data', done => {
Copy link
Member

Choose a reason for hiding this comment

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

This test should not be removed. We need to be sure that the specified data has been passed to the editor.

const setDataStub = sandbox.stub( MockEditor.prototype, 'setData' );
const { wrapper } = createComponent( {
it( 'should set the initial data by using innerHTML, not by "setData()"', done => {
Copy link
Member

Choose a reason for hiding this comment

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

Missing link to the issue.

const { wrapper, vm } = createComponent( {
value: 'foo'
} );

Vue.nextTick( () => {
sinon.assert.calledWithExactly( setDataStub, 'foo' );

expect( vm.$el.innerHTML ).to.equal( 'foo' );
expect( vm.instance.setDataCounter).to.equal(0);
pjasiun marked this conversation as resolved.
Show resolved Hide resolved
pjasiun marked this conversation as resolved.
Show resolved Hide resolved

wrapper.destroy();
done();
Expand Down