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 #264 from ckeditor/t/225
Browse files Browse the repository at this point in the history
Other: Made the UI component initialization and destruction processes synchronous. Closes #225.

BREAKING CHANGE: `View#init()`, `View#destroy()` (also `ViewCollection#init()`, `ViewCollection#destroy()` and `ViewCollection#add()`) no longer return `Promise`. It may require updates in UI components which inherit from `View` and rely on the value returned by these methods.

BREAKING CHANGE: Various UI components switched to synchronous `init()` and `destroy()` (no longer returning `Promise`), which means that features using these components may need some updates to work properly.
  • Loading branch information
Reinmar committed Jul 5, 2017
2 parents 892e2da + d938f52 commit 07e1502
Show file tree
Hide file tree
Showing 31 changed files with 343 additions and 796 deletions.
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 ) {
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

0 comments on commit 07e1502

Please sign in to comment.