Skip to content

Commit

Permalink
fix(#7724,#9414): Make ticks respect bin/timeUnit bands and custom band
Browse files Browse the repository at this point in the history
  • Loading branch information
@kanitw
kanitw committed Aug 27, 2024
1 parent 71d1a8e commit ea85f81
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 70 deletions.
33 changes: 33 additions & 0 deletions build/vega-lite-schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28308,6 +28308,11 @@
],
"description": "For text marks, the vertical text baseline. One of `\"alphabetic\"` (default), `\"top\"`, `\"middle\"`, `\"bottom\"`, `\"line-top\"`, `\"line-bottom\"`, or an expression reference that provides one of the valid values. The `\"line-top\"` and `\"line-bottom\"` values operate similarly to `\"top\"` and `\"bottom\"`, but are calculated relative to the `lineHeight` rather than `fontSize` alone.\n\nFor range marks, the vertical alignment of the marks. One of `\"top\"`, `\"middle\"`, `\"bottom\"`.\n\n__Note:__ Expression reference is *not* supported for range marks."
},
"binSpacing": {
"description": "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).\n\n__Default value:__ `1`",
"minimum": 0,
"type": "number"
},
"blend": {
"anyOf": [
{
Expand All @@ -28333,6 +28338,11 @@
],
"description": "Default color.\n\n__Default value:__ <span style=\"color: #4682b4;\">&#9632;</span> `\"#4682b4\"`\n\n__Note:__\n- This property cannot be used in a [style config](https://vega.github.io/vega-lite/docs/mark.html#style-config).\n- The `fill` and `stroke` properties have higher precedence than `color` and will override `color`."
},
"continuousBandSize": {
"description": "The default size of the bars on continuous scales.\n\n__Default value:__ `5`",
"minimum": 0,
"type": "number"
},
"cornerRadius": {
"anyOf": [
{
Expand Down Expand Up @@ -28421,6 +28431,18 @@
}
]
},
"discreteBandSize": {
"anyOf": [
{
"type": "number"
},
{
"$ref": "#/definitions/RelativeBandSize"
}
],
"description": "The default size of the bars with discrete dimensions. If unspecified, the default size is `step-2`, which provides 2 pixel offset between bars.",
"minimum": 0
},
"dx": {
"anyOf": [
{
Expand Down Expand Up @@ -28633,6 +28655,17 @@
}
]
},
"minBandSize": {
"anyOf": [
{
"type": "number"
},
{
"$ref": "#/definitions/ExprRef"
}
],
"description": "The minimum band size for bar and rectangle marks. __Default value:__ `0.25`"
},
"opacity": {
"anyOf": [
{
Expand Down
23 changes: 16 additions & 7 deletions src/compile/mark/encode/position-rect.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,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 isBarOrTickBand =
(mark === 'bar' && (channel === 'x' ? orient === 'vertical' : orient === 'horizontal')) ||
(mark === 'tick' && (channel === 'y' ? orient === 'vertical' : orient === 'horizontal'));

// x, x2, and width -- we must specify two of these in all conditions
if (
Expand All @@ -68,7 +70,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)) || isBarOrTickBand) && !channelDef2) {
return positionAndSize(channelDef, channel, model);
} else {
return rangePosition(channel, model, {defaultPos: 'zeroOrMax', defaultPos2: 'zeroOrMin'});
Expand Down Expand Up @@ -118,8 +120,11 @@ function defaultSizeRef(
}
}
if (!hasFieldDef) {
const {bandPaddingInner, barBandPaddingInner, rectBandPaddingInner} = config.scale;
const padding = getFirstDefined(bandPaddingInner, mark === 'bar' ? barBandPaddingInner : rectBandPaddingInner); // this part is like paddingInner in scale.ts
const {bandPaddingInner, barBandPaddingInner, rectBandPaddingInner, tickBandPaddingInner} = config.scale;
const padding = getFirstDefined(
bandPaddingInner,
mark === 'tick' ? tickBandPaddingInner : mark === 'bar' ? barBandPaddingInner : rectBandPaddingInner
); // this part is like paddingInner in scale.ts
if (isSignalRef(padding)) {
return {signal: `(1 - (${padding.signal})) * ${sizeChannel}`};
} else if (isNumber(padding)) {
Expand Down Expand Up @@ -150,8 +155,12 @@ function positionAndSize(
const offsetScaleName = model.scaleName(offsetScaleChannel);
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 =
// Always uses size channel for ticks, because tick only calls rectPosition() for the size channel
markDef.type === 'tick' ||
// use "size" channel for bars, if there is orient and the channel matches the right orientation
(orient === 'horizontal' && channel === 'y') ||
(orient === 'vertical' && channel === 'x');

// Use size encoding / mark property / config if it exists
let sizeMixins;
Expand Down Expand Up @@ -315,7 +324,7 @@ function rectBinPosition({
const axis = (model.component.axes as any)[channel]?.[0];
const axisTranslate = axis?.get('translate') ?? 0.5; // vega default is 0.5

const spacing = isXorY(channel) ? (getMarkPropOrConfig('binSpacing', markDef, config) ?? 0) : 0;
const spacing = isXorY(channel) ? getMarkPropOrConfig('binSpacing', markDef, config) ?? 0 : 0;

Check warning on line 327 in src/compile/mark/encode/position-rect.ts

View workflow job for this annotation

GitHub Actions / Runtime, Linting, and Coverage 

Replace `getMarkPropOrConfig('binSpacing',·markDef,·config)·??·0` with `(getMarkPropOrConfig('binSpacing',·markDef,·config)·??·0)`

const channel2 = getSecondaryRangeChannel(channel);
const vgChannel = getVgPositionChannel(channel);
Expand Down
53 changes: 7 additions & 46 deletions src/compile/mark/tick.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import {isNumber} from 'vega-util';
import {isVgRangeStep, VgValueRef} from '../../vega.schema';
import {exprFromSignalRefOrValue, getMarkPropOrConfig, signalOrValueRef} from '../common';
import {getMarkPropOrConfig, signalOrValueRef} from '../common';
import {UnitModel} from '../unit';
import {MarkCompiler} from './base';
import * as encode from './encode';
import {getOffsetScaleChannel} from '../../channel';

export const tick: MarkCompiler = {
vgMark: 'rect',
Expand All @@ -13,7 +10,8 @@ export const tick: MarkCompiler = {
const {config, markDef} = model;
const orient = markDef.orient;

const vgSizeChannel = orient === 'horizontal' ? 'width' : 'height';
const vgSizeAxisChannel = orient === 'horizontal' ? 'x' : 'y';
const vgThicknessAxisChannel = orient === 'horizontal' ? 'y' : 'x';
const vgThicknessChannel = orient === 'horizontal' ? 'height' : 'width';

return {
Expand All @@ -26,49 +24,12 @@ export const tick: MarkCompiler = {
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, {
defaultRef: defaultSize(model),
vgChannel: vgSizeChannel
...encode.rectPosition(model, vgSizeAxisChannel),
...encode.pointPosition(vgThicknessAxisChannel, model, {
defaultPos: 'mid',
vgChannel: vgThicknessAxisChannel === 'y' ? 'yc' : 'xc'
}),
[vgThicknessChannel]: signalOrValueRef(getMarkPropOrConfig('thickness', markDef, config))
};
}
};

function defaultSize(model: UnitModel): VgValueRef {
const {config, markDef} = model;
const {orient} = markDef;

const vgSizeChannel = orient === 'horizontal' ? 'width' : 'height';
const positionChannel = orient === 'horizontal' ? 'x' : 'y';

const offsetScaleChannel = getOffsetScaleChannel(positionChannel);

// Use offset scale if exists
const scale = model.getScaleComponent(offsetScaleChannel) || model.getScaleComponent(positionChannel);

const markPropOrConfig =
getMarkPropOrConfig('size', markDef, config, {vgChannel: vgSizeChannel}) ?? config.tick.bandSize;

if (markPropOrConfig !== undefined) {
return signalOrValueRef(markPropOrConfig);
} else if (scale?.get('type') === 'band') {
const scaleName = model.scaleName(offsetScaleChannel) || model.scaleName(positionChannel);
return {scale: scaleName, band: 1};
}

const scaleRange = scale?.get('range');
const {tickBandPaddingInner} = config.scale;

const step = scaleRange && isVgRangeStep(scaleRange) ? scaleRange.step : model[vgSizeChannel];

if (isNumber(step) && isNumber(tickBandPaddingInner)) {
return {value: step * (1 - tickBandPaddingInner)};
} else {
return {signal: `(1 - ${exprFromSignalRefOrValue(tickBandPaddingInner)}) * ${exprFromSignalRefOrValue(step)}`};
}
}
22 changes: 12 additions & 10 deletions src/mark.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ export function isPathMark(m: Mark | CompositeMark): m is PathMark {
return ['line', 'area', 'trail'].includes(m);
}

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);
export function isRectBasedMark(m: Mark | CompositeMark): m is 'rect' | 'bar' | 'image' | 'arc' | 'tick' {
return ['rect', 'bar', 'image', 'arc', 'tick' /* arc is rect/interval in polar coordinate */].includes(m);
}

export const PRIMITIVE_MARKS = new Set(keys(Mark));
Expand Down Expand Up @@ -647,21 +647,22 @@ export interface MarkDef<M extends string | Mark = Mark, ES extends ExprRef | Si

const DEFAULT_RECT_BAND_SIZE = 5;

export const defaultBarConfig: RectConfig<SignalRef> = {
binSpacing: 1,
continuousBandSize: DEFAULT_RECT_BAND_SIZE,
minBandSize: 0.25,
timeUnitBandPosition: 0.5
};

export const defaultRectConfig: RectConfig<SignalRef> = {
binSpacing: 0,
continuousBandSize: DEFAULT_RECT_BAND_SIZE,
minBandSize: 0.25,
timeUnitBandPosition: 0.5
};

export interface TickConfig<ES extends ExprRef | SignalRef> extends MarkConfig<ES>, TickThicknessMixins {
export const defaultBarConfig: RectConfig<SignalRef> = {
...defaultRectConfig,
binSpacing: 1
};

export interface TickConfig<ES extends ExprRef | SignalRef>
extends MarkConfig<ES>,
TickThicknessMixins,
RectConfig<ES> {
/**
* The width of the ticks.
*
Expand All @@ -672,6 +673,7 @@ export interface TickConfig<ES extends ExprRef | SignalRef> extends MarkConfig<E
}

export const defaultTickConfig: TickConfig<SignalRef> = {
...defaultRectConfig,
thickness: 1
};

Expand Down
13 changes: 6 additions & 7 deletions test/compile/mark/tick.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,15 @@ describe('Mark: Tick', () => {
});

it('should scale on y', () => {
expect(props.yc).toEqual({scale: Y, field: 'Cylinders', band: 0.5});
expect(props.y).toEqual({scale: Y, field: 'Cylinders'});
});

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({scale: 'y', band: 1});
expect(props.height).toEqual({signal: "max(0.25, bandwidth('y'))"});
});
});
describe('with quantitative x and ordinal y with yOffset', () => {
Expand All @@ -139,11 +139,10 @@ describe('Mark: Tick', () => {
});

it('should scale on y', () => {
expect(props.yc).toEqual({
expect(props.y).toEqual({
scale: Y,
field: 'Cylinders',
offset: {
band: 0.5,
field: 'Acceleration',
scale: 'yOffset'
}
Expand All @@ -155,7 +154,7 @@ describe('Mark: Tick', () => {
});

it('height should be matched to field with default orient vertical', () => {
expect(props.height).toEqual({scale: 'yOffset', band: 1});
expect(props.height).toEqual({signal: "max(0.25, bandwidth('yOffset'))"});
});
});

Expand Down Expand Up @@ -217,7 +216,7 @@ describe('Mark: Tick', () => {
});
const props = tick.encodeEntry(model);
it('sets mark height to (1-tickBandPaddingInner) * plot_height', () => {
expect(props.height).toEqual({signal: '(1 - 0.25) * height'});
expect(props.height).toEqual({signal: '0.75 * height'});
});
});

Expand All @@ -231,7 +230,7 @@ describe('Mark: Tick', () => {
});
const props = tick.encodeEntry(model);
it('sets mark width to (1-tickBandPaddingInner) * plot_width', () => {
expect(props.width).toEqual({signal: '(1 - 0.25) * width'});
expect(props.width).toEqual({signal: '0.75 * width'});
});
});
});

0 comments on commit ea85f81

Please sign in to comment.