Enhance plugin notification system

Change the plugin notification behavior: this method now returns false as soon as a plugin *explicitly* returns false, else returns true. Also, plugins are now called in their own scope (so remove the never used `scope` parameter).

diff --git a/src/core/core.plugin.js b/src/core/core.plugin.js
index 7126dcd43d5a49e88b0443f341d1751b0f0c0b4e..2b020ac687c37fb3750645c439fa218d0690986c 100644 (file)
--- a/src/core/core.plugin.js
+++ b/src/core/core.plugin.js
@@ -2,8 +2,7 @@
 
 module.exports = function(Chart) {
 
-       var helpers = Chart.helpers;
-       var noop = helpers.noop;
+       var noop = Chart.helpers.noop;
 
        /**
         * The plugin service singleton
@@ -30,21 +29,33 @@ module.exports = function(Chart) {
                },
 
                /**
-                * Calls registered plugins on the specified method, with the given args. This
-                * method immediately returns as soon as a plugin explicitly returns false.
+                * Calls registered plugins on the specified extension, with the given args. This
+                * method immediately returns as soon as a plugin explicitly returns false. The
+                * returned value can be used, for instance, to interrupt the current action.
+                * @param {String} extension the name of the plugin method to call (e.g. 'beforeUpdate').
+                * @param {Array} [args] extra arguments to apply to the extension call.
                 * @returns {Boolean} false if any of the plugins return false, else returns true.
                 */
-               notify: function(method, args, scope) {
-                       helpers.each(this._plugins, function(plugin) {
-                               if (plugin[method] && typeof plugin[method] === 'function') {
-                                       plugin[method].apply(scope, args);
+               notify: function(extension, args) {
+                       var plugins = this._plugins;
+                       var ilen = plugins.length;
+                       var i, plugin;
+
+                       for (i=0; i<ilen; ++i) {
+                               plugin = plugins[i];
+                               if (typeof plugin[extension] === 'function') {
+                                       if (plugin[extension].apply(plugin, args || []) === false) {
+                                               return false;
+                                       }
                                }
-                       }, scope);
+                       }
+
+                       return true;
                }
        };
 
        Chart.PluginBase = Chart.Element.extend({
-               // Plugin methods. All functions are passed the chart instance
+               // Plugin extensions. All functions are passed the chart instance
 
                // Called at start of chart init
                beforeInit: noop,

diff --git a/test/core.plugin.tests.js b/test/core.plugin.tests.js
index 62e5e21880822cbe372d8c53a43d5fd1a50b6817..c3c290128bbcce091bd771094d26c14a86d743a4 100644 (file)
--- a/test/core.plugin.tests.js
+++ b/test/core.plugin.tests.js
@@ -37,17 +37,36 @@ describe('Test the plugin system', function() {
                Chart.plugins.remove(myplugin);
        });
 
-       it ('Should allow notifying plugins', function() {
-               var myplugin = {
-                       count: 0,
-                       trigger: function(chart) {
-                               myplugin.count = chart.count;
-                       }
-               };
-               Chart.plugins.register(myplugin);
+       describe('Chart.plugins.notify', function() {
+               it ('should call plugins with arguments', function() {
+                       var myplugin = {
+                               count: 0,
+                               trigger: function(chart) {
+                                       myplugin.count = chart.count;
+                               }
+                       };
+
+                       Chart.plugins.register(myplugin);
+                       Chart.plugins.notify('trigger', [{ count: 10 }]);
+                       expect(myplugin.count).toBe(10);
+               });
 
-               Chart.plugins.notify('trigger', [{ count: 10 }]);
+               it('should return TRUE if no plugin explicitly returns FALSE', function() {
+                       Chart.plugins.register({ check: function() {} });
+                       Chart.plugins.register({ check: function() { return; } });
+                       Chart.plugins.register({ check: function() { return null; } });
+                       Chart.plugins.register({ check: function() { return 42 } });
+                       var res = Chart.plugins.notify('check');
+                       expect(res).toBeTruthy();
+               });
 
-               expect(myplugin.count).toBe(10);
+               it('should return FALSE if no plugin explicitly returns FALSE', function() {
+                       Chart.plugins.register({ check: function() {} });
+                       Chart.plugins.register({ check: function() { return; } });
+                       Chart.plugins.register({ check: function() { return false; } });
+                       Chart.plugins.register({ check: function() { return 42 } });
+                       var res = Chart.plugins.notify('check');
+                       expect(res).toBeFalsy();
+               });
        });
 });