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

t/16: Introduced the configuration API #19

Merged
merged 5 commits into from
Mar 2, 2015
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
12 changes: 9 additions & 3 deletions src/ckeditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* @singleton
*/

CKEDITOR.define( [ 'editor', 'mvc/collection', 'promise' ], function( Editor, Collection, Promise ) {
CKEDITOR.define( [ 'editor', 'mvc/collection', 'promise', 'config' ], function( Editor, Collection, Promise, Config ) {
var CKEDITOR = {
/**
* A collection containing all editor instances created.
Expand All @@ -40,7 +40,7 @@ CKEDITOR.define( [ 'editor', 'mvc/collection', 'promise' ], function( Editor, Co
* created instance.
* @returns {Promise} A promise, which will be fulfilled with the created editor.
*/
create: function( element ) {
create: function( element, config ) {
var that = this;

return new Promise( function( resolve, reject ) {
Expand All @@ -53,7 +53,7 @@ CKEDITOR.define( [ 'editor', 'mvc/collection', 'promise' ], function( Editor, Co
}
}

var editor = new Editor( element );
var editor = new Editor( element, config );

that.instances.add( editor );

Expand All @@ -66,6 +66,12 @@ CKEDITOR.define( [ 'editor', 'mvc/collection', 'promise' ], function( Editor, Co
} );
},

/**
* Holds global configuration defaults, which will be used by editor instances when such configurations are not
* available on them directly.
*/
config: new Config(),

/**
* Gets the full URL path for the specified plugin.
*
Expand Down
200 changes: 200 additions & 0 deletions src/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
/**
* @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

'use strict';

/**
* Handles a configuration dictionary.
*
* @class Config
* @extends Model
*/

CKEDITOR.define( [ 'mvc/model', 'utils' ], function( Model, utils ) {
var Config = Model.extend( {
/**
* Creates an instance of the {@link Config} class.
*
* @param {Object} [configurations] The initial configurations to be set.
* @constructor
*/
constructor: function Config( configurations ) {
// Call super-constructor.
Model.apply( this );

if ( configurations ) {
this.set( configurations );
}
},

/**
* Set configuration values.
*
* It accepts both a name/value pair or an object, which properties and values will be used to set
* configurations.
*
* It also accepts setting a "deep configuration" by using dots in the name. For example, `'resize.width'` sets
* the value for the `width` configuration in the `resize` subset.
*
* config.set( 'width', 500 );
* config.set( 'toolbar.collapsed', true );
*
* // Equivalent to:
* config.set( {
* width: 500
* toolbar: {
* collapsed: true
* }
* } );
*
* Passing an object as the value will amend the configuration, not replace it.
*
* config.set( 'toolbar', {
* collapsed: true,
* } );
*
* config.set( 'toolbar', {
* color: 'red',
* } );
*
* config.toolbar.collapsed; // true
* config.toolbar.color; // 'red'
*
* @param {String|Object} nameOrConfigurations The configuration name or an object from which take properties as
* configuration entries. Configuration names are case-insensitive.
* @param {*} [value=null] The configuration value. Used if a name is passed to nameOrConfigurations.
*/
set: function( name, value ) {
// Just pass the call to the original set() in case of an object. It'll deal with recursing through the
// object and calling set( name, value ) again for each property.
if ( utils.isObject( name ) ) {
Model.prototype.set.apply( this, arguments );

return;
}

// The target for this configuration is, for now, this object.
//jscs:disable safeContextKeyword
var target = this;
//jscs:enable

// The configuration name should be split into parts if it has dots. E.g: `resize.width`.
var parts = name.toLowerCase().split( '.' );
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to stress it across API docs that keys are automatically lowercased. I can't see it anywhere in docs ATM.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The docs are pointing to get() to retrieve configurations, which is also case-insensitive so it doesn't make much difference.

Ofc it is in any case worth to at least make it notice that the config keys are case-insensitive.

Btw, this is for now documented here:
https://github.com/ckeditor/ckeditor5-design/wiki/Configuration


// Take the name of the configuration out of the parts. E.g. `resize.width` -> `width`
name = parts.pop();

// Retrieves the final target for this configuration recursively.
for ( var i = 0; i < parts.length; i++ ) {
// The target will always be an instance of Config.
if ( !( target[ parts[ i ] ] instanceof Config ) ) {
target.set( parts[ i ], new Config() );
}

target = target[ parts[ i ] ];
}

// Values set as pure objects will be treated as Config subsets.
if ( utils.isPlainObject( value ) ) {
// If the target is an instance of Config (a deep config subset).
if ( target[ name ] instanceof Config ) {
// Amend the target with the value, instead of replacing it.
target[ name ].set( value );

return;
}

value = new Config( value );
}

// Values will never be undefined.
if ( typeof value == 'undefined' ) {
value = null;
}

// Call the original set() on the target.
Model.prototype.set.call( target, name, value );
},

/**
* Gets the value for a configuration entry.
*
* config.get( 'name' );
*
* Deep configurations can be retrieved by separating each part with a dot.
*
* config.get( 'toolbar.collapsed' );
*
* @param {String} name The configuration name. Configuration names are case-insensitive.
* @returns {*} The configuration value or `undefined` if the configuration entry was not found.
*/
get: function( name ) {
// The target for this configuration is, for now, this object.
//jscs:disable safeContextKeyword
var source = this;
Copy link

Choose a reason for hiding this comment

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

Why we need this variable? And why it's sometimes called source and sometimes target? I suggest to follow common approach and call them that or self .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is a trick to reference this inside inner-functions. That's not the case here.

You should read the rest of the code... here source (on get()) and target (on set()) refer to the source/target object for the configurations, which at start is this but then eventually moves to child properties of this. Therefore a more semantical name is used for the variable and unfortunately the jscs rules we have applied are too strict for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

y, it make sense - good naming.

Copy link

Choose a reason for hiding this comment

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

Ahh I see now, my bad.

//jscs:enable

// The configuration name should be split into parts if it has dots. E.g. `resize.width` -> [`resize`, `width`]
var parts = name.toLowerCase().split( '.' );

// Take the name of the configuration from the parts. E.g. `resize.width` -> `width`
name = parts.pop();

// Retrieves the source for this configuration recursively.
for ( var i = 0; i < parts.length; i++ ) {
// The target will always be an instance of Config.
if ( !( source[ parts[ i ] ] instanceof Config ) ) {
source = null;
break;
}

source = source[ parts[ i ] ];
}

// Try to retrieve it from the source object.
if ( source && ( typeof source[ name ] != 'undefined' ) ) {
return source[ name ];
}

// If not found, take it from the definition.
if ( this.definition ) {
return this.definition[ name ];
}

return undefined;
},

/**
* Defines the name and default value for configurations. It accepts the same parameters as the
* {@link Config#set set()} method.
*
* On first call, the {@link Config#definition definition} property is created to hold all defined
* configurations.
*
* This method is supposed to be called by plugin developers to setup plugin's configurations. It would be
* rarely used for other needs.
*
* @param {String|Object} nameOrConfigurations The configuration name or an object from which take properties as
* configuration entries.
* @param {*} [value] The configuration value. Used if a name is passed to nameOrConfigurations. If undefined,
* the configuration is set to `null`.
*/
define: function( name, value ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not really convinced to the name of this method... Wouldn't it make more sense to name it default and store its values in this.defaults?

Copy link
Member

Choose a reason for hiding this comment

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

I'm out of context right now so ignore my message if it misses the point. But I remember that defining config properties was aimed at not only defining defaults, but also what configuration options are out there (possible validation).

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, that makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aimed at not only defining defaults, but also what configuration options are out there (possible validation)

Exactly... that's why it's named "define".

if ( !this.definition ) {
/**
*
*
* @property
* @type {Config}
*/
this.definition = new Config();
}

this.definition.set( name, value );
}
} );

return Config;
} );
15 changes: 13 additions & 2 deletions src/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* @class Editor
*/

CKEDITOR.define( [ 'ckeditor', 'mvc/model', 'utils' ], function( CKEDITOR, Model ) {
CKEDITOR.define( [ 'mvc/model', 'editorconfig' ], function( Model, EditorConfig ) {
var Editor = Model.extend( {
/**
* Creates a new instance of the Editor class.
Expand All @@ -22,7 +22,7 @@ CKEDITOR.define( [ 'ckeditor', 'mvc/model', 'utils' ], function( CKEDITOR, Model
* @param {HTMLElement} element The DOM element that will be the source for the created editor.
* @constructor
*/
constructor: function Editor( element ) {
constructor: function Editor( element, config ) {
/**
* The original host page element upon which the editor is created. It is only supposed to be provided on
* editor creation and is not subject to be modified.
Expand All @@ -31,6 +31,17 @@ CKEDITOR.define( [ 'ckeditor', 'mvc/model', 'utils' ], function( CKEDITOR, Model
* @property {HTMLElement}
*/
this.element = element;

/**
* Holds all configurations specific to this editor instance.
*
* This instance of the {@link Config} class is customized so its {@link Config#get} method will retrieve
* global configurations available in {@link CKEDITOR.config} if configurations are not found in the
* instance itself.
*
* @type {Config}
*/
this.config = new EditorConfig( config );
},

/**
Expand Down
55 changes: 55 additions & 0 deletions src/editorconfig.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

'use strict';

/**
* Handles a configuration dictionary for an editor instance.
*
* The basic difference between {@link EditorConfig} and {@link Config} is that {@link EditorConfig#get} retrieves
* configurations from {@link CKEDITOR#config} if they are not found.
*
* @class EditorConfig
* @extends Config
*/

CKEDITOR.define( [ 'ckeditor', 'config' ], function( CKE, Config ) {
var EditorConfig = Config.extend( {
/**
* Creates an instance of the {@link EditorConfig} class.
*
* @param {Object} [configurations] The initial configurations to be set.
* @constructor
*/
constructor: function EditorConfig() {
// Call super-constructor.
Config.apply( this, arguments );
},

/**
* @inheritdoc Config#get
*/
get: function() {
// Try to take it from this editor instance.
var value = Config.prototype.get.apply( this, arguments );

// If the configuration is not defined in the instance, try to take it from CKEDITOR.config.
if ( typeof value == 'undefined' ) {
// There is a circular dependency issue here: CKEDITOR -> Editor -> EditorConfig -> CKEDITOR.
// Therefore we need to require() it again here. That's why the parameter was named CKE.
//
// Note additionally that we still keep 'ckeditor' in the dependency list for correctness, to ensure
// that the module is loaded.

CKE = CKE || CKEDITOR.require( 'ckeditor' );
value = CKE.config.get.apply( CKE.config, arguments );
}

return value;
}
} );

return EditorConfig;
} );
Loading