Skip to content

Conversation

@Adondriel
Copy link

Added an optional Label option to Line Annotation.
Updated the documentation to display this.

Adam Pine added 5 commits July 8, 2016 14:06
Fixes an inconsistency between the "configuration" section and the "line annotations" section, where config section had value in single quotes and line annotation section has it as a number.
Made it so the badge is drawn conditionally, so that if you don't have a label, it doesn't show up, to avoid an null label badge.
Added a todo comment about what I would like to do with this eventually.
},

afterDraw: function(chartInstance, easingDecimal) {
beforeDraw: function(chartInstance, easingDecimal) {
Copy link
Member

Choose a reason for hiding this comment

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

probably should be beforeDatasetsDraw per my comment in the other issue

if (options.label) {
//draw the background of the badge.
ctx.fillStyle = 'white';
ctx.fillRect((chartArea.right / 2), pixel - 5, 35, 15);
Copy link
Member

Choose a reason for hiding this comment

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

Sorry that this took a while. I went to merge this and do some testing and I noticed this is incorrect. The width of the fill rect should be the width of the text. The text can be measured using ctx.measureText. To use this effectively, the font should be configurable and would need to be set before calling measureText.

The height of the rect would be the font size in px.

@compwright
Copy link
Collaborator

It doesn't appear that this works correctly with vertical lines yet.

petewalker pushed a commit to petewalker/chartjs-plugin-annotation that referenced this pull request Oct 18, 2016
Originally based on chartjs#12.
Added an optional Label option to Line Annotation.
Updated the documentation to display this. Also added a sample showing
how it can work.
@petewalker
Copy link
Contributor

I've added a follow-up to this pull request that addresses the configurability issues, and the compatibility with vertical lines: #31

petewalker pushed a commit to petewalker/chartjs-plugin-annotation that referenced this pull request Oct 19, 2016
Originally based on chartjs#12.
Added an optional Label option to Line Annotation.
Updated the documentation to display this. Also added a sample showing
how it can work.
@compwright
Copy link
Collaborator

@etimberg This one should be closed if you're planning to accept #31.

petewalker pushed a commit to petewalker/chartjs-plugin-annotation that referenced this pull request Nov 4, 2016
Originally based on chartjs#12.
Added an optional Label option to Line Annotation.
Updated the documentation to display this. Also added a sample showing
how it can work.
@compwright compwright closed this Nov 14, 2016
compwright pushed a commit that referenced this pull request Nov 15, 2016
* Added tooltip label to line chart

Originally based on #12.
Added an optional Label option to Line Annotation.
Updated the documentation to display this. Also added a sample showing
how it can work.

* Fixed calculations as per comments

Moved rendering calculations to lineUpdate function, also fixed a minor
reference bug that prevented animations from working at all.

* Fixed font bug introduced by last commit

* Fixed sample post-update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants