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

T/248: Made UI destruction a repeatable process #254

Merged
merged 4 commits into from
Jun 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 0 additions & 10 deletions src/panel/balloon/contextualballoon.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,14 +205,4 @@ export default class ContextualBalloon extends Plugin {
_getBalloonPosition() {
return this._stack.values().next().value.position;
}

/**
* @inheritDoc
*/
destroy() {
this.editor.ui.view.body.remove( this.view );
this.view.destroy();
this._stack.clear();
super.destroy();
}
}
9 changes: 1 addition & 8 deletions src/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,14 +309,7 @@ export default class View {
destroy() {
this.stopListening();

return Promise.all( this._viewCollections.map( c => c.destroy() ) )
.then( () => {
this._unboundChildren.clear();
this._viewCollections.clear();

this.element = this.template = this.locale = this.t =
this._viewCollections = this._unboundChildren = null;
} );
return Promise.all( this._viewCollections.map( c => c.destroy() ) );
}

/**
Expand Down
10 changes: 10 additions & 0 deletions tests/editableui/editableuiview.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ describe( 'EditableUIView', () => {
} );
} );

it( 'can be called multiple times', done => {
expect( () => {
view.destroy().then( () => {
return view.destroy().then( () => {
done();
} );
} );
} ).to.not.throw();
} );

describe( 'when #editableElement as an argument', () => {
it( 'reverts contentEditable property of editableElement (was false)', () => {
editableElement = document.createElement( 'div' );
Expand Down
10 changes: 10 additions & 0 deletions tests/editorui/editoruiview.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,15 @@ describe( 'EditorUIView', () => {
expect( el.parentNode ).to.be.null;
} );
} );

it( 'can be called multiple times', done => {
expect( () => {
view.destroy().then( () => {
return view.destroy().then( () => {
done();
} );
} );
} ).to.not.throw();
} );
} );
} );
12 changes: 9 additions & 3 deletions tests/panel/balloon/contextualballoon.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,17 @@ describe( 'ContextualBalloon', () => {
} );

describe( 'destroy()', () => {
it( 'should remove balloon panel view from editor body collection and clear stack', () => {
it( 'can be called multiple times', () => {
expect( () => {
balloon.destroy();
balloon.destroy();
} ).to.not.throw();
} );

it( 'should not touch the DOM', () => {
balloon.destroy();

expect( editor.ui.view.body.getIndex( balloon.view ) ).to.equal( -1 );
expect( balloon.visibleView ).to.null;
expect( editor.ui.view.body.getIndex( balloon.view ) ).to.not.equal( -1 );
} );
} );
} );
7 changes: 7 additions & 0 deletions tests/toolbar/contextual/contextualtoolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,13 @@ describe( 'ContextualToolbar', () => {
} );

describe( 'destroy()', () => {
it( 'can be called multiple times', () => {
expect( () => {
contextualToolbar.destroy();
contextualToolbar.destroy();
} ).to.not.throw();
} );

it( 'should not fire `_selectionChangeDebounced` after plugin destroy', done => {
const spy = sandbox.spy();

Expand Down
10 changes: 10 additions & 0 deletions tests/toolbar/sticky/stickytoolbarview.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,16 @@ describe( 'StickyToolbarView', () => {
expect( view.destroy() ).to.be.instanceof( Promise );
} );

it( 'can be called multiple times', done => {
expect( () => {
view.destroy().then( () => {
return view.destroy().then( () => {
done();
} );
} );
} ).to.not.throw();
} );

it( 'calls destroy on parent class', () => {
const spy = testUtils.sinon.spy( ToolbarView.prototype, 'destroy' );

Expand Down
48 changes: 34 additions & 14 deletions tests/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,41 +262,55 @@ describe( 'View', () => {
beforeEach( createViewWithChildren );

it( 'should return a promise', () => {
expect( view.destroy() ).to.be.instanceof( Promise );
const promise = view.destroy();

expect( promise ).to.be.instanceof( Promise );

return promise;
} );

it( 'can be called multiple times', done => {
expect( () => {
view.destroy().then( () => {
return view.destroy().then( () => {
done();
} );
} );
} ).to.not.throw();
} );

it( 'should set basic properties null', () => {
it( 'should not touch the basic properties', () => {
return view.destroy().then( () => {
expect( view.element ).to.be.null;
expect( view.template ).to.be.null;
expect( view.locale ).to.be.null;
expect( view.t ).to.be.null;
expect( view.element ).to.be.an.instanceof( HTMLElement );
expect( view.template ).to.be.an.instanceof( Template );
expect( view.locale ).to.be.an( 'object' );
expect( view.locale.t ).to.be.a( 'function' );

expect( view._unboundChildren ).to.be.null;
expect( view._viewCollections ).to.be.null;
expect( view._viewCollections ).to.be.instanceOf( Collection );
expect( view._unboundChildren ).to.be.instanceOf( ViewCollection );
} );
} );

it( 'clears #_unboundChildren', () => {
it( 'should not clear the #_unboundChildren', () => {
const cached = view._unboundChildren;

return view.addChildren( [ new View(), new View() ] )
.then( () => {
expect( cached ).to.have.length.above( 2 );
expect( cached ).to.have.length( 4 );

return view.destroy().then( () => {
expect( cached ).to.have.length( 0 );
expect( cached ).to.have.length( 4 );
} );
} );
} );

it( 'clears #_viewCollections', () => {
it( 'should not clear the #_viewCollections', () => {
const cached = view._viewCollections;

expect( cached ).to.have.length( 1 );

return view.destroy().then( () => {
expect( cached ).to.have.length( 0 );
expect( cached ).to.have.length( 1 );
} );
} );

Expand Down Expand Up @@ -326,11 +340,15 @@ describe( 'View', () => {
} );

it( 'destroy a template–less view', () => {
let promise;

view = new View();

expect( () => {
view.destroy();
promise = view.destroy();
} ).to.not.throw();

return promise;
} );

// https://github.com/ckeditor/ckeditor5-ui/issues/203
Expand Down Expand Up @@ -383,6 +401,8 @@ function setTestViewClass( templateDef ) {
constructor() {
super();

this.locale = { t() {} };

if ( templateDef ) {
this.template = new Template( templateDef );
}
Expand Down