Add inline plugin support to charts. #3363
Conversation
| * @returns {Boolean} false if any of the plugins return false, else returns true. | ||
| */ | ||
| notify: function(extension, args) { | ||
| var plugins = this._plugins; |
There was a problem hiding this comment.
I think this would have been easier and shorter:
var options = (chart && chart.options) || {};
var plugins = [].concat(this._plugins, options.plugins || []);
// ... previous code ...There was a problem hiding this comment.
good call. I will update that and fix the merge conflict
|
You guys rock! |
simonbrunel
force-pushed
the
inline-plugins
branch
2 times, most recently
from
19bdfdc
to
a026696
Compare
|
@etimberg I pushed the new implementation per our Slack discussions: it doesn't contain user documentation but once we switch to the new docs, I will move most of the proposal content into it. |
| }); | ||
| var res = Chart.plugins.notify('check'); | ||
| expect(res).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('should return FALSE if no plugin explicitly returns FALSE', function() { |
There was a problem hiding this comment.
This was already existing but I think it should be 'should return FALSE if any plugin explicitly returns FALSE'
There was a problem hiding this comment.
Nice catch! I will change it, rebase the branch and fix conflicts.
There was a problem hiding this comment.
perfect! then I think we can merge this if you feel its ready
There was a problem hiding this comment.
I would wait until we have the doc written and merge everything in one commit.
There was a problem hiding this comment.
ok, sounds like a good plan to me
simonbrunel
force-pushed
the
inline-plugins
branch
from
a026696
to
e3d0e24
Compare
Plugins can now be declared in the chart `config.plugins` array and will only be applied to the associated chart(s), after the globally registered plugins. Plugin specific options are now scoped under the `config.options.plugins` options. Hooks now receive the chart instance as first argument and the plugin options as last argument.
simonbrunel
force-pushed
the
inline-plugins
branch
from
e3d0e24
to
ff57293
Compare
|
@etimberg I rebased the branch on master, so should be ready to be merged. Since we can't get the new doc ready soon, maybe we should consider to merge it now and write doc later? |
|
Sure, that's ok by me. We should just make sure we don't release without the docs. |
Implementation of this plugins proposal.
Fixed #3335