8
0
فهرست منبع

Flattened UI architecture by moving Controller logic into View class.

Aleksander Nowodzinski 9 سال پیش
والد
کامیت
4a2fba7852
2فایلهای تغییر یافته به همراه288 افزوده شده و 291 حذف شده
  1. 97 131
      packages/ckeditor5-ui/src/view.js
  2. 191 160
      packages/ckeditor5-ui/tests/view.js

+ 97 - 131
packages/ckeditor5-ui/src/view.js

@@ -3,12 +3,12 @@
  * For licensing, see LICENSE.md.
  */
 
-import Collection from '../utils/collection.js';
-import Region from './region.js';
-import Template from './template.js';
 import CKEditorError from '../utils/ckeditorerror.js';
+import ViewCollection from './viewcollection.js';
+import Template from './template.js';
 import DOMEmitterMixin from './domemittermixin.js';
 import ObservableMixin from '../utils/observablemixin.js';
+import Collection from '../utils/collection.js';
 import mix from '../utils/mix.js';
 
 /**
@@ -22,10 +22,14 @@ export default class View {
 	/**
 	 * Creates an instance of the {@link ui.View} class.
 	 *
+	 * TODO: A simple example how to create one.
+	 *
 	 * @param {utils.Locale} [locale] The {@link core.editor.Editor#locale editor's locale} instance.
 	 */
 	constructor( locale ) {
 		/**
+		 * A set of tools to localize the user interface. See {@link core.editor.Editor#locale}.
+		 *
 		 * @readonly
 		 * @member {utils.Locale} ui.View#locale
 		 */
@@ -34,7 +38,7 @@ export default class View {
 		/**
 		 * Shorthand for {@link utils.Locale#t}.
 		 *
-		 * Note: If locale instance hasn't been passed to the view this method may not be available.
+		 * Note: If locale instance hasn't been 	passed to the view this method may not be available.
 		 *
 		 * @see utils.Locale#t
 		 * @method ui.View#t
@@ -42,27 +46,48 @@ export default class View {
 		this.t = locale && locale.t;
 
 		/**
-		 * Regions of this view. See {@link ui.View#register}.
+		 * Set `true` after {@link ui.View#init}, which can be asynchronous.
 		 *
-		 * @member {utils.Collection} ui.View#regions
+		 * @readonly
+		 * @observable
+		 * @member {Boolean} ui.View#ready
 		 */
-		this.regions = new Collection( {
-			idProperty: 'name'
+		this.set( 'ready', false );
+
+		/**
+		 * Collections registered with {@link ui.View#createCollection}.
+		 *
+		 * @protected
+		 * @member {Set.<ui.ViewCollection>} ui.view#_viewCollections
+		 */
+		this._viewCollections = new Collection();
+
+		// Let the new collection determine the {@link ui.View#ready} state of this view and,
+		// accordingly, initialize (or not) children views as they are added in the future.
+		this._viewCollections.on( 'add', ( evt, collection ) => {
+			collection.bind( 'ready' ).to( this );
+		} );
+
+		// Once the collection is removed from the view, the {@link ui.View#ready} binding
+		// should be removed.
+		this._viewCollections.on( 'remove', ( evt, collection ) => {
+			collection.unbind( 'ready' );
 		} );
 
 		/**
-		 * Template of this view.
+		 * A collection of view instances, which have been added directly
+		 * into the {@link ui.View.template#children}.
 		 *
-		 * @member {ui.Template} ui.View#template
+		 * @protected
+		 * @member {ui.ViewCollection} ui.view#_unboundChildren
 		 */
+		this._unboundChildren = this.createCollection();
 
 		/**
-		 * Region selectors of this view. See {@link ui.View#register}.
+		 * Template of this view.
 		 *
-		 * @private
-		 * @member {Object} ui.View#_regionSelectors
+		 * @member {ui.Template} ui.View#template
 		 */
-		this._regionSelectors = {};
 
 		/**
 		 * Element of this view.
@@ -122,156 +147,97 @@ export default class View {
 	}
 
 	/**
-	 * Initializes the view.
+	 * Creates a new collection of views, which can be used in this view instance
+	 * i.e. as a member of {@link ui.TemplateDefinition#children}.
 	 *
-	 * Note: {@link ui.Controller} supports if a promise is returned by this method,
-	 * what means that view initialization may be asynchronous.
+	 * TODO: An example how to use created collection in a template definition.
+	 *
+	 * @returns {ui.ViewCollection} A new collection of view instances.
 	 */
-	init() {
-		this._initRegions();
+	createCollection() {
+		const collection = new ViewCollection();
+
+		this._viewCollections.add( collection );
+
+		return collection;
 	}
 
 	/**
-	 * Registers a region in {@link ui.View#regions}.
-	 *
-	 *		let view = new View();
-	 *
-	 *		// region.name == "foo", region.element == view.element.firstChild
-	 *		view.register( 'foo', el => el.firstChild );
-	 *
-	 *		// region.name == "bar", region.element == view.element.querySelector( 'span' )
-	 *		view.register( new Region( 'bar' ), 'span' );
+	 * Registers a new child view under this view instance. Once registered, a child
+	 * view is managed by its parent, including initialization ({@link ui.view#init})
+	 * and destruction ({@link ui.view#destroy}).
 	 *
-	 *		// region.name == "bar", region.element == view.element.querySelector( '#div#id' )
-	 *		view.register( 'bar', 'div#id', true );
-	 *
-	 *		// region.name == "baz", region.element == null
-	 *		view.register( 'baz', true );
-	 *
-	 * @param {String|Region} stringOrRegion The name or an instance of the Region
-	 * to be registered. If `String`, the region will be created on the fly.
-	 * @param {String|Function|true} regionSelector The selector to retrieve region's element
-	 * in DOM when the region instance is initialized (see {@link Region#init}, {@link ui.View#init}).
-	 * @param {Boolean} [override] When set `true` it will allow overriding of registered regions.
+	 * @param {...ui.View} children Children views to be registered.
 	 */
-	register( ...args ) {
-		let region, regionName;
-
-		if ( typeof args[ 0 ] === 'string' ) {
-			regionName = args[ 0 ];
-			region = this.regions.get( regionName ) || new Region( regionName );
-		} else if ( args[ 0 ] instanceof Region ) {
-			regionName = args[ 0 ].name;
-			region = args[ 0 ];
-		} else {
-			/**
-			 * A name of the region or an instance of Region is required.
-			 *
-			 * @error ui-view-register-wrongtype
-			 */
-			throw new CKEditorError( 'ui-view-register-wrongtype' );
+	addChild( ...children ) {
+		for ( let child of children ) {
+			this._unboundChildren.add( child );
 		}
+	}
 
-		const regionSelector = args[ 1 ];
-
-		if ( !regionSelector || !isValidRegionSelector( regionSelector ) ) {
+	/**
+	 * Initializes the view and child views located in {@link ui.View#_viewCollections}.
+	 *
+	 * @returns {Promise} A Promise resolved when the initialization process is finished.
+	 */
+	init() {
+		if ( this.ready ) {
 			/**
-			 * The selector must be String, Function or `true`.
+			 * This View has already been initialized.
 			 *
-			 * @error ui-view-register-badselector
+			 * @error ui-view-init-reinit
 			 */
-			throw new CKEditorError( 'ui-view-register-badselector' );
+			throw new CKEditorError( 'ui-view-init-reinit: This View has already been initialized.' );
 		}
 
-		const registered = this.regions.get( regionName );
-
-		if ( !registered ) {
-			this.regions.add( region );
-		} else {
-			if ( registered !== region ) {
-				if ( !args[ 2 ] ) {
-					/**
-					 * Overriding is possible only when `override` flag is set.
-					 *
-					 * @error ui-view-register-override
-					 */
-					throw new CKEditorError( 'ui-view-register-override' );
-				}
+		return Promise.resolve()
+			// Initialize child views in #_viewCollections.
+			.then( () => {
+				const promises = [];
 
-				this.regions.remove( registered );
-				this.regions.add( region );
-			}
-		}
+				for ( let collection of this._viewCollections ) {
+					for ( let view of collection ) {
+						promises.push( view.init() );
+					}
+				}
 
-		this._regionSelectors[ regionName ] = regionSelector;
+				return Promise.all( promises );
+			} )
+			// Spread the word that this view is ready!
+			.then( () => {
+				this.ready = true;
+			} );
 	}
 
 	/**
-	 * Destroys the view instance. The process includes:
+	 * Destroys the view instance and child views located in {@link ui.View#_viewCollections}.
 	 *
-	 * 1. Removal of child views from {@link ui.View#regions}.
-	 * 2. Destruction of the {@link ui.View#regions}.
-	 * 3. Removal of {@link #_el} from DOM.
+	 * @returns {Promise} A Promise resolved when the destruction process is finished.
 	 */
 	destroy() {
-		let childView;
-
 		this.stopListening();
 
-		for ( let region of this.regions ) {
-			while ( ( childView = region.views.get( 0 ) ) ) {
-				region.views.remove( childView );
-			}
+		let promises = [];
 
-			this.regions.remove( region ).destroy();
+		for ( let collection of this._viewCollections ) {
+			for ( let view of collection ) {
+				promises.push( view.destroy() );
+			}
 		}
 
-		if ( this.template ) {
+		this._unboundChildren.clear();
+		this._viewCollections.clear();
+
+		if ( this.element ) {
 			this.element.remove();
 		}
 
-		this.model = this.regions = this.template = this.locale = this.t = null;
-		this._regionSelectors = this._element = null;
-	}
-
-	/**
-	 * Initializes {@link ui.View#regions} of this view by passing a DOM element
-	 * generated from {@link ui.View#_regionSelectors} into {@link Region#init}.
-	 *
-	 * @protected
-	 */
-	_initRegions() {
-		let region, regionEl, regionSelector;
-
-		for ( region of this.regions ) {
-			regionSelector = this._regionSelectors[ region.name ];
-
-			if ( typeof regionSelector == 'string' ) {
-				regionEl = this.element.querySelector( regionSelector );
-			} else if ( typeof regionSelector == 'function' ) {
-				regionEl = regionSelector( this.element );
-			} else {
-				regionEl = null;
-			}
+		this.element = this.template = this.locale = this.t =
+			this._viewCollections = this._unboundChildren = null;
 
-			region.init( regionEl );
-		}
+		return Promise.all( promises );
 	}
 }
 
 mix( View, DOMEmitterMixin );
 mix( View, ObservableMixin );
-
-const validSelectorTypes = new Set( [ 'string', 'boolean', 'function' ] );
-
-/**
- * Check whether region selector is valid.
- *
- * @ignore
- * @private
- * @param {*} selector Selector to be checked.
- * @returns {Boolean}
- */
-function isValidRegionSelector( selector ) {
-	return validSelectorTypes.has( typeof selector ) && selector !== false;
-}

+ 191 - 160
packages/ckeditor5-ui/tests/view.js

@@ -3,16 +3,17 @@
  * For licensing, see LICENSE.md.
  */
 
-/* global document, HTMLElement */
+/* global document, setTimeout, HTMLElement */
 /* bender-tags: ui */
 
 import testUtils from 'tests/core/_utils/utils.js';
 import View from 'ckeditor5/ui/view.js';
 import Template from 'ckeditor5/ui/template.js';
-import Region from 'ckeditor5/ui/region.js';
 import CKEditorError from 'ckeditor5/utils/ckeditorerror.js';
+import Collection from 'ckeditor5/utils/collection.js';
+import ViewCollection from 'ckeditor5/ui/viewcollection.js';
 
-let TestView, view;
+let TestView, view, childA, childB;
 
 testUtils.createSinonSandbox();
 
@@ -27,7 +28,11 @@ describe( 'View', () => {
 			view = new View();
 
 			expect( view.t ).to.be.undefined;
-			expect( view.regions.length ).to.equal( 0 );
+			expect( view.locale ).to.be.undefined;
+			expect( view.ready ).to.be.false;
+			expect( view.template ).to.be.undefined;
+			expect( view._viewCollections ).to.be.instanceOf( Collection );
+			expect( view._unboundChildren ).to.be.instanceOf( ViewCollection );
 		} );
 
 		it( 'defines the locale property and the "t" function', () => {
@@ -40,78 +45,96 @@ describe( 'View', () => {
 		} );
 	} );
 
-	describe( 'init', () => {
+	describe( 'createCollection', () => {
 		beforeEach( () => {
-			setTestViewClass( {
-				tag: 'p',
-				children: [
-					{ tag: 'span' },
-					{ tag: 'strong' }
-				]
-			} );
-		} );
-
-		it( 'calls child regions #init', () => {
+			setTestViewClass();
 			setTestViewInstance();
+		} );
 
-			const region1 = new Region( 'x' );
-			const region2 = new Region( 'y' );
-
-			view.register( region1, el => el );
-			view.register( region2, el => el );
+		it( 'returns an instance of view collection', () => {
+			expect( view.createCollection() ).to.be.instanceOf( ViewCollection );
+		} );
 
-			const spy1 = testUtils.sinon.spy( region1, 'init' );
-			const spy2 = testUtils.sinon.spy( region2, 'init' );
+		it( 'adds a new collection to the #_viewCollections', () => {
+			expect( view._viewCollections ).to.have.length( 1 );
 
-			view.init();
+			const collection = view.createCollection();
 
-			sinon.assert.calledOnce( spy1 );
-			sinon.assert.calledOnce( spy2 );
+			expect( view._viewCollections ).to.have.length( 2 );
+			expect( view._viewCollections.get( 1 ) ).to.equal( collection );
 		} );
+	} );
 
-		it( 'initializes view regions with string selector', () => {
+	describe( 'addChild', () => {
+		beforeEach( () => {
+			setTestViewClass();
 			setTestViewInstance();
+		} );
 
-			const region1 = new Region( 'x' );
-			const region2 = new Region( 'y' );
-
-			view.register( region1, 'span' );
-			view.register( region2, 'strong' );
+		it( 'should add a view to #_unboundChildren', () => {
+			expect( view._unboundChildren ).to.have.length( 0 );
 
-			view.init();
+			const child = {};
 
-			expect( region1.element ).to.equal( view.element.firstChild );
-			expect( region2.element ).to.equal( view.element.lastChild );
+			view.addChild( child );
+			expect( view._unboundChildren ).to.have.length( 1 );
+			expect( view._unboundChildren.get( 0 ) ).to.equal( child );
 		} );
 
-		it( 'initializes view regions with function selector', () => {
-			setTestViewInstance();
+		it( 'should support multiple ...arguments', () => {
+			expect( view._unboundChildren ).to.have.length( 0 );
 
-			const region1 = new Region( 'x' );
-			const region2 = new Region( 'y' );
+			const children = [ {}, {}, {} ];
 
-			view.register( region1, el => el.firstChild );
-			view.register( region2, el => el.lastChild );
+			view.addChild( ...children );
+			expect( view._unboundChildren ).to.have.length( 3 );
+		} );
+	} );
 
-			view.init();
+	describe( 'init', () => {
+		beforeEach( createViewWithChildren );
+
+		it( 'should throw if already initialized', () => {
+			return view.init()
+				.then( () => {
+					view.init();
+
+					throw new Error( 'This should not be executed.' );
+				} )
+				.catch( ( err ) => {
+					expect( err ).to.be.instanceof( CKEditorError );
+					expect( err.message ).to.match( /ui-view-init-re/ );
+				} );
+		} );
 
-			expect( region1.element ).to.equal( view.element.firstChild );
-			expect( region2.element ).to.equal( view.element.lastChild );
+		it( 'returns a promise', () => {
+			expect( view.init() ).to.be.instanceof( Promise );
 		} );
 
-		it( 'initializes view regions with boolean selector', () => {
-			setTestViewInstance();
+		it( 'should set view#ready', () => {
+			expect( view.ready ).to.be.false;
+
+			return view.init().then( () => {
+				expect( view.ready ).to.be.true;
+			} );
+		} );
 
-			const region1 = new Region( 'x' );
-			const region2 = new Region( 'y' );
+		it( 'calls init() on all views in #_viewCollections', () => {
+			const spy = testUtils.sinon.spy( view, 'init' );
+			const spyA = testUtils.sinon.spy( childA, 'init' );
+			const spyB = testUtils.sinon.spy( childB, 'init' );
 
-			view.register( region1, true );
-			view.register( region2, true );
+			expect( childA.ready ).to.be.false;
+			expect( childB.ready ).to.be.false;
 
-			view.init();
+			return view.init().then( () => {
+				expect( childA.ready ).to.be.true;
+				expect( childB.ready ).to.be.true;
 
-			expect( region1.element ).to.be.null;
-			expect( region2.element ).to.be.null;
+				sinon.assert.calledOnce( spyA );
+				sinon.assert.calledOnce( spyB );
+				sinon.assert.callOrder( spy, spyA, spyB );
+			} );
 		} );
 	} );
 
@@ -132,85 +155,6 @@ describe( 'View', () => {
 		} );
 	} );
 
-	describe( 'register', () => {
-		beforeEach( () => {
-			setTestViewClass();
-			setTestViewInstance();
-		} );
-
-		it( 'should throw when first argument is neither Region instance nor string', () => {
-			expect( () => {
-				view.register( new Date() );
-			} ).to.throw( CKEditorError, /ui-view-register-wrongtype/ );
-		} );
-
-		it( 'should throw when missing the selector argument', () => {
-			expect( () => {
-				view.register( 'x' );
-			} ).to.throw( CKEditorError, /ui-view-register-badselector/ );
-		} );
-
-		it( 'should throw when selector argument is of a wrong type', () => {
-			expect( () => {
-				view.register( 'x', new Date() );
-			} ).to.throw( CKEditorError, /ui-view-register-badselector/ );
-
-			expect( () => {
-				view.register( 'x', false );
-			} ).to.throw( CKEditorError, /ui-view-register-badselector/ );
-		} );
-
-		it( 'should throw when overriding an existing region but without override flag set', () => {
-			expect( () => {
-				view.register( 'x', true );
-				view.register( new Region( 'x' ), true );
-			} ).to.throw( CKEditorError, /ui-view-register-override/ );
-		} );
-
-		it( 'should register a new region with region name as a first argument', () => {
-			view.register( 'x', true );
-
-			expect( view.regions.get( 'x' ) ).to.be.an.instanceof( Region );
-		} );
-
-		it( 'should register a new region with Region instance as a first argument', () => {
-			view.register( new Region( 'y' ), true );
-
-			expect( view.regions.get( 'y' ) ).to.be.an.instanceof( Region );
-		} );
-
-		it( 'should override an existing region with override flag', () => {
-			view.template = new Template( {
-				tag: 'div',
-				children: [
-					{ tag: 'span' }
-				]
-			} );
-
-			const region1 = new Region( 'x' );
-			const region2 = new Region( 'x' );
-
-			view.register( region1, true );
-			view.register( region2, true, true );
-			view.register( 'x', 'span', true );
-
-			view.init();
-
-			expect( view.regions.get( 'x' ) ).to.equal( region2 );
-			expect( view.regions.get( 'x' ).element ).to.equal( view.element.firstChild );
-		} );
-
-		it( 'should not override an existing region with the same region with override flag', () => {
-			const region = new Region( 'x' );
-			const spy = testUtils.sinon.spy( view.regions, 'remove' );
-
-			view.register( region, true );
-			view.register( region, true, true );
-
-			sinon.assert.notCalled( spy );
-		} );
-	} );
-
 	describe( 'element', () => {
 		beforeEach( createViewInstanceWithTemplate );
 
@@ -241,41 +185,72 @@ describe( 'View', () => {
 	} );
 
 	describe( 'destroy', () => {
-		beforeEach( createViewInstanceWithTemplate );
+		beforeEach( createViewWithChildren );
 
-		it( 'should destroy the view', () => {
-			view.destroy();
+		it( 'should return a promise', () => {
+			expect( view.destroy() ).to.be.instanceof( Promise );
+		} );
+
+		it( 'should set basic properties null', () => {
+			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.model ).to.be.null;
-			expect( view.regions ).to.be.null;
-			expect( view.template ).to.be.null;
-			expect( view.locale ).to.be.null;
-			expect( view.t ).to.be.null;
+				expect( view._unboundChildren ).to.be.null;
+				expect( view._viewCollections ).to.be.null;
+			} );
 		} );
 
-		it( 'detaches the element from DOM', () => {
-			const elRef = view.element;
+		it( 'clears #_unboundChildren', () => {
+			const cached = view._unboundChildren;
 
-			document.createElement( 'div' ).appendChild( view.element );
+			view.addChild( new View(), new View() );
+			expect( cached ).to.have.length.above( 1 );
+
+			return view.destroy().then( () => {
+				expect( cached ).to.have.length( 0 );
+			} );
+		} );
+
+		it( 'clears #_viewCollections', () => {
+			const cached = view._viewCollections;
 
-			view.destroy();
+			expect( cached ).to.have.length( 1 );
 
-			expect( elRef.parentNode ).to.be.null;
+			return view.destroy().then( () => {
+				expect( cached ).to.have.length( 0 );
+			} );
 		} );
 
-		it( 'destroys child regions', () => {
-			const region = new Region( 'x' );
-			const spy = testUtils.sinon.spy( region, 'destroy' );
-			const regionsRef = view.regions;
-			const regionViewsRef = region.views;
+		it( 'detaches the element from DOM', () => {
+			const elRef = view.element;
 
-			view.register( region, true );
-			view.regions.get( 'x' ).views.add( new View() );
-			view.destroy();
+			document.createElement( 'div' ).appendChild( view.element );
+			expect( elRef.parentNode ).to.be.not.null;
+
+			return view.destroy().then( () => {
+				expect( elRef.parentNode ).to.be.null;
+			} );
+		} );
 
-			expect( regionsRef.length ).to.equal( 0 );
-			expect( regionViewsRef.length ).to.equal( 0 );
-			expect( spy.calledOnce ).to.be.true;
+		it( 'calls destroy() on all views in #_viewCollections', () => {
+			const spy = testUtils.sinon.spy( view, 'destroy' );
+			const spyA = testUtils.sinon.spy( childA, 'destroy' );
+			const spyB = testUtils.sinon.spy( childB, 'destroy' );
+
+			return view.init().then( () => {
+				expect( view.ready ).to.be.true;
+				expect( childA.ready ).to.be.true;
+				expect( childB.ready ).to.be.true;
+
+				return view.destroy().then( () => {
+					sinon.assert.calledOnce( spyA );
+					sinon.assert.calledOnce( spyB );
+					sinon.assert.callOrder( spy, spyA, spyB );
+				} );
+			} );
 		} );
 
 		it( 'destroy a template–less view', () => {
@@ -293,7 +268,7 @@ function createViewInstanceWithTemplate() {
 	setTestViewInstance();
 }
 
-function setTestViewClass( templateDef, regionsFn ) {
+function setTestViewClass( templateDef ) {
 	TestView = class V extends View {
 		constructor() {
 			super();
@@ -301,10 +276,6 @@ function setTestViewClass( templateDef, regionsFn ) {
 			if ( templateDef ) {
 				this.template = new Template( templateDef );
 			}
-
-			if ( templateDef && regionsFn ) {
-				regionsFn.call( this );
-			}
 		}
 	};
 }
@@ -316,3 +287,63 @@ function setTestViewInstance() {
 		document.body.appendChild( view.element );
 	}
 }
+
+function createViewWithChildren() {
+	class ChildView extends View {
+		constructor() {
+			super();
+
+			this.template = new Template( {
+				tag: 'span'
+			} );
+		}
+	}
+
+	class ChildViewA extends ChildView {
+		init() {
+			const promise = new Promise( resolve => {
+				setTimeout( resolve, 50 );
+			} );
+
+			return super.init().then( promise );
+		}
+
+		destroy() {
+			const promise = new Promise( resolve => {
+				setTimeout( resolve, 10 );
+			} );
+
+			return super.destroy().then( promise );
+		}
+	}
+
+	class ChildViewB extends ChildView {
+		init() {
+			const promise = new Promise( resolve => {
+				setTimeout( resolve, 10 );
+			} );
+
+			return super.init().then( promise );
+		}
+
+		destroy() {
+			const promise = new Promise( resolve => {
+				setTimeout( resolve, 50 );
+			} );
+
+			return super.destroy().then( promise );
+		}
+	}
+
+	childA = new ChildViewA();
+	childB = new ChildViewB();
+
+	setTestViewClass( {
+		tag: 'p',
+		children: [ childA, childB ]
+	} );
+
+	setTestViewInstance();
+
+	view.addChild( childA, childB );
+}