hadn't considered case when y axis range is fully negative

separated the code for aligning axes into function alignYAxisRanges to reduce cyclomatic complexity
dc-js · Jul 28, 2016 · a1510ca · a1510ca
1 parent aa67976
commit a1510ca
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 20 deletions.
diff --git a/spec/composite-chart-spec.js b/spec/composite-chart-spec.js
@@ -1,14 +1,15 @@
 /* global appendChartID, loadDateFixture, makeDate */
 describe('dc.compositeChart', function () {
     var id, chart, data, dateDimension, dateValueSumGroup, dateValueNegativeSumGroup,
-        dateIdSumGroup, dateGroup;
+        dateIdSumGroup, dateIdNegativeSumGroup, dateGroup;
 
     beforeEach(function () {
         data = crossfilter(loadDateFixture());
         dateDimension = data.dimension(function (d) { return d3.time.day.utc(d.dd); });
         dateValueSumGroup = dateDimension.group().reduceSum(function (d) { return d.value; });
         dateValueNegativeSumGroup = dateDimension.group().reduceSum(function (d) { return -d.value; });
         dateIdSumGroup = dateDimension.group().reduceSum(function (d) { return d.id; });
+        dateIdNegativeSumGroup = dateDimension.group().reduceSum(function (d) { return -d.id; });
         dateGroup = dateDimension.group();
 
         id = 'composite-chart';
@@ -598,9 +599,9 @@ describe('dc.compositeChart', function () {
                 chart
                     .compose([
                         leftChart = dc.barChart(chart)
-                            .group(dateIdSumGroup, 'Date Value Group'),
+                            .group(dateIdSumGroup, 'Date ID Group'),
                         rightChart = dc.lineChart(chart)
-                            .group(dateValueNegativeSumGroup, 'Date ID Group')
+                            .group(dateValueNegativeSumGroup, 'Date Value Group')
                             .useRightYAxis(true)
                     ])
                     .render();
@@ -621,6 +622,35 @@ describe('dc.compositeChart', function () {
                 });
             });
         });
+
+        describe('when composing left and right axes charts with negative values', function () {
+          var leftChart, rightChart;
+          beforeEach(function () {
+              chart
+                  .compose([
+                      leftChart = dc.barChart(chart)
+                          .group(dateIdNegativeSumGroup, 'Date ID Group'),
+                      rightChart = dc.lineChart(chart)
+                          .group(dateValueNegativeSumGroup, 'Date Value Group')
+                          .useRightYAxis(true)
+                  ])
+                  .render();
+          });
+
+          it('the axis baselines shouldn\'t match', function () {
+              expect(leftChart.y()(0)).not.toEqual(rightChart.y()(0));
+          });
+
+          describe('with alignYAxes', function () {
+              beforeEach(function () {
+                  chart.alignYAxes(true)
+                      .render();
+              });
+              it('the axis baselines should match', function () {
+                  expect(leftChart.y()(0)).toEqual(rightChart.y()(0));
+              });
+          });
+      });
     });
 
     describe('sub-charts with different filter types', function () {

diff --git a/src/composite-chart.js b/src/composite-chart.js
@@ -117,27 +117,45 @@ dc.compositeChart = function (parent, chartGroup) {
         }
 
         if (_chart.alignYAxes() && left && right && (lyAxisMin < 0 || ryAxisMin < 0)) {
-            // both y axis are linear and at least one doesn't start at zero
-            var leftYRatio, rightYRatio;
+            return alignYAxisRanges(lyAxisMin, lyAxisMax, ryAxisMin, ryAxisMax);
+        }
+        return {
+            lyAxisMin: lyAxisMin,
+            lyAxisMax: lyAxisMax,
+            ryAxisMin: ryAxisMin,
+            ryAxisMax: ryAxisMax
+        };
+    }
 
-            if (lyAxisMin < 0) {
-                leftYRatio = lyAxisMax / lyAxisMin;
-            }
+    function alignYAxisRanges (lyAxisMin, lyAxisMax, ryAxisMin, ryAxisMax) {
+        // both y axis are linear and at least one doesn't start at zero
+        var leftYRatio, rightYRatio;
 
-            if (ryAxisMin < 0) {
-                rightYRatio = ryAxisMax / ryAxisMin;
-            }
+        if (lyAxisMin < 0) {
+            leftYRatio = lyAxisMax / lyAxisMin;
+        }
 
-            if (lyAxisMin < 0 && ryAxisMin < 0) {
-                if (leftYRatio < rightYRatio) {
-                    ryAxisMax = ryAxisMin * leftYRatio;
-                } else {
-                    lyAxisMax = lyAxisMin * rightYRatio;
-                }
-            } else if (lyAxisMin < 0) {
-                ryAxisMin = ryAxisMax / leftYRatio;
+        if (ryAxisMin < 0) {
+            rightYRatio = ryAxisMax / ryAxisMin;
+        }
+
+        if (lyAxisMin < 0 && ryAxisMin < 0) {
+            if (leftYRatio < rightYRatio) {
+                ryAxisMax = ryAxisMin * leftYRatio;
             } else {
-                lyAxisMin = lyAxisMax / (ryAxisMax / ryAxisMin);
+                lyAxisMax = lyAxisMin * rightYRatio;
+            }
+        } else if (lyAxisMin < 0) {
+            ryAxisMin = ryAxisMax / leftYRatio;
+            if (lyAxisMax < 0) {
+                lyAxisMax = -lyAxisMax;
+                ryAxisMin = -ryAxisMin;
+            }
+        } else {
+            lyAxisMin = lyAxisMax / rightYRatio;
+            if (ryAxisMax < 0) {
+                ryAxisMax = -ryAxisMax;
+                lyAxisMin = -lyAxisMin;
             }
         }
         return {