Parcourir la source

Merge branch 'master' into t/35

Aleksander Nowodzinski il y a 9 ans
Parent
commit
9b0fd01058

+ 6 - 0
packages/ckeditor5-ui/src/bindings/stickytoolbar.js

@@ -28,6 +28,12 @@ export default class StickyToolbar extends BaseStickyToolbar {
 
 		this.editor = editor;
 	}
+
+	init() {
+		this.bindToolbarItems();
+
+		return super.init();
+	}
 }
 
 mix( StickyToolbar, ToolbarBindingsMixin );

+ 6 - 0
packages/ckeditor5-ui/src/bindings/toolbar.js

@@ -28,6 +28,12 @@ export default class Toolbar extends BaseToolbar {
 
 		this.editor = editor;
 	}
+
+	init() {
+		this.bindToolbarItems();
+
+		return super.init();
+	}
 }
 
 mix( Toolbar, ToolbarBindingsMixin );

+ 21 - 7
packages/ckeditor5-ui/src/bindings/toolbarbindingsmixin.js

@@ -3,6 +3,8 @@
  * For licensing, see LICENSE.md.
  */
 
+import Collection from '../../utils/collection.js';
+
 /**
  * Mixin that injects the common Toolbar–like bindings.
  *
@@ -12,15 +14,27 @@
  */
 const ToolbarBindingsMixin = {
 	/**
-	 * Adds buttons to the toolbar. Buttons are taken from the {@link ui.editorUI.EditorUI#featureComponents}
-	 * factory.
-	 *
-	 * @param {String[]} buttons The name of the buttons to add to the toolbar.
+	 * Adds items like buttons to the toolbar. Buttons are generated by the
+	 * {@link ui.editorUI.EditorUI#featureComponents} factory.
 	 */
-	addButtons( buttons ) {
-		for ( let button of buttons ) {
-			this.add( 'buttons', this.editor.ui.featureComponents.create( button ) );
+	bindToolbarItems() {
+		/**
+		 * Collection of toolbar items.
+		 *
+		 * @member {utils.Collection} ui.bindings.ToolbarBindingsMixin#items
+		 */
+		this.items = new Collection();
+
+		if ( this.model.config ) {
+			// Translate config to dynamic item collection.
+			for ( let name of this.model.config ) {
+				this.items.add( { name } );
+			}
 		}
+
+		this.collections.get( 'items' ).bind( this.items ).as( ( { name } ) => {
+			return this.editor.ui.featureComponents.create( name );
+		} );
 	}
 };
 

+ 54 - 53
packages/ckeditor5-ui/src/controllercollection.js

@@ -140,8 +140,8 @@ export default class ControllerCollection extends Collection {
 	 *		data.remove( 0 );
 	 *		console.log( controllers.length == 1 );
 	 *
-	 * @param {utils.Collection.<ui.Model>} models Models to be synchronized with this controller collection.
-	 * @returns {Function} The `as` function in the `bind( models ).as( ... )` chain.
+	 * @param {utils.Collection.<ui.Model>} collection Models to be synchronized with this controller collection.
+	 * @returns {Function} The `as` function in the `bind( collection ).as( ... )` chain.
 	 * It activates factory using controller and view classes or uses a custom callback to produce
 	 * controller (view) instances.
 	 * @returns {Function} return.ControllerClassOrFunction Specifies the constructor of the controller to be used or
@@ -149,28 +149,28 @@ export default class ControllerCollection extends Collection {
 	 * @returns {Function} [return.ViewClass] Specifies constructor of the view to be used. If not specified,
 	 * `ControllerClassOrFunction` works as as custom callback function.
 	 */
-	bind( models ) {
-		const idProperty = models._idProperty;
+	bind( collection ) {
+		const controllerMap = new Map();
 
 		return {
 			as: ( ControllerClassOrFunction, ViewClass ) => {
-				const createController = ViewClass ?
-						defaultControllerFactory( ControllerClassOrFunction, ViewClass, idProperty )
+				const factory = ViewClass ?
+						defaultControllerFactory( ControllerClassOrFunction, ViewClass, controllerMap )
 					:
-						customControllerFactory( ControllerClassOrFunction, idProperty );
+						genericControllerFactory( ControllerClassOrFunction, controllerMap );
 
-				for ( let model of models ) {
-					this.add( createController( model, this.locale ) );
+				for ( let item of collection ) {
+					this.add( factory.create( item, this.locale ) );
 				}
 
-				// Updated controller collection when a new model is added.
-				models.on( 'add', ( evt, model, index ) => {
-					this.add( createController( model, this.locale ), index );
+				// Updated controller collection when a new item is added.
+				collection.on( 'add', ( evt, item, index ) => {
+					this.add( factory.create( item, this.locale ), index );
 				} );
 
-				// Update controller collection when a model is removed.
-				models.on( 'remove', ( evt, model ) => {
-					this.remove( model[ idProperty ] );
+				// Update controller collection when a item is removed.
+				collection.on( 'remove', ( evt, item ) => {
+					this.remove( factory.delete( item ) );
 				} );
 			}
 		};
@@ -247,50 +247,51 @@ export default class ControllerCollection extends Collection {
 	}
 }
 
-// Initializes controller factory with controller and view classes.
+// Initializes a generic controller factory.
 //
-// @param {Function} ControllerClass Specifies the constructor of the controller to be used.
-// @param {Function} ViewClass Specifies constructor of the view.
-// @param {String} idProperty A property used to associate the controller with its model {@link utils.Collection._idProperty}.
-// @returns {Function}
-function defaultControllerFactory( ControllerClass, ViewClass, idProperty ) {
-	// Returns a controller instance (and its view) for given model and class names.
-	//
-	// @param {ui.Model} model A model of the controller.
-	// @param {utils.Locale} [locale] The {@link core.editor.Editor#locale editor's locale} instance.
-	// @returns {ui.Controller}
-	return ( model, locale ) => {
-		return flagController( new ControllerClass( model, new ViewClass( locale ) ), idProperty );
-	};
-}
+// @param {Function} createController A function which returns controller instance if provided with data and locale.
+// @param {Map} controllerMap A Map used to associate data in the collection with corresponding controller instance.
+// @returns {Object}
+function genericControllerFactory( createController, controllerMap ) {
+	return {
+		// Returns a controller instance (and its view) for given data (i.e. Model) and locale.
+		//
+		// @param {Object} data A data to creates the controller, usually {@link ui.Model}.
+		// @param {utils.Locale} [locale] The {@link ckeditor5.Editor#locale editor's locale} instance.
+		// @returns {ui.Controller}
+		create( data, locale ) {
+			const controller = createController( data, locale );
 
-// Initializes controller factory which is fed by a custom callback.
-//
-// @param {Function} callback A callback which is to return an instance of {@link ui.Controller}.
-// @param {String} idProperty A property used to associate the controller with its model {@link utils.Collection._idProperty}.
-// @returns {Function}
-function customControllerFactory( callback, idProperty ) {
-	// Returns a controller instance (and its view) produced by the custom callback.
-	//
-	// @param {ui.Model} model A model of the controller.
-	// @param {utils.Locale} [locale] The {@link core.editor.Editor#locale editor's locale} instance.
-	// @returns {ui.Controller}
-	return ( ...args ) => {
-		return flagController( callback( ...args ), idProperty );
+			controllerMap.set( data, controller );
+
+			return controller;
+		},
+
+		// Deletes controller instance in the factory by associated data and returns
+		// the controller instance.
+		//
+		// @param {Object} data A data associated with the controller, usually {@link ui.Model}.
+		// @returns {ui.Controller}
+		delete( data ) {
+			const controller = controllerMap.get( data );
+
+			controllerMap.delete( controller );
+
+			return controller;
+		}
 	};
 }
 
-// Gives the controller an id corresponding with {@link utils.Collection#_idProperty} of the model.
-// It allows retrieving this controller instance by the model in the future
-// and avoids a brute–force search in the entire controller collection.
+// Initializes controller factory with controller and view classes.
 //
-// @param {ui.Controller} controller An instance of controller.
-// @param {String} idProperty A property used to associate the controller with its model {@link utils.Collection._idProperty}.
-// @returns {ui.Controller}
-function flagController( controller, idProperty ) {
-	controller.id = controller.model[ idProperty ];
-
-	return controller;
+// @param {Function} ControllerClass Specifies the constructor of the controller to be used.
+// @param {Function} ViewClass Specifies constructor of the view.
+// @param {Map} controllerMap A Map used to associate data in the collection with corresponding controller instance.
+// @returns {Object}
+function defaultControllerFactory( ControllerClass, ViewClass, controllerMap ) {
+	return genericControllerFactory( ( model, locale ) => {
+		return new ControllerClass( model, new ViewClass( locale ) );
+	}, controllerMap );
 }
 
 // Check if all entries of the array are of `String` type.

+ 28 - 5
packages/ckeditor5-ui/tests/bindings/stickytoolbar.js

@@ -3,21 +3,34 @@
  * For licensing, see LICENSE.md.
  */
 
-/* bender-tags: ui, toolbar */
+/* bender-tags: ui, bindings, toolbar */
 
 import Editor from '/ckeditor5/core/editor/editor.js';
 import Model from '/ckeditor5/ui/model.js';
 import View from '/ckeditor5/ui/view.js';
-import StickyToolbar from '/ckeditor5/ui/bindings/toolbar.js';
+import Controller from '/ckeditor5/ui/controller.js';
+import StickyToolbar from '/ckeditor5/ui/bindings/stickytoolbar.js';
+
+import testUtils from '/tests/core/_utils/utils.js';
+testUtils.createSinonSandbox();
 
 describe( 'StickyToolbar', () => {
-	let toolbar, model, editor;
+	let toolbar, model;
+
+	const editor = new Editor();
+
+	editor.ui = {
+		featureComponents: {
+			create: () => new Controller()
+		}
+	};
 
 	beforeEach( () => {
-		editor = new Editor();
 		model = new Model( {
-			isActive: false
+			isActive: false,
+			config: [ 'bold', 'italic' ]
 		} );
+
 		toolbar = new StickyToolbar( model, new View(), editor );
 	} );
 
@@ -26,4 +39,14 @@ describe( 'StickyToolbar', () => {
 			expect( toolbar ).to.have.property( 'editor', editor );
 		} );
 	} );
+
+	describe( 'init', () => {
+		it( 'calls bindToolbarItems', () => {
+			const spy = testUtils.sinon.spy( toolbar, 'bindToolbarItems' );
+
+			return toolbar.init().then( () => {
+				expect( spy.calledOnce ).to.be.true;
+			} );
+		} );
+	} );
 } );

+ 27 - 4
packages/ckeditor5-ui/tests/bindings/toolbar.js

@@ -3,21 +3,34 @@
  * For licensing, see LICENSE.md.
  */
 
-/* bender-tags: ui, toolbar */
+/* bender-tags: ui, bindings, toolbar */
 
 import Editor from '/ckeditor5/core/editor/editor.js';
 import Model from '/ckeditor5/ui/model.js';
 import View from '/ckeditor5/ui/view.js';
+import Controller from '/ckeditor5/ui/controller.js';
 import Toolbar from '/ckeditor5/ui/bindings/toolbar.js';
 
+import testUtils from '/tests/core/_utils/utils.js';
+testUtils.createSinonSandbox();
+
 describe( 'Toolbar', () => {
-	let toolbar, model, editor;
+	let toolbar, model;
+
+	const editor = new Editor();
+
+	editor.ui = {
+		featureComponents: {
+			create: () => new Controller()
+		}
+	};
 
 	beforeEach( () => {
-		editor = new Editor();
 		model = new Model( {
-			isActive: false
+			isActive: false,
+			config: [ 'bold', 'italic' ]
 		} );
+
 		toolbar = new Toolbar( model, new View(), editor );
 	} );
 
@@ -26,4 +39,14 @@ describe( 'Toolbar', () => {
 			expect( toolbar ).to.have.property( 'editor', editor );
 		} );
 	} );
+
+	describe( 'init', () => {
+		it( 'calls bindToolbarItems', () => {
+			const spy = testUtils.sinon.spy( toolbar, 'bindToolbarItems' );
+
+			return toolbar.init().then( () => {
+				expect( spy.calledOnce ).to.be.true;
+			} );
+		} );
+	} );
 } );

+ 37 - 34
packages/ckeditor5-ui/tests/bindings/toolbarbindingsmixin.js

@@ -3,63 +3,66 @@
  * For licensing, see LICENSE.md.
  */
 
-/* bender-tags: ui, toolbar */
+/* bender-tags: ui, bindings, toolbar */
 
 import mix from '/ckeditor5/utils/mix.js';
 import Editor from '/ckeditor5/core/editor/editor.js';
+import Collection from '/ckeditor5/utils/collection.js';
+import Model from '/ckeditor5/ui/model.js';
 import Controller from '/ckeditor5/ui/controller.js';
 import ToolbarBindingsMixin from '/ckeditor5/ui/bindings/toolbarbindingsmixin.js';
 
 describe( 'ToolbarBindingsMixin', () => {
-	let mixinInstance, editor;
+	const editor = new Editor();
+	let mixinInstance;
 
-	beforeEach( () => {
-		editor = new Editor();
+	editor.ui = {
+		featureComponents: {
+			create: () => new Controller()
+		}
+	};
 
-		class MixClass extends Controller {
-			constructor( model, view ) {
-				super( model, view );
+	class MixClass extends Controller {
+		constructor( model, view ) {
+			super( model, view );
 
-				this.addCollection( 'buttons' );
-			}
+			this.editor = editor;
+			this.addCollection( 'items' );
 		}
+	}
 
-		mix( MixClass, ToolbarBindingsMixin );
+	mix( MixClass, ToolbarBindingsMixin );
 
-		mixinInstance = new MixClass();
-		mixinInstance.editor = editor;
+	beforeEach( () => {
+		mixinInstance = new MixClass( new Model( {
+			config: [ 'bold', 'italic' ]
+		} ) );
 	} );
 
-	describe( 'addButtons', () => {
-		it( 'creates buttons for each button name', () => {
-			const createSpy = sinon.spy( () => new Controller() );
+	describe( 'bindToolbarItems', () => {
+		it( 'creates item collection', () => {
+			mixinInstance.bindToolbarItems();
 
-			editor.ui = {
-				featureComponents: {
-					create: createSpy
-				}
-			};
+			expect( mixinInstance.items ).to.be.instanceOf( Collection );
+			expect( mixinInstance.items.map( i => i.name ) ).to.have.members( [ 'bold', 'italic' ] );
+		} );
 
-			mixinInstance.addButtons( [ 'foo', 'bar' ] );
+		it( 'works when no config specified in the model', () => {
+			mixinInstance = new MixClass( new Model( {} ) );
+			mixinInstance.bindToolbarItems();
 
-			expect( createSpy.callCount ).to.equal( 2 );
-			expect( createSpy.firstCall.calledWith( 'foo' ) ).to.be.true;
-			expect( createSpy.secondCall.calledWith( 'bar' ) ).to.be.true;
+			expect( mixinInstance.items ).to.be.instanceOf( Collection );
+			expect( mixinInstance.items ).to.have.length( 0 );
 		} );
 
-		it( 'adds created components to the collection of buttons', () => {
-			const component = new Controller();
-			const createSpy = sinon.spy( () => component );
+		it( 'binds item collection to "items" controller collection', () => {
+			const items = mixinInstance.collections.get( 'items' );
 
-			editor.ui = {
-				featureComponents: {
-					create: createSpy
-				}
-			};
+			expect( items ).to.have.length( 0 );
 
-			mixinInstance.addButtons( [ 'foo' ] );
+			mixinInstance.bindToolbarItems();
 
-			expect( mixinInstance.collections.get( 'buttons' ).get( 0 ) ).to.equal( component );
+			expect( items ).to.have.length( 2 );
 		} );
 	} );
 } );

+ 26 - 2
packages/ckeditor5-ui/tests/controllercollection.js

@@ -167,7 +167,7 @@ describe( 'ControllerCollection', () => {
 					models.remove( 2 );
 					models.remove( 3 );
 
-					expect( controllers.map( c => c.id ) ).to.have.members( [ '0', '1', '3' ] );
+					expect( controllers.map( c => c.model.uid ) ).to.have.members( [ '0', '1', '3' ] );
 				} );
 
 				it( 'passes controller collection\'s locale to the views', () => {
@@ -229,7 +229,7 @@ describe( 'ControllerCollection', () => {
 					models.remove( 2 );
 					models.remove( 3 );
 
-					expect( controllers.map( c => c.id ) ).to.have.members( [ '0', '1', '3' ] );
+					expect( controllers.map( c => c.model.uid ) ).to.have.members( [ '0', '1', '3' ] );
 				} );
 
 				it( 'passes controller collection\'s locale to the views', () => {
@@ -243,6 +243,30 @@ describe( 'ControllerCollection', () => {
 					expect( controllers.get( 0 ).view.locale ).to.equal( locale );
 				} );
 			} );
+
+			describe( 'custom data format with custom factory', () => {
+				it( 'expands the initial collection of the models', () => {
+					const controllers = new ControllerCollection( 'synced' );
+					const data = new Collection();
+
+					data.add( { foo: 'a' } );
+					data.add( { foo: 'b' } );
+
+					controllers.bind( data ).as( ( item, locale ) => {
+						const model = new Model( {
+							custom: item.foo
+						} );
+
+						return new ItemController( model, new ItemView( locale ) );
+					} );
+
+					expect( controllers ).to.have.length( 2 );
+					expect( controllers.get( 0 ).model.custom ).to.equal( 'a' );
+					expect( controllers.get( 1 ).model.custom ).to.equal( 'b' );
+					expect( controllers.get( 0 ) ).to.be.instanceOf( ItemController );
+					expect( controllers.get( 0 ).view ).to.be.instanceOf( ItemView );
+				} );
+			} );
 		} );
 	} );