From 1fc3ca0a56b624184bf48828679d38fc5703d34f Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Fri, 15 Nov 2019 11:42:46 -0800
Subject: [PATCH]  feat(encodable): handle edge cases when making domain
 includes zero (#257)

* feat: handle edge cases when making domain includes zero

* test: remove redundant tests
---
 .../src/parsers/scale/applyZero.ts            | 13 +++++--
 .../test/parsers/scale/applyZero.test.ts      | 38 +++++++++++++++++++
 .../scale/createScaleFromScaleConfig.test.ts  | 18 ---------
 3 files changed, 47 insertions(+), 22 deletions(-)
 create mode 100644 packages/superset-ui-encodable/test/parsers/scale/applyZero.test.ts

diff --git a/packages/superset-ui-encodable/src/parsers/scale/applyZero.ts b/packages/superset-ui-encodable/src/parsers/scale/applyZero.ts
index eea0b731d579..005f9a1eb2e7 100644
--- a/packages/superset-ui-encodable/src/parsers/scale/applyZero.ts
+++ b/packages/superset-ui-encodable/src/parsers/scale/applyZero.ts
@@ -1,12 +1,17 @@
 import { Value } from '../../types/VegaLite';
-import { ScaleConfig, D3Scale, ContinuousD3Scale } from '../../types/Scale';
+import { ScaleConfig, D3Scale } from '../../types/Scale';
+import { isContinuousScale } from '../../typeGuards/Scale';
 
 export default function applyZero<Output extends Value>(
   config: ScaleConfig<Output>,
   scale: D3Scale<Output>,
 ) {
-  if ('zero' in config && config.zero === true) {
-    const [min, max] = (scale as ContinuousD3Scale<Output>).domain() as number[];
-    scale.domain([Math.min(0, min), Math.max(0, max)]);
+  if ('zero' in config && config.zero === true && isContinuousScale(scale, config.type)) {
+    const domain = scale.domain() as number[];
+    const [a, b] = domain;
+    const isDescending = b < a;
+    const [min, max] = isDescending ? [b, a] : [a, b];
+    const domainWithZero = [Math.min(0, min), Math.max(0, max)];
+    scale.domain(isDescending ? domainWithZero.reverse() : domainWithZero);
   }
 }
diff --git a/packages/superset-ui-encodable/test/parsers/scale/applyZero.test.ts b/packages/superset-ui-encodable/test/parsers/scale/applyZero.test.ts
new file mode 100644
index 000000000000..9d0c85fdabca
--- /dev/null
+++ b/packages/superset-ui-encodable/test/parsers/scale/applyZero.test.ts
@@ -0,0 +1,38 @@
+import { scaleLinear } from 'd3-scale';
+import applyZero from '../../../src/parsers/scale/applyZero';
+
+describe('applyZero()', () => {
+  describe('zero = true', () => {
+    it('positive domain', () => {
+      const scale = scaleLinear().domain([2, 10]);
+      applyZero({ type: 'linear', zero: true }, scale);
+      expect(scale.domain()).toEqual([0, 10]);
+    });
+    it('negative domain', () => {
+      const scale = scaleLinear().domain([-10, -2]);
+      applyZero({ type: 'linear', zero: true }, scale);
+      expect(scale.domain()).toEqual([-10, 0]);
+    });
+    it('domain contains zero', () => {
+      const scale = scaleLinear().domain([-5, 5]);
+      applyZero({ type: 'linear', zero: true }, scale);
+      expect(scale.domain()).toEqual([-5, 5]);
+    });
+    it('descending domain', () => {
+      const scale = scaleLinear().domain([5, 1]);
+      applyZero({ type: 'linear', zero: true }, scale);
+      expect(scale.domain()).toEqual([5, 0]);
+    });
+    it('descending negative domain', () => {
+      const scale = scaleLinear().domain([-1, -5]);
+      applyZero({ type: 'linear', zero: true }, scale);
+      expect(scale.domain()).toEqual([0, -5]);
+    });
+  });
+
+  it('zero = false', () => {
+    const scale = scaleLinear().domain([2, 10]);
+    applyZero({ type: 'linear', zero: false }, scale);
+    expect(scale.domain()).toEqual([2, 10]);
+  });
+});
diff --git a/packages/superset-ui-encodable/test/parsers/scale/createScaleFromScaleConfig.test.ts b/packages/superset-ui-encodable/test/parsers/scale/createScaleFromScaleConfig.test.ts
index 779a9a2e82ed..ee3e3416f8c3 100644
--- a/packages/superset-ui-encodable/test/parsers/scale/createScaleFromScaleConfig.test.ts
+++ b/packages/superset-ui-encodable/test/parsers/scale/createScaleFromScaleConfig.test.ts
@@ -175,24 +175,6 @@ describe('createScaleFromScaleConfig(config)', () => {
       });
       expect(scale(5)).toEqual(5);
     });
-    it('with zero (negative domain)', () => {
-      const scale = createScaleFromScaleConfig({
-        type: 'linear',
-        domain: [-10, -2],
-        range: [0, 10],
-        zero: true,
-      });
-      expect(scale(-5)).toEqual(5);
-    });
-    it('with zero (no effect)', () => {
-      const scale = createScaleFromScaleConfig({
-        type: 'linear',
-        domain: [-5, 5],
-        range: [0, 10],
-        zero: true,
-      });
-      expect(scale(0)).toEqual(5);
-    });
     it('with interpolate', () => {
       expect(() =>
         createScaleFromScaleConfig({