Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When the legend label options are not defined, no error should occur #4402

Merged
merged 1 commit into from Jun 24, 2017

Conversation

etimberg
Copy link
Member

Resolves #4398

@etimberg etimberg added this to the Version 2.7 milestone Jun 22, 2017
var labelOpts = me.options.labels;
var legendItems = labelOpts.generateLabels.call(me, me.chart);
var labelOpts = me.options.labels || {};
var legendItems = labelOpts.generateLabels ? labelOpts.generateLabels.call(me, me.chart) : [];
Copy link
Member

Choose a reason for hiding this comment

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

We could use helpers.callback here:

var items = helpers.callback(options.generateLabels, [me.chart], me) || [];

Copy link
Member Author

Choose a reason for hiding this comment

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

good call! Will make that change

Copy link
Member Author

Choose a reason for hiding this comment

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

actually, turns out that that won't work because helpers.callback doesn't return the result of the callback.

Copy link
Member

Choose a reason for hiding this comment

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

We could change the callback method to return the fn result:

callback: function(fn, args, thisArg) {
	if (fn && typeof fn.call === 'function') {
		return fn.apply(thisArg, args);
	}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm ok with that. Will test out that change locally and see how it goes

@simonbrunel simonbrunel merged commit 6c82c93 into master Jun 24, 2017
@simonbrunel simonbrunel deleted the fix/4398 branch June 24, 2017 15:35
exwm pushed a commit to exwm/Chart.js that referenced this pull request Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants