Minor refactor:

- Undo encodingHasRangeChannels - Set `true` for continuous size scale regardless to config - Fix doc
vega · Aug 16, 2022 · f384cc4 · f384cc4
1 parent c07e532
commit f384cc4
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 65 deletions.
diff --git a/build/vega-lite-schema.json b/build/vega-lite-schema.json
@@ -21610,7 +21610,7 @@
           "description": "Reverse x-scale by default (useful for right-to-left charts)."
         },
         "zero": {
-          "description": "Default for ensuring that a zero baseline values for [`quantize`](https://vega.github.io/vega-lite/docs/scale.html#quantize) scale.\n\n\n__Default value:__ `true`",
+          "description": "Override the default `scale.zero` for [`continuous`](https://vega.github.io/vega-lite/docs/scale.html#continuous) scales except for (1) x/y-scales of non-ranged bar or area charts and (2) size scales.\n\n__Default value:__ `true`",
           "type": "boolean"
         }
       },

diff --git a/site/docs/encoding/scale.md b/site/docs/encoding/scale.md
@@ -444,15 +444,15 @@ To provide themes for all scales, the scale config (`config: {scale: {...}}`) ca
 
 #### Padding
 
-{% include table.html props="bandPaddingInner,barBandPaddingInner,rectBandPaddingInner,bandWithNestedOffsetPaddingInner,offsetBandPaddingInner,bandPaddingOuter,bandWithNestedOffsetPaddingOuter,offsetBandPaddingOuter,continuousPadding,pointPadding,zero" source="ScaleConfig" %}
+{% include table.html props="bandPaddingInner,barBandPaddingInner,rectBandPaddingInner,bandWithNestedOffsetPaddingInner,offsetBandPaddingInner,bandPaddingOuter,bandWithNestedOffsetPaddingOuter,offsetBandPaddingOuter,continuousPadding,pointPadding" source="ScaleConfig" %}
 
 #### Range
 
 {% include table.html props="maxBandSize,minBandSize,maxFontSize,minFontSize,maxOpacity,minOpacity,maxSize,minSize,maxStrokeWidth,minStrokeWidth" source="ScaleConfig" %}
 
 #### Other
 
-{% include table.html props="clamp,round,xReverse,useUnaggregatedDomain" source="ScaleConfig" %}
+{% include table.html props="clamp,round,xReverse,useUnaggregatedDomain,zero" source="ScaleConfig" %}
 
 {:#range-config}
 

diff --git a/src/compile/scale/properties.ts b/src/compile/scale/properties.ts
@@ -4,6 +4,7 @@ import {isBinned, isBinning, isBinParams} from '../../bin';
 import {
   COLOR,
   FILL,
+  getSecondaryRangeChannel,
   isXorY,
   isXorYOffset,
   POLAR_POSITION_SCALE_CHANNELS,
@@ -22,7 +23,7 @@ import {
 } from '../../channeldef';
 import {Config} from '../../config';
 import {isDateTime} from '../../datetime';
-import {channelHasNestedOffsetScale, encodingHasRangeChannels} from '../../encoding';
+import {channelHasNestedOffsetScale} from '../../encoding';
 import * as log from '../../log';
 import {Mark, MarkDef, RectConfig} from '../../mark';
 import {
@@ -121,7 +122,7 @@ function parseUnitScaleProperty(model: UnitModel, property: Exclude<keyof (Scale
                 markDef,
                 config,
                 hasNestedOffsetScale: channelHasNestedOffsetScale(encoding, channel),
-                hasRangeChannels: encodingHasRangeChannels(encoding)
+                hasSecondaryRangeChannel: !!encoding[getSecondaryRangeChannel(channel)]
               })
             : config.scale[property];
         if (value !== undefined) {
@@ -145,7 +146,7 @@ export interface ScaleRuleParams {
   domainMax: Scale['domainMax'];
   markDef: MarkDef<Mark, SignalRef>;
   config: Config<SignalRef>;
-  hasRangeChannels: boolean;
+  hasSecondaryRangeChannel: boolean;
 }
 
 export const scaleRules: {
@@ -171,8 +172,8 @@ export const scaleRules: {
     const sort = isFieldDef(fieldOrDatumDef) ? fieldOrDatumDef.sort : undefined;
     return reverse(scaleType, sort, channel, config.scale);
   },
-  zero: ({channel, fieldOrDatumDef, domain, markDef, scaleType, config, hasRangeChannels}) =>
-    zero(channel, fieldOrDatumDef, domain, markDef, scaleType, config.scale, hasRangeChannels)
+  zero: ({channel, fieldOrDatumDef, domain, markDef, scaleType, config, hasSecondaryRangeChannel}) =>
+    zero(channel, fieldOrDatumDef, domain, markDef, scaleType, config.scale, hasSecondaryRangeChannel)
 };
 
 // This method is here rather than in range.ts to avoid circular dependency.
@@ -403,7 +404,7 @@ export function zero(
   markDef: MarkDef,
   scaleType: ScaleType,
   scaleConfig: ScaleConfig<SignalRef>,
-  hasRangeChannels: boolean
+  hasSecondaryRangeChannel: boolean
 ) {
   // If users explicitly provide a domain, we should not augment zero as that will be unexpected.
   const hasCustomDomain = !!specifiedDomain && specifiedDomain !== 'unaggregated';
@@ -422,16 +423,14 @@ export function zero(
     }
   }
 
-  const defaultZero = scaleConfig?.zero === undefined ? true : scaleConfig?.zero;
-
   // If there is no custom domain, return configZero value (=`true` as default) only for the following cases:
 
   // 1) using quantitative field with size
   // While this can be either ratio or interval fields, our assumption is that
   // ratio are more common. However, if the scaleType is discretizing scale, we want to return
   // false so that range doesn't start at zero
   if (channel === 'size' && fieldDef.type === 'quantitative' && !isContinuousToDiscrete(scaleType)) {
-    return defaultZero;
+    return true;
   }
 
   // 2) non-binned, quantitative x-scale or y-scale
@@ -448,11 +447,12 @@ export function zero(
       }
     }
 
-    if (contains(['bar', 'area'], type) && !hasRangeChannels) {
+    if (contains(['bar', 'area'], type) && !hasSecondaryRangeChannel) {
       return true;
     }
 
-    return defaultZero;
+    return scaleConfig?.zero;
   }
+
   return false;
 }
diff --git a/src/encoding.ts b/src/encoding.ts
@@ -12,7 +12,6 @@ import {
   FILL,
   FILLOPACITY,
   getMainChannelFromOffsetChannel,
-  getMainRangeChannel,
   getOffsetScaleChannel,
   HREF,
   isChannel,
@@ -29,7 +28,6 @@ import {
   ORDER,
   RADIUS,
   RADIUS2,
-  SECONDARY_RANGE_CHANNEL,
   SHAPE,
   SIZE,
   STROKE,
@@ -371,15 +369,6 @@ export function channelHasNestedOffsetScale<F extends Field>(
   }
   return false;
 }
-export function encodingHasRangeChannels<F extends Field>(encoding: EncodingWithFacet<F>): boolean {
-  return some(SECONDARY_RANGE_CHANNEL, channel => {
-    if (channelHasField(encoding, channel)) {
-      const mainRangeChannel = getMainRangeChannel(channel);
-      return channelHasField(encoding, mainRangeChannel);
-    }
-    return false;
-  });
-}
 
 export function isAggregate(encoding: EncodingWithFacet<any>) {
   return some(CHANNELS, channel => {

diff --git a/src/scale.ts b/src/scale.ts
@@ -417,7 +417,7 @@ export interface ScaleConfig<ES extends ExprRef | SignalRef> {
   xReverse?: boolean | ES;
 
   /**
-   * Override the default `scale.zero` for [`continuous`](https://vega.github.io/vega-lite/docs/scale.html#continuous) scale except for non-ranged bar or area charts.
+   * Override the default `scale.zero` for [`continuous`](https://vega.github.io/vega-lite/docs/scale.html#continuous) scales except for (1) x/y-scales of non-ranged bar or area charts and (2) size scales.
    *
    * __Default value:__ `true`
    *
@@ -447,7 +447,9 @@ export const defaultScaleConfig: ScaleConfig<SignalRef> = {
   minStrokeWidth: 1,
   maxStrokeWidth: 4,
   quantileCount: 4,
-  quantizeCount: 4
+  quantizeCount: 4,
+
+  zero: true
 };
 
 export interface SchemeParams {

diff --git a/test/compile/scale/properties.test.ts b/test/compile/scale/properties.test.ts
@@ -199,10 +199,10 @@ describe('compile/scale', () => {
   });
 
   describe('zero', () => {
-    it('should return true when mapping a quantitative field to x with scale.domain = "unaggregated"', () => {
+    it('should return default (undefined) when mapping a quantitative field to x with scale.domain = "unaggregated"', () => {
       expect(
         rules.zero('x', {field: 'a', type: 'quantitative'}, 'unaggregated', {type: 'point'}, 'linear', undefined, false)
-      ).toBeTruthy();
+      ).toBeUndefined();
     });
 
     it('should return true when mapping a quantitative field to size', () => {
@@ -217,7 +217,7 @@ describe('compile/scale', () => {
       ).toBeTruthy();
     });
 
-    it('should return true when mapping a non-binned quantitative field to x/y of point', () => {
+    it('should return default (undefined) when mapping a non-binned quantitative field to x/y of point', () => {
       for (const channel of ['x', 'y'] as const) {
         expect(
           rules.zero(
@@ -229,7 +229,7 @@ describe('compile/scale', () => {
             undefined,
             false
           )
-        ).toBeTruthy();
+        ).toBeUndefined();
       }
     });
 
@@ -312,18 +312,6 @@ describe('compile/scale', () => {
         ).toBe(configZero);
       }
 
-      expect(
-        rules.zero(
-          'size',
-          {field: 'a', type: 'quantitative'},
-          undefined,
-          {type: 'point'},
-          'linear',
-          {zero: configZero},
-          false
-        )
-      ).toBe(configZero);
-
       expect(
         rules.zero(
           'size',
@@ -342,7 +330,7 @@ describe('compile/scale', () => {
       ).toBe(configZero);
     });
 
-    it(`should return true for non-ranged are/bar chart regardless to config`, () => {
+    it(`should return true for x/y scales of the non-ranged are/bar charts regardless to config`, () => {
       for (const mark of [BAR, AREA]) {
         for (const channel of ['x', 'y'] as const) {
           expect(
@@ -359,5 +347,19 @@ describe('compile/scale', () => {
         }
       }
     });
+
+    it(`should return true for the continuous & quantitative size scale regardless to config`, () => {
+      expect(
+        rules.zero(
+          'size',
+          {field: 'a', type: 'quantitative'},
+          undefined,
+          {type: 'point'},
+          'linear',
+          {zero: false},
+          false
+        )
+      ).toBe(true);
+    });
   });
 });
diff --git a/test/encoding.test.ts b/test/encoding.test.ts
@@ -14,7 +14,6 @@ import {isPositionFieldOrDatumDef} from '../src/channeldef';
 import {defaultConfig} from '../src/config';
 import {
   Encoding,
-  encodingHasRangeChannels,
   extractTransformsFromEncoding,
   fieldDefs,
   initEncoding,
@@ -545,24 +544,4 @@ describe('encoding', () => {
       ]);
     });
   });
-
-  describe('encodingHasRangeChannels', () => {
-    it('should return true if encoding has a pair of range channels', () => {
-      expect(
-        encodingHasRangeChannels({
-          x: {field: 'foo', type: 'quantitative'},
-          x2: {field: 'boo', type: 'quantitative'}
-        })
-      ).toBeTruthy();
-    });
-
-    it('should return false if encoding does not have a pair of range channels', () => {
-      expect(
-        encodingHasRangeChannels({
-          x: {field: 'foo', type: 'quantitative'},
-          y2: {field: 'boo', type: 'quantitative'}
-        })
-      ).toBeFalsy();
-    });
-  });
 });