fix: remove redundant code that leads to bugs + improve test coverage

vega · Feb 2, 2021 · e11e158 · e11e158
1 parent d218151
commit e11e158
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 18 deletions.
diff --git a/site/docs/mark/rect.md b/site/docs/mark/rect.md
@@ -47,7 +47,7 @@ The `rect` mark represents an arbitrary rectangle.
 
 A rect mark definition can contain any [standard mark properties](mark.html#mark-def) and the following special properties:
 
-{% include table.html props="align,baseline,cornerRadius" source="MarkConfig" %}
+{% include table.html props="width,height,align,baseline,cornerRadius" source="MarkConfig" %}
 
 ## Examples
 

diff --git a/src/channeldef.ts b/src/channeldef.ts
@@ -47,7 +47,7 @@ import {
   Y,
   Y2
 } from './channel';
-import {getMarkConfig} from './compile/common';
+import {getMarkConfig, getMarkPropOrConfig} from './compile/common';
 import {isCustomFormatType} from './compile/format';
 import {CompositeAggregate} from './compositemark';
 import {Config} from './config';
@@ -548,18 +548,24 @@ export function getBandSize({
   fieldDef2,
   markDef: mark,
   config,
-  scaleType
+  scaleType,
+  useVlSizeChannel
 }: {
   channel: PositionScaleChannel | PolarPositionScaleChannel;
   fieldDef: ChannelDef<string>;
   fieldDef2?: SecondaryChannelDef<string>;
   markDef: MarkDef<Mark, SignalRef>;
   config: Config<SignalRef>;
   scaleType: ScaleType;
+  useVlSizeChannel?: boolean;
 }): number | RelativeBandSize | SignalRef {
-  const dimensionSize = mark[getSizeChannel(channel)];
-  if (dimensionSize !== undefined) {
-    return dimensionSize;
+  const sizeChannel = getSizeChannel(channel);
+  const size = getMarkPropOrConfig(useVlSizeChannel ? 'size' : sizeChannel, mark, config, {
+    vgChannel: sizeChannel
+  });
+
+  if (size !== undefined) {
+    return size;
   }
 
   if (isFieldDef(fieldDef)) {

diff --git a/src/compile/mark/encode/position-rect.ts b/src/compile/mark/encode/position-rect.ts
@@ -137,30 +137,21 @@ function positionAndSize(
   // 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 sizeFromMarkOrConfig = getMarkPropOrConfig(useVlSizeChannel ? 'size' : vgSizeChannel, markDef, config, {
-    vgChannel: vgSizeChannel
-  });
-
   // Use size encoding / mark property / config if it exists
   let sizeMixins;
-  if (encoding.size || sizeFromMarkOrConfig !== undefined) {
+  if (encoding.size || markDef.size) {
     if (useVlSizeChannel) {
       sizeMixins = nonPosition('size', model, {
         vgChannel: vgSizeChannel,
-        defaultRef: isRelativeBandSize(sizeFromMarkOrConfig)
-          ? {
-              scale: scaleName,
-              band: sizeFromMarkOrConfig.band
-            }
-          : signalOrValueRef(sizeFromMarkOrConfig)
+        defaultRef: signalOrValueRef(markDef.size)
       });
     } else {
       log.warn(log.message.cannotApplySizeToNonOrientedMark(markDef.type));
     }
   }
 
   // Otherwise, apply default value
-  const bandSize = getBandSize({channel, fieldDef, markDef, config, scaleType: scale?.get('type')});
+  const bandSize = getBandSize({channel, fieldDef, markDef, config, scaleType: scale?.get('type'), useVlSizeChannel});
 
   sizeMixins = sizeMixins || {[vgSizeChannel]: defaultSizeRef(vgSizeChannel, scaleName, scale, config, bandSize)};
 

diff --git a/test/compile/mark/bar.test.ts b/test/compile/mark/bar.test.ts
@@ -6,6 +6,24 @@ import {defaultBarConfig} from '../../../src/mark';
 import {parseUnitModelWithScaleAndLayoutSize} from '../../util';
 
 describe('Mark: Bar', () => {
+  describe('1D vertical', () => {
+    const model = parseUnitModelWithScaleAndLayoutSize({
+      data: {url: 'data/cars.json'},
+      mark: {type: 'bar', width: {band: 0.5}},
+      encoding: {
+        y: {type: 'quantitative', field: 'Acceleration', aggregate: 'mean'}
+      }
+    });
+    const props = bar.encodeEntry(model);
+
+    it('should draw bar, with y from zero to field value and with band value for x/width', () => {
+      expect(props.width).toEqual({mult: 0.5, field: {group: 'width'}});
+      expect(props.y).toEqual({scale: 'y', field: 'mean_Acceleration'});
+      expect(props.y2).toEqual({scale: 'y', value: 0});
+      expect(props.height).toBeUndefined();
+    });
+  });
+
   describe('simple vertical', () => {
     const model = parseUnitModelWithScaleAndLayoutSize({
       data: {url: 'data/cars.json'},

diff --git a/test/compile/mark/text.test.ts b/test/compile/mark/text.test.ts
@@ -25,6 +25,22 @@ describe('Mark: Text', () => {
     });
   });
 
+  it('should use stack_mid on theta for stacked theta', () => {
+    // This is a simplified example for stacked text.
+    // In reality this will be used as stacked's overlayed marker
+    const model = parseUnitModelWithScaleAndLayoutSize({
+      mark: 'text',
+      encoding: {
+        theta: {field: 'value', type: 'quantitative', stack: true},
+        color: {field: 'b', type: 'ordinal'}
+      },
+      data: {url: 'data/barley.json'}
+    });
+
+    const props = text.encodeEntry(model);
+    expect(props.theta).toEqual({signal: 'scale("theta", 0.5 * datum["value_start"] + 0.5 * datum["value_end"])'});
+  });
+
   describe('with stacked y', () => {
     // This is a simplified example for stacked text.
     // In reality this will be used as stacked's overlayed marker