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

Refresh annotations when on config change #37

Merged
merged 7 commits into from
Nov 21, 2016

Conversation

compwright
Copy link
Collaborator

Resolves #36

@compwright compwright added the bug label Nov 19, 2016
@compwright
Copy link
Collaborator Author

@etimberg could I get you to review this before I move forward?

Copy link
Member

@etimberg etimberg left a comment

Choose a reason for hiding this comment

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

I just had a few minor comments looks good.

I tried out the functionality. The new sample works well. I noticed that the label sample doesn't have any labels. Not sure if that was because of these changes or not.

}
});
}
// Chartjs Zoom Plugin
Copy link
Member

Choose a reason for hiding this comment

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

should probably be 'Annotation' not 'Zoom' but that was already there

drawTime: "afterDraw", // defaults to drawing after draw
annotations: [] // default to no annotations
Chart.Annotation.drawTimeOptions = {
AFTER: 'afterDraw',
Copy link
Member

Choose a reason for hiding this comment

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

should not this change in the next release notes as it's a breaking change

Chart.Annotation.annotationTypes = {
line: lineAnnotation.Constructor,
box: boxAnnotation.Constructor
Chart.Annotation.types = {
Copy link
Member

Choose a reason for hiding this comment

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

might be a good idea to store this in a local variable as well to improve minification

let types = Chart.Annotation.types = { ... }

* Don’t break `Chart.Annotation.drawTimeOptions`
* Fixed an issue where annotations weren’t rendered if the `drawTime` option was not set
* Remove duplicate plugin hook
@compwright compwright merged commit 5e4b6c2 into chartjs:master Nov 21, 2016
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.

None yet

2 participants