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

t/225: Made the UI component initialisation and destruction processes synchronous #264

Merged
merged 15 commits into from
Jul 5, 2017
Merged
6 changes: 2 additions & 4 deletions src/button/buttonview.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,6 @@ export default class ButtonView extends View {
* @inheritDoc
*/
init() {
let promise = Promise.resolve();

if ( this.icon && !this.iconView ) {
const iconView = this.iconView = new IconView();

Expand All @@ -217,10 +215,10 @@ export default class ButtonView extends View {
this.element.insertBefore( iconView.element, this.element.firstChild );

// Make sure the icon view will be destroyed along with button.
promise = promise.then( () => this.addChildren( iconView ) );
this.addChildren( iconView );
}

return promise.then( () => super.init() );
super.init();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/dropdown/dropdownview.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export default class DropdownView extends View {
this.keystrokes.set( 'arrowleft', closeDropdown );
this.keystrokes.set( 'esc', closeDropdown );

return super.init();
super.init();
}

/**
Expand Down
6 changes: 2 additions & 4 deletions src/editableui/editableuiview.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ export default class EditableUIView extends View {
/**
* Initializes the view by either applying the {@link #template} to the existing
* {@link #editableElement} or assigning {@link #element} as {@link #editableElement}.
*
* @returns {Promise}
*/
init() {
if ( this.externalElement ) {
Expand All @@ -88,7 +86,7 @@ export default class EditableUIView extends View {
this.editableElement = this.element;
}

return super.init();
super.init();
}

/**
Expand All @@ -99,6 +97,6 @@ export default class EditableUIView extends View {
this.template.revert( this.externalElement );
}

return super.destroy();
super.destroy();
}
}
5 changes: 2 additions & 3 deletions src/editorui/editoruiview.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,8 @@ export default class EditorUIView extends View {
* @inheritDoc
*/
init() {
return Promise.resolve()
.then( () => this._renderBodyCollection() )
.then( () => super.init() );
this._renderBodyCollection();
super.init();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/list/listview.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export default class ListView extends View {
// Start listening for the keystrokes coming from #element.
this.keystrokes.listenTo( this.element );

return super.init();
super.init();
}

/**
Expand Down
19 changes: 6 additions & 13 deletions src/panel/balloon/contextualballoon.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export default class ContextualBalloon extends Plugin {
this.editor.ui.focusTracker.add( this.view.element );

// Add balloon panel view to editor `body` collection and wait until view will be ready.
return this.editor.ui.view.body.add( this.view );
this.editor.ui.view.body.add( this.view );
}

/**
Expand Down Expand Up @@ -92,7 +92,6 @@ export default class ContextualBalloon extends Plugin {
* @param {module:ui/view~View} [data.view] Content of the balloon.
* @param {module:utils/dom/position~Options} [data.position] Positioning options.
* @param {String} [data.balloonClassName] Additional css class for {@link #view} added when given view is visible.
* @returns {Promise} A Promise resolved when the child {@link module:ui/view~View#init} is done.
*/
add( data ) {
if ( this.hasView( data.view ) ) {
Expand All @@ -112,8 +111,9 @@ export default class ContextualBalloon extends Plugin {

// Add new view to the stack.
this._stack.set( data.view, data );

// And display it.
return this._show( data );
this._show( data );
}

/**
Expand All @@ -122,7 +122,6 @@ export default class ContextualBalloon extends Plugin {
* When there is no view in the stack then balloon will hide.
*
* @param {module:ui/view~View} view A view to be removed from the balloon.
* @returns {Promise} A Promise resolved when the preceding view is ready.
*/
remove( view ) {
if ( !this.hasView( view ) ) {
Expand All @@ -134,9 +133,6 @@ export default class ContextualBalloon extends Plugin {
throw new CKEditorError( 'contextualballoon-remove-view-not-exist: Cannot remove configuration of not existing view.' );
}

// A Promise resolved when the preceding view is ready.
let promise = Promise.resolve();

// When visible view is being removed.
if ( this.visibleView === view ) {
// We need to remove it from the view content.
Expand All @@ -151,7 +147,7 @@ export default class ContextualBalloon extends Plugin {
// If it is some other view.
if ( last ) {
// Just show it.
promise = this._show( last );
this._show( last );
} else {
// Hide the balloon panel.
this.view.hide();
Expand All @@ -160,8 +156,6 @@ export default class ContextualBalloon extends Plugin {
// Just remove given view from the stack.
this._stack.delete( view );
}

return promise;
}

/**
Expand Down Expand Up @@ -190,9 +184,8 @@ export default class ContextualBalloon extends Plugin {
_show( { view, balloonClassName = '' } ) {
this.view.className = balloonClassName;

return this.view.content.add( view ).then( () => {
this.view.pin( this._getBalloonPosition() );
} );
this.view.content.add( view );
this.view.pin( this._getBalloonPosition() );
}

/**
Expand Down
51 changes: 21 additions & 30 deletions src/toolbar/contextual/contextualtoolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export default class ContextualToolbar extends Plugin {
const config = this.editor.config.get( 'contextualToolbar' );
const factory = this.editor.ui.componentFactory;

return this.toolbarView.fillFromConfig( config, factory );
this.toolbarView.fillFromConfig( config, factory );
}

/**
Expand Down Expand Up @@ -142,55 +142,46 @@ export default class ContextualToolbar extends Plugin {
* Fires {@link #event:beforeShow} event just before displaying the panel.
*
* @protected
* @return {Promise} A promise resolved when the {@link #toolbarView} {@link module:ui/view~View#init} is done.
*/
_showPanel() {
const editingView = this.editor.editing.view;
let isStopped = false;

// Do not add toolbar to the balloon stack twice.
if ( this._balloon.hasView( this.toolbarView ) ) {
return Promise.resolve();
return;
}

// This implementation assumes that only non–collapsed selections gets the contextual toolbar.
if ( !editingView.isFocused || editingView.selection.isCollapsed ) {
return Promise.resolve();
return;
}

const showPromise = new Promise( resolve => {
// If `beforeShow` event is not stopped by any external code then panel will be displayed.
this.once( 'beforeShow', () => {
if ( isStopped ) {
resolve();

return;
}

// Update panel position when selection changes while balloon will be opened
// (by an external document changes).
this.listenTo( editingView, 'render', () => {
this._balloon.updatePosition( this._getBalloonPositionData() );
} );

resolve(
// Add panel to the common editor contextual balloon.
this._balloon.add( {
view: this.toolbarView,
position: this._getBalloonPositionData(),
balloonClassName: 'ck-toolbar-container ck-editor-toolbar-container'
} )
);
// If `beforeShow` event is not stopped by any external code then panel will be displayed.
this.once( 'beforeShow', () => {
if ( isStopped ) {
Copy link
Member

Choose a reason for hiding this comment

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

AFAIR, this could've been done using event's cancelling mechanism. Maybe we can change it now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

OK, so let's not do that now.

return;
}

// Update panel position when selection changes while balloon will be opened
// (by an external document changes).
this.listenTo( editingView, 'render', () => {
this._balloon.updatePosition( this._getBalloonPositionData() );
} );

// Add panel to the common editor contextual balloon.
this._balloon.add( {
view: this.toolbarView,
position: this._getBalloonPositionData(),
balloonClassName: 'ck-toolbar-container ck-editor-toolbar-container'
} );
}, { priority: 'lowest' } );
} );

// Fire this event to inform that `ContextualToolbar` is going to be shown.
// Helper function for preventing the panel from being displayed is passed along with the event.
this.fire( 'beforeShow', () => {
isStopped = true;
} );

return showPromise;
}

/**
Expand Down
5 changes: 2 additions & 3 deletions src/toolbar/sticky/stickytoolbarview.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,8 @@ export default class StickyToolbarView extends ToolbarView {
* Destroys the toolbar and removes the {@link #_elementPlaceholder}.
*/
destroy() {
return super.destroy().then( () => {
this._elementPlaceholder.remove();
} );
super.destroy();
this._elementPlaceholder.remove();
}

/**
Expand Down
11 changes: 5 additions & 6 deletions src/toolbar/toolbarview.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export default class ToolbarView extends View {
// Start listening for the keystrokes coming from #element.
this.keystrokes.listenTo( this.element );

return super.init();
super.init();
}

/**
Expand All @@ -119,18 +119,17 @@ export default class ToolbarView extends View {
*
* @param {Array} config The toolbar config.
* @param {module:ui/componentfactory~ComponentFactory} factory A factory producing toolbar items.
* @returns {Promise} A promise resolved when created toolbar items are initialized.
*/
fillFromConfig( config, factory ) {
if ( !config ) {
return Promise.resolve();
return;
}

return Promise.all( config.map( name => {
config.map( name => {
const component = name == '|' ? new ToolbarSeparatorView() : factory.create( name );

return this.items.add( component );
} ) );
this.items.add( component );
} );
}
}

53 changes: 22 additions & 31 deletions src/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ import isIterable from '@ckeditor/ckeditor5-utils/src/isiterable';
*
* const view = new SampleView( locale );
*
* view.init().then( () => {
* // Will append <p class="foo">Hello<b>world</b></p>
* document.body.appendChild( view.element );
* } );
* view.init();
*
* // Will append <p class="foo">Hello<b>world</b></p>
* document.body.appendChild( view.element );
*
* @mixes module:utils/observablemixin~ObservableMixin
*/
Expand Down Expand Up @@ -190,14 +190,14 @@ export default class View {
* const view = new SampleView( locale );
* const anotherView = new AnotherSampleView( locale );
*
* view.init().then( () => {
* // Will append <p></p>
* document.body.appendChild( view.element );
* view.init();
*
* // Will append <p></p>
* document.body.appendChild( view.element );
*
* // `anotherView` becomes a child of the view, which is reflected in DOM
* // <p><anotherView#element></p>
* view.items.add( anotherView );
* } );
* // `anotherView` becomes a child of the view, which is reflected in DOM
* // <p><anotherView#element></p>
* view.items.add( anotherView );
*
* @returns {module:ui/viewcollection~ViewCollection} A new collection of view instances.
*/
Expand Down Expand Up @@ -237,10 +237,10 @@ export default class View {
*
* const view = new SampleView( locale );
*
* view.init().then( () => {
* // Will append <p><childA#element><b></b><childB#element></p>
* document.body.appendChild( view.element );
* } );
* view.init();
*
* // Will append <p><childA#element><b></b><childB#element></p>
* document.body.appendChild( view.element );
*
* **Note**: There's no need to add child views if they're used in the
* {@link #template} explicitly:
Expand All @@ -265,20 +265,17 @@ export default class View {
* }
*
* @param {module:ui/view~View|Iterable.<module:ui/view~View>} children Children views to be registered.
* @returns {Promise}
*/
addChildren( children ) {
if ( !isIterable( children ) ) {
children = [ children ];
}

return Promise.all( children.map( c => this._unboundChildren.add( c ) ) );
children.map( c => this._unboundChildren.add( c ) );
}

/**
* Initializes the view and child views located in {@link #_viewCollections}.
*
* @returns {Promise} A Promise resolved when the initialization process is finished.
*/
init() {
if ( this.ready ) {
Expand All @@ -290,26 +287,20 @@ export default class View {
throw new CKEditorError( 'ui-view-init-reinit: This View has already been initialized.' );
}

return Promise.resolve()
// Initialize collections in #_viewCollections.
.then( () => {
return Promise.all( this._viewCollections.map( c => c.init() ) );
} )
// Spread the word that this view is ready!
.then( () => {
this.ready = true;
} );
// Initialize collections in #_viewCollections.
this._viewCollections.map( c => c.init() );

// Spread the word that this view is ready!
this.ready = true;
}

/**
* Destroys the view instance and child views located in {@link #_viewCollections}.
*
* @returns {Promise} A Promise resolved when the destruction process is finished.
*/
destroy() {
this.stopListening();

return Promise.all( this._viewCollections.map( c => c.destroy() ) );
this._viewCollections.map( c => c.destroy() );
}

/**
Expand Down
Loading