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

Fixed: Cannot read property '$context' of null #54

Merged
merged 3 commits into from
Apr 28, 2018

Conversation

pfrankov
Copy link
Contributor

Since there is a possibility to get label as null (https://github.com/chartjs/chartjs-plugin-datalabels/blob/master/src/plugin.js#L178) it should be also filtered out from updating.

Copy link
Member

@simonbrunel simonbrunel left a comment

Choose a reason for hiding this comment

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

Good catch! I'm not sure when that happen though, can you build a jsfiddle that reproduces this issue?

src/plugin.js Outdated
@@ -232,8 +232,10 @@ Chart.plugins.register({
update = updates[i];
if (update[1]) {
label = update[0][EXPANDO_KEY];
label.$context.active = (update[1] === 1);
label.update(label.$context);
if (label && label.$context) {
Copy link
Member

Choose a reason for hiding this comment

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

I would only test for label since label.$context should always be defined, else that's a bug and should not be silently ignored.

@simonbrunel simonbrunel added this to the Version 0.4 milestone Apr 27, 2018
@pfrankov
Copy link
Contributor Author

Reproduced in a complex chart but unfortunately can not isolate the issue :(

@simonbrunel
Copy link
Member

Can you share your complex chart code? I'm curious why that happens because label could be null only if the element is hidden or skipped, in which case it should not be part of any interactions (chart.lastActive).

@pfrankov
Copy link
Contributor Author

I'd share it already if I could.

@simonbrunel simonbrunel merged commit b3e7564 into chartjs:master Apr 28, 2018
@simonbrunel
Copy link
Member

Thanks @pfrankov

@pfrankov pfrankov deleted the patch-1 branch April 28, 2018 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants