fix: better adapt tick width dimension with binned fields

(by sharing the code for tick's dimension axis with rectangle/bar) Also convert `RectBinSpacingMixins` into `BandSizeConfigMixins`
vega · Nov 16, 2023 · 7b999db · 7b999db
1 parent e71751e
commit 7b999db
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 64 deletions.
diff --git a/src/compile/mark/encode/position-rect.ts b/src/compile/mark/encode/position-rect.ts
@@ -50,7 +50,9 @@ export function rectPosition(model: UnitModel, channel: 'x' | 'y' | 'theta' | 'r
 
   const offsetScaleChannel = getOffsetChannel(channel);
 
-  const isBarBand = mark === 'bar' && (channel === 'x' ? orient === 'vertical' : orient === 'horizontal');
+  const isBandChannel =
+    (mark === 'bar' && (channel === 'x' ? orient === 'vertical' : orient === 'horizontal')) ||
+    (mark === 'tick' && (channel === 'x' ? orient === 'horizontal' : orient === 'vertical'));
 
   // x, x2, and width -- we must specify two of these in all conditions
   if (
@@ -66,7 +68,7 @@ export function rectPosition(model: UnitModel, channel: 'x' | 'y' | 'theta' | 'r
       channel,
       model
     });
-  } else if (((isFieldOrDatumDef(channelDef) && hasDiscreteDomain(scaleType)) || isBarBand) && !channelDef2) {
+  } else if (((isFieldOrDatumDef(channelDef) && hasDiscreteDomain(scaleType)) || isBandChannel) && !channelDef2) {
     return positionAndSize(channelDef, channel, model);
   } else {
     return rangePosition(channel, model, {defaultPos: 'zeroOrMax', defaultPos2: 'zeroOrMin'});
@@ -136,7 +138,7 @@ function positionAndSize(
   channel: 'x' | 'y' | 'theta' | 'radius',
   model: UnitModel
 ) {
-  const {markDef, encoding, config, stack} = model;
+  const {mark, markDef, encoding, config, stack} = model;
   const orient = markDef.orient;
 
   const scaleName = model.scaleName(channel);
@@ -149,7 +151,11 @@ function positionAndSize(
   const offsetScale = model.getScaleComponent(getOffsetScaleChannel(channel));
 
   // use "size" channel for bars, if there is orient and the channel matches the right orientation
-  const useVlSizeChannel = (orient === 'horizontal' && channel === 'y') || (orient === 'vertical' && channel === 'x');
+  const useVlSizeChannel =
+    mark === 'tick'
+      ? // tick's orientation is opposite to other marks
+        (orient === 'vertical' && channel === 'y') || (orient === 'horizontal' && channel === 'x')
+      : (orient === 'horizontal' && channel === 'y') || (orient === 'vertical' && channel === 'x');
 
   // Use size encoding / mark property / config if it exists
   let sizeMixins;

diff --git a/src/compile/mark/tick.ts b/src/compile/mark/tick.ts
@@ -1,7 +1,3 @@
-import type {SignalRef} from 'vega';
-import {isNumber} from 'vega-util';
-import {getViewConfigDiscreteStep} from '../../config';
-import {isVgRangeStep} from '../../vega.schema';
 import {getMarkPropOrConfig, signalOrValueRef} from '../common';
 import {UnitModel} from '../unit';
 import {MarkCompiler} from './base';
@@ -14,10 +10,9 @@ export const tick: MarkCompiler = {
     const {config, markDef} = model;
     const orient = markDef.orient;
 
-    const vgSizeChannel = orient === 'horizontal' ? 'width' : 'height';
     const vgThicknessChannel = orient === 'horizontal' ? 'height' : 'width';
 
-    return {
+    const baseAndThickness = {
       ...encode.baseEncodeEntry(model, {
         align: 'ignore',
         baseline: 'ignore',
@@ -26,40 +21,22 @@ export const tick: MarkCompiler = {
         size: 'ignore',
         theta: 'ignore'
       }),
-
-      ...encode.pointPosition('x', model, {defaultPos: 'mid', vgChannel: 'xc'}),
-      ...encode.pointPosition('y', model, {defaultPos: 'mid', vgChannel: 'yc'}),
-
-      // size / thickness => width / height
-      ...encode.nonPosition('size', model, {
-        defaultValue: defaultSize(model),
-        vgChannel: vgSizeChannel
-      }),
       [vgThicknessChannel]: signalOrValueRef(getMarkPropOrConfig('thickness', markDef, config))
     };
-  }
-};
-
-function defaultSize(model: UnitModel): number | SignalRef {
-  const {config, markDef} = model;
-  const {orient} = markDef;
 
-  const vgSizeChannel = orient === 'horizontal' ? 'width' : 'height';
-  const scale = model.getScaleComponent(orient === 'horizontal' ? 'x' : 'y');
-
-  const markPropOrConfig =
-    getMarkPropOrConfig('size', markDef, config, {vgChannel: vgSizeChannel}) ?? config.tick.bandSize;
-
-  if (markPropOrConfig !== undefined) {
-    return markPropOrConfig;
-  } else {
-    const scaleRange = scale ? scale.get('range') : undefined;
-    if (scaleRange && isVgRangeStep(scaleRange) && isNumber(scaleRange.step)) {
-      return (scaleRange.step * 3) / 4;
+    if (orient === 'horizontal') {
+      return {
+        ...baseAndThickness,
+        ...encode.rectPosition(model, 'x'),
+        ...encode.pointPosition('y', model, {defaultPos: 'mid', vgChannel: 'yc'})
+      };
+    } else {
+      // vertical
+      return {
+        ...baseAndThickness,
+        ...encode.pointPosition('x', model, {defaultPos: 'mid', vgChannel: 'xc'}),
+        ...encode.rectPosition(model, 'y')
+      };
     }
-
-    const defaultViewStep = getViewConfigDiscreteStep(config.view, vgSizeChannel);
-
-    return (defaultViewStep * 3) / 4;
   }
-}
+};
diff --git a/src/compile/scale/type.ts b/src/compile/scale/type.ts
@@ -77,7 +77,7 @@ function defaultType(
       }
 
       if (isXorY(channel) || isXorYOffset(channel)) {
-        if (util.contains(['rect', 'bar', 'image', 'rule'], mark.type)) {
+        if (util.contains(['rect', 'bar', 'image', 'tick', 'rule'], mark.type)) {
           // The rect/bar mark should fit into a band.
           // For rule, using band scale to make rule align with axis ticks better https://github.com/vega/vega-lite/issues/3429
           return 'band';

diff --git a/src/mark.ts b/src/mark.ts
@@ -50,7 +50,7 @@ export function isPathMark(m: Mark | CompositeMark): m is 'line' | 'area' | 'tra
 }
 
 export function isRectBasedMark(m: Mark | CompositeMark): m is 'rect' | 'bar' | 'image' | 'arc' {
-  return ['rect', 'bar', 'image', 'arc' /* arc is rect/interval in polar coordinate */].includes(m);
+  return ['rect', 'bar', 'image', 'arc', 'tick' /* arc is rect/interval in polar coordinate */].includes(m);
 }
 
 export const PRIMITIVE_MARKS = new Set(keys(Mark));
@@ -284,17 +284,6 @@ export interface MarkConfig<ES extends ExprRef | SignalRef>
   outerRadius?: number | ES;
 }
 
-export interface RectBinSpacingMixins {
-  /**
-   * Offset between bars for binned field. The ideal value for this is either 0 (preferred by statisticians) or 1 (Vega-Lite default, D3 example style).
-   *
-   * __Default value:__ `1`
-   *
-   * @minimum 0
-   */
-  binSpacing?: number;
-}
-
 export type AnyMark = CompositeMark | CompositeMarkDef | Mark | MarkDef;
 
 export function isMarkDef(mark: string | GenericMarkDef<any>): mark is GenericMarkDef<any> {
@@ -333,14 +322,23 @@ const VL_ONLY_MARK_CONFIG_INDEX: Flag<keyof VLOnlyMarkConfig<any>> = {
 
 export const VL_ONLY_MARK_CONFIG_PROPERTIES = keys(VL_ONLY_MARK_CONFIG_INDEX);
 
+const VL_ONLY_BAND_SIZE_CONFIG_MIXINS_INDEX: Flag<keyof BandSizeConfigMixins<any>> = {
+  binSpacing: 1,
+  continuousBandSize: 1,
+  discreteBandSize: 1,
+  minBandSize: 1
+};
+
+const VL_ONLY_BAND_SIZE_CONFIG_MIXINS_PROPS = keys(VL_ONLY_BAND_SIZE_CONFIG_MIXINS_INDEX);
+
 export const VL_ONLY_MARK_SPECIFIC_CONFIG_PROPERTY_INDEX: {
   [k in Mark]?: (keyof Required<MarkConfigMixins<any>>[k])[];
 } = {
   area: ['line', 'point'],
-  bar: ['binSpacing', 'continuousBandSize', 'discreteBandSize', 'minBandSize'],
-  rect: ['binSpacing', 'continuousBandSize', 'discreteBandSize', 'minBandSize'],
+  bar: VL_ONLY_BAND_SIZE_CONFIG_MIXINS_PROPS,
+  rect: VL_ONLY_BAND_SIZE_CONFIG_MIXINS_PROPS,
   line: ['point'],
-  tick: ['bandSize', 'thickness']
+  tick: ['bandSize', 'thickness', ...VL_ONLY_BAND_SIZE_CONFIG_MIXINS_PROPS]
 };
 
 export const defaultMarkConfig: MarkConfig<SignalRef> = {
@@ -427,7 +425,9 @@ const MARK_CONFIG_INDEX: Flag<keyof MarkConfigMixins<any>> = {
 
 export const MARK_CONFIGS = keys(MARK_CONFIG_INDEX);
 
-export interface RectConfig<ES extends ExprRef | SignalRef> extends RectBinSpacingMixins, MarkConfig<ES> {
+export type RectConfig<ES extends ExprRef | SignalRef> = MarkConfig<ES> & BandSizeConfigMixins<ES>;
+
+interface BandSizeConfigMixins<ES extends ExprRef | SignalRef> {
   /**
    * The default size of the bars on continuous scales.
    *
@@ -448,6 +448,15 @@ export interface RectConfig<ES extends ExprRef | SignalRef> extends RectBinSpaci
    * __Default value:__ `0.25`
    */
   minBandSize?: number | ES;
+
+  /**
+   * Offset between bars for binned field. The ideal value for this is either 0 (preferred by statisticians) or 1 (Vega-Lite default, D3 example style).
+   *
+   * __Default value:__ `1`
+   *
+   * @minimum 0
+   */
+  binSpacing?: number;
 }
 
 export type BandSize = number | RelativeBandSize | SignalRef;
@@ -667,7 +676,10 @@ export const defaultRectConfig: RectConfig<SignalRef> = {
   timeUnitBandPosition: 0.5
 };
 
-export interface TickConfig<ES extends ExprRef | SignalRef> extends MarkConfig<ES>, TickThicknessMixins {
+export interface TickConfig<ES extends ExprRef | SignalRef>
+  extends MarkConfig<ES>,
+    TickThicknessMixins,
+    BandSizeConfigMixins<ES> {
   /**
    * The width of the ticks.
    *
@@ -678,7 +690,10 @@ export interface TickConfig<ES extends ExprRef | SignalRef> extends MarkConfig<E
 }
 
 export const defaultTickConfig: TickConfig<SignalRef> = {
-  thickness: 1
+  thickness: 1,
+  discreteBandSize: {band: 0.75},
+  timeUnitBandPosition: 0.5,
+  timeUnitBandSize: 0.75
 };
 
 export function getMarkType(m: string | GenericMarkDef<any>) {

diff --git a/test/compile/mark/tick.test.ts b/test/compile/mark/tick.test.ts
@@ -111,15 +111,15 @@ describe('Mark: Tick', () => {
     });
 
     it('should scale on y', () => {
-      expect(props.yc).toEqual({scale: Y, field: 'Cylinders'});
+      expect(props.y).toEqual({scale: Y, field: 'Cylinders', band: 0.125});
     });
 
     it('width should be tick thickness with default orient vertical', () => {
       expect(props.width).toEqual({value: 1});
     });
 
     it('height should be matched to field with default orient vertical', () => {
-      expect(props.height).toEqual({value: 15});
+      expect(props.height).toEqual({signal: "0.75 * bandwidth('y')"});
     });
   });
 

diff --git a/test/compile/scale/type.test.ts b/test/compile/scale/type.test.ts
@@ -91,7 +91,7 @@ describe('compile/scale', () => {
       describe('continuous', () => {
         it('should return point scale for ordinal X,Y for marks others than rect, rule, bar, and arc', () => {
           PRIMITIVE_MARKS.forEach(mark => {
-            if (util.contains(['bar', 'rule', 'rect', 'image', 'arc'], mark)) {
+            if (util.contains(['bar', 'rule', 'rect', 'image', 'arc', 'tick'], mark)) {
               return;
             }