Allow selected points where canvas-y coordinate is 0 #692
Conversation
|
Not sure why this failed. The tests all ran and passed but some post test step failed.
|
| @@ -1815,7 +1815,7 @@ Dygraph.prototype.updateSelection_ = function(opt_animFraction) { | |||
| ctx.save(); | |||
| for (i = 0; i < this.selPoints_.length; i++) { | |||
| var pt = this.selPoints_[i]; | |||
| if (!utils.isOK(pt.canvasy)) continue; | |||
| if (isNaN(pt.canvasy) || pt.canvasy === null) continue; | |||
There was a problem hiding this comment.
I think you also want to check for undefined here (and in legend.js), unless there's a reason for that to be excluded?
This returns true unless the parameter is 0, null, undefined or NaN.
There was a problem hiding this comment.
Ok. I actually have a question about that, what is the distinction between undefined and null for canvas points?
There was a problem hiding this comment.
To be honest, I don't see how either would happen. They get set in dygraph-canvas.js:
point.canvasx = this.area.w * point.x + this.area.x;
point.canvasy = this.area.h * point.y + this.area.y;If point.x is null, then we get:
> 600 * null
0
whereas
> 600 * undefined
NaN
this is... strange. But since it never comes out to null or undefined, I'd guess that those checks aren't necessary.
|
Any reason for chmoding the test files? (They go from 644→755.) |
|
No reason for the chmod. I'll change them back. It's something that my editor or VM is doing and I let it slip through accidentally. |
This test demonstrates how updateSelection_ and generateLegendHTML were ignoring points whose canvas-y value was 0. The next commit will fix these breaking tests
The check for `utils.isOK` blacklists the value 0. In the context of validating selected points and reporting them in the legend, any point where canvas-y zero should be included.
|
Try pulling |
musicist288
force-pushed
the
zero-coordinate-bugs
branch
from
a25c5c7 to
7454418
Compare
|
Ok thanks. That seemed to work. I also updated the request with the reverted file modes and the check for undefined. |
|
Can you try dropping the |
|
Ok. Updated and tests pass. |
|
Perfect, thanks! We'll see if anyone complains. |
Allow selected points where canvas-y coordinate is 0
As discussed in #691
The check for
utils.isOKblacklists the value 0. In the context ofvalidating selected points and reporting them in the legend, any point
where canvas-y zero should be included.