feat(encodable): handle edge cases when making domain includes zero (#…

…257)

* feat: handle edge cases when making domain includes zero

* test: remove redundant tests

superset-frontend/temporary_superset_ui/superset-ui/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);
   }
 }

superset-frontend/temporary_superset_ui/superset-ui/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]);
+  });
+});

superset-frontend/temporary_superset_ui/superset-ui/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({