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

Commit

Permalink
Merge pull request #41 from alexeckermann/t/ckeditor5/1150
Browse files Browse the repository at this point in the history
Fix: Child views should be added in `InlineEditorUIView#render()` instead of `#constructor()` to allow early template manipulation. Closes ckeditor/ckeditor5#1150.

Huge thanks to [Alex Eckermann](https://github.com/alexeckermann) for this contribution!
  • Loading branch information
oleq authored Jul 26, 2018
2 parents 470308f + 9b2ce26 commit b0be713
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 23 deletions.
5 changes: 2 additions & 3 deletions src/inlineeditoruiview.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,6 @@ export default class InlineEditorUIView extends EditorUIView {
* @member {module:ui/editableui/inline/inlineeditableuiview~InlineEditableUIView}
*/
this.editable = new InlineEditableUIView( locale, editableElement );

this.body.add( this.panel );
this.registerChild( this.editable );
}

/**
Expand All @@ -141,6 +138,8 @@ export default class InlineEditorUIView extends EditorUIView {
render() {
super.render();

this.body.add( this.panel );
this.registerChild( this.editable );
this.panel.content.add( this.toolbar );
}

Expand Down
64 changes: 44 additions & 20 deletions tests/inlineeditoruiview.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ describe( 'InlineEditorUIView', () => {
beforeEach( () => {
locale = new Locale( 'en' );
view = new InlineEditorUIView( locale );
view.render();
} );

afterEach( () => {
view.destroy();
} );

describe( 'constructor()', () => {
Expand All @@ -32,12 +27,8 @@ describe( 'InlineEditorUIView', () => {
expect( view.toolbar.locale ).to.equal( locale );
} );

it( 'is given the right CSS classes', () => {
expect( view.toolbar.element.classList.contains( 'ck-toolbar_floating' ) ).to.be.true;
} );

it( 'sets the default value of the #viewportTopOffset attribute', () => {
expect( view.viewportTopOffset ).to.equal( 0 );
it( 'is not rendered', () => {
expect( view.toolbar.isRendered ).to.be.false;
} );
} );

Expand All @@ -50,28 +41,60 @@ describe( 'InlineEditorUIView', () => {
expect( view.panel.locale ).to.equal( locale );
} );

it( 'is given the right CSS class', () => {
expect( view.panel.element.classList.contains( 'ck-toolbar-container' ) ).to.be.true;
} );

it( 'is put into the #body collection', () => {
expect( view.body.get( 0 ) ).to.equal( view.panel );
} );

it( 'gets view.panel#withArrow set', () => {
expect( view.panel.withArrow ).to.be.false;
} );

it( 'is not rendered', () => {
expect( view.panel.isRendered ).to.be.false;
} );
} );

describe( '#editable', () => {
it( 'is created', () => {
expect( view.editable ).to.be.instanceof( InlineEditableUIView );
} );

it( 'is given a locate object', () => {
it( 'is given a locale object', () => {
expect( view.editable.locale ).to.equal( locale );
} );

it( 'is not rendered', () => {
expect( view.editable.isRendered ).to.be.false;
} );
} );
} );

describe( 'render()', () => {
beforeEach( () => {
view.render();
} );

afterEach( () => {
view.destroy();
} );

describe( '#toolbar', () => {
it( 'is given the right CSS classes', () => {
expect( view.toolbar.element.classList.contains( 'ck-toolbar_floating' ) ).to.be.true;
} );

it( 'sets the default value of the #viewportTopOffset attribute', () => {
expect( view.viewportTopOffset ).to.equal( 0 );
} );
} );

describe( '#panel', () => {
it( 'is given the right CSS class', () => {
expect( view.panel.element.classList.contains( 'ck-toolbar-container' ) ).to.be.true;
} );

it( 'is put into the #body collection', () => {
expect( view.body.get( 0 ) ).to.equal( view.panel );
} );
} );

describe( '#editable', () => {
it( 'is registered as a child', () => {
const spy = sinon.spy( view.editable, 'destroy' );

Expand All @@ -96,6 +119,7 @@ describe( 'InlineEditorUIView', () => {

describe( 'editableElement', () => {
it( 'returns editable\'s view element', () => {
view.render();
expect( view.editableElement.getAttribute( 'contentEditable' ) ).to.equal( 'true' );
view.destroy();
} );
Expand Down

0 comments on commit b0be713

Please sign in to comment.