-
Notifications
You must be signed in to change notification settings - Fork 12k
V2.0 dev skip null fixes #1566
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
V2.0 dev skip null fixes #1566
Changes from all commits
8081e9c
ae0d9b0
afc40e7
9d540ce
7b13e90
e50d2f7
ea57100
1c40d50
2cd4b13
e7b71aa
6adf39e
4d009d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,3 +15,5 @@ script: | |
|
|
||
| notifications: | ||
| slack: chartjs:pcfCZR6ugg5TEcaLtmIfQYuA | ||
|
|
||
| sudo: false | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,8 +27,6 @@ | |
| borderDashOffset: 0.0, | ||
| borderJoinStyle: 'miter', | ||
| fill: true, // do we fill in the area between the line and its base axis | ||
| skipNull: true, | ||
| drawNull: false, | ||
| }; | ||
|
|
||
|
|
||
|
|
@@ -44,50 +42,51 @@ | |
|
|
||
| // Draw the background first (so the border is always on top) | ||
| helpers.each(this._children, function(point, index) { | ||
|
|
||
| var previous = helpers.previousItem(this._children, index); | ||
| var next = helpers.nextItem(this._children, index); | ||
|
|
||
| // First point only | ||
| if (index === 0) { | ||
| ctx.moveTo(point._view.x, point._view.y); | ||
| return; | ||
| // First point moves to it's starting position no matter what | ||
| if (!index) { | ||
| ctx.moveTo(point._view.x, vm.scaleZero); | ||
| } | ||
|
|
||
| // Start Skip and drag along scale baseline | ||
| if (point._view.skip && vm.skipNull && !this._loop) { | ||
| ctx.lineTo(previous._view.x, point._view.y); | ||
| ctx.moveTo(next._view.x, point._view.y); | ||
| // Skip this point, draw to scaleZero, move to next point, and draw to next point | ||
| if (point._view.skip && !this.loop) { | ||
| ctx.lineTo(previous._view.x, vm.scaleZero); | ||
| ctx.moveTo(next._view.x, vm.scaleZero); | ||
| return; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is adding a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I adopted a return fast pattern instead of a bunch of nested if else statements. |
||
| } | ||
| // End Skip Stright line from the base line | ||
| else if (previous._view.skip && vm.skipNull && !this._loop) { | ||
| ctx.moveTo(point._view.x, previous._view.y); | ||
|
|
||
| // The previous line was skipped, so just draw a normal straight line to the point | ||
| if (previous._view.skip) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we consciously losing the distinction between
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I believe so for now. Both features were broken to some extent. I think it's better we go with a solid base. If someone down the line needs to draw falsey values as zeros I think they would probably just scrub their data for that and then expect chartjs to deliver accurately. |
||
| ctx.lineTo(point._view.x, point._view.y); | ||
| return; | ||
| } | ||
|
|
||
| if (previous._view.skip && vm.skipNull) { | ||
| ctx.moveTo(point._view.x, point._view.y); | ||
| } | ||
| // Normal Bezier Curve | ||
| else { | ||
| if (vm.tension > 0) { | ||
| ctx.bezierCurveTo( | ||
| previous._view.controlPointNextX, | ||
| previous._view.controlPointNextY, | ||
| point._view.controlPointPreviousX, | ||
| point._view.controlPointPreviousY, | ||
| point._view.x, | ||
| point._view.y | ||
| ); | ||
| } else { | ||
| ctx.lineTo(point._view.x, point._view.y); | ||
| } | ||
| // Draw a bezier to point | ||
| if (vm.tension > 0 && index) { | ||
| //ctx.lineTo(point._view.x, point._view.y); | ||
| ctx.bezierCurveTo( | ||
| previous._view.controlPointNextX, | ||
| previous._view.controlPointNextY, | ||
| point._view.controlPointPreviousX, | ||
| point._view.controlPointPreviousY, | ||
| point._view.x, | ||
| point._view.y | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| // Draw a straight line to the point | ||
| ctx.lineTo(point._view.x, point._view.y); | ||
|
|
||
| }, this); | ||
|
|
||
| // For radial scales, loop back around to the first point | ||
| if (this._loop) { | ||
| // Draw a bezier line | ||
| if (vm.tension > 0 && !first._view.skip) { | ||
|
|
||
| ctx.bezierCurveTo( | ||
| last._view.controlPointNextX, | ||
| last._view.controlPointNextY, | ||
|
|
@@ -96,9 +95,10 @@ | |
| first._view.x, | ||
| first._view.y | ||
| ); | ||
| } else { | ||
| ctx.lineTo(first._view.x, first._view.y); | ||
| return; | ||
| } | ||
| // Draw a straight line | ||
| ctx.lineTo(first._view.x, first._view.y); | ||
| } | ||
|
|
||
| // If we had points and want to fill this line, do so. | ||
|
|
@@ -130,31 +130,24 @@ | |
| var previous = helpers.previousItem(this._children, index); | ||
| var next = helpers.nextItem(this._children, index); | ||
|
|
||
| // First point only | ||
| if (index === 0) { | ||
| ctx.moveTo(point._view.x, point._view.y); | ||
| return; | ||
| if (!index) { | ||
| ctx.moveTo(point._view.x, vm.scaleZero); | ||
| } | ||
|
|
||
| // Start Skip and drag along scale baseline | ||
| if (point._view.skip && vm.skipNull && !this._loop) { | ||
| ctx.moveTo(previous._view.x, point._view.y); | ||
| ctx.moveTo(next._view.x, point._view.y); | ||
| return; | ||
| } | ||
| // End Skip Stright line from the base line | ||
| if (previous._view.skip && vm.skipNull && !this._loop) { | ||
| ctx.moveTo(point._view.x, previous._view.y); | ||
| ctx.moveTo(point._view.x, point._view.y); | ||
| // Skip this point and move to the next points zeroPoint | ||
| if (point._view.skip && !this.loop) { | ||
| ctx.moveTo(next._view.x, vm.scaleZero); | ||
| return; | ||
| } | ||
|
|
||
| if (previous._view.skip && vm.skipNull) { | ||
| // Previous point was skipped, just move to the point | ||
| if (previous._view.skip) { | ||
| ctx.moveTo(point._view.x, point._view.y); | ||
| return; | ||
| } | ||
| // Normal Bezier Curve | ||
| if (vm.tension > 0) { | ||
|
|
||
| // Draw a bezier line to the point | ||
| if (vm.tension > 0 && index) { | ||
| ctx.bezierCurveTo( | ||
| previous._view.controlPointNextX, | ||
| previous._view.controlPointNextY, | ||
|
|
@@ -163,14 +156,18 @@ | |
| point._view.x, | ||
| point._view.y | ||
| ); | ||
| } else { | ||
| ctx.lineTo(point._view.x, point._view.y); | ||
| return; | ||
| } | ||
|
|
||
| // Draw a straight line to the point | ||
| ctx.lineTo(point._view.x, point._view.y); | ||
|
|
||
| }, this); | ||
|
|
||
| if (this._loop && !first._view.skip) { | ||
| if (vm.tension > 0) { | ||
|
|
||
| // Draw a bezier line to the first point | ||
| if (vm.tension > 0) { | ||
| ctx.bezierCurveTo( | ||
| last._view.controlPointNextX, | ||
| last._view.controlPointNextY, | ||
|
|
@@ -179,14 +176,16 @@ | |
| first._view.x, | ||
| first._view.y | ||
| ); | ||
| } else { | ||
| ctx.lineTo(first._view.x, first._view.y); | ||
| return; | ||
| } | ||
|
|
||
| // Draw a straight line to the first point | ||
| ctx.lineTo(first._view.x, first._view.y); | ||
| } | ||
|
|
||
| ctx.stroke(); | ||
| ctx.restore(); | ||
| }, | ||
| }); | ||
|
|
||
| }).call(this); | ||
| }).call(this); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it ok to lose the
return?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah