Include 0 as max in elastic axis if all values are negative #1156
Conversation
sebgrohn
changed the title
sebgrohn
changed the title
|
@sebgrohn, this looks like an excellent approach. I like the symmetry of it. Would you be willing to also write/improve a few tests for this? In particular, the row chart already has tests for all-negative values, but it looks like it doesn't test The bar chart apparently does test elasticY with negative values, but it didn't detect any problems. Ditto for the line chart. Don't worry about |
|
Great! I will try to produce some tests for negative values with elasticY enabled. I can't give any time frame right now, though. |
sebgrohn
force-pushed
the
fix-only-negative-values
branch
from
93df36f
to
198f25b
Compare
…values negative Use the same algorithm as for the y axis minimum in stackMixin, so that zero is always included in the y axis domain.
…are negative Do the same check in rowChart for the x axis maximum as for the minimum, so that zero is always included in the x axis domain.
sebgrohn
force-pushed
the
fix-only-negative-values
branch
from
198f25b
to
b05902c
Compare
|
Added tests for Row, Bar, and Line chart that checks for the correct x/y axis ticks when Notes:
@gordonwoodhull and others: comments on this? |
gordonwoodhull
changed the title
| it('should generate x axis domain dynamically', function () { | ||
| var nthText = function (n) { return d3.select(chart.selectAll('g.axis .tick text')[0][n]); }; | ||
|
|
||
| for (let i = 0; i < xAxisTicks.length; i++) { |
There was a problem hiding this comment.
Crazy... all your code works well, but for some reason the let here caused Jasmine to skip the row chart tests! Completely silent failure!
dc.js is still ES5 and will be for a while yet. @sebgrohn, did you have to do anything special to get the tests to run?
Of course, I noticed 58 commits later that my test count was way down. Easy enough to fix, but whoah that's a little scary if one ES6 keyword causes a file to get dropped entirely.
There was a problem hiding this comment.
Wow, that's interesting (and scary)! Sloppy to let a let (sic.) slip in there; I am truly ES6 damaged.
I don't remember if I did something out of the ordinary, and I have the code and dev env set up on another computer I can't access right now... :o
|
Thanks @sebgrohn, rebased and merged in 2.0 beta 32! |
|
Great! |
For Row chart and charts using Stack mixin, use the corresponding method for determining the max value for the y axis as used for min value:
Should fix issue #879.
Two Jasmine tests are currently failing, both related to Composite chart and its
alignYAxes. As far as I can tell, it seems to be related to the calculations incalculateYAxisRangesthat does something magic with the y axis ranges if there are two y axes andalignYAxesare set. I am at loss here what to do, all suggestions are welcome!jasmine-failing-tests.txt