Skip to content

Commit

Permalink
fix: support x/yOffset without x/y
Browse files Browse the repository at this point in the history
  • Loading branch information
kanitw committed Oct 12, 2023
1 parent f42c534 commit 14ca358
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 37 deletions.
13 changes: 11 additions & 2 deletions src/compile/mark/encode/position-rect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,14 @@ function positionAndSize(
const hasSizeFromMarkOrEncoding = !!sizeMixins;

// Otherwise, apply default value
const bandSize = getBandSize({channel, fieldDef, markDef, config, scaleType: scale?.get('type'), useVlSizeChannel});
const bandSize = getBandSize({
channel,
fieldDef,
markDef,
config,
scaleType: (scale || offsetScale)?.get('type'),
useVlSizeChannel
});

sizeMixins = sizeMixins || {
[vgSizeChannel]: defaultSizeRef(
Expand All @@ -190,7 +197,9 @@ function positionAndSize(
*/

const defaultBandAlign =
scale?.get('type') === 'band' && isRelativeBandSize(bandSize) && !hasSizeFromMarkOrEncoding ? 'top' : 'middle';
(scale || offsetScale)?.get('type') === 'band' && isRelativeBandSize(bandSize) && !hasSizeFromMarkOrEncoding
? 'top'
: 'middle';

const vgChannel = vgAlignedPositionChannel(channel, markDef, config, defaultBandAlign);
const center = vgChannel === 'xc' || vgChannel === 'yc';
Expand Down
14 changes: 1 addition & 13 deletions src/compile/scale/parse.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {getMainChannelFromOffsetChannel, isXorYOffset, ScaleChannel, SCALE_CHANNELS, SHAPE} from '../../channel';
import {ScaleChannel, SCALE_CHANNELS, SHAPE} from '../../channel';
import {getFieldOrDatumDef, ScaleDatumDef, TypedFieldDef} from '../../channeldef';
import {channelHasNestedOffsetScale} from '../../encoding';
import * as log from '../../log';
import {GEOSHAPE} from '../../mark';
import {
NON_TYPE_DOMAIN_RANGE_VEGA_SCALE_PROPERTIES,
Expand Down Expand Up @@ -56,17 +55,6 @@ function parseUnitScaleCore(model: UnitModel): ScaleComponentIndex {
}

let specifiedScale = fieldOrDatumDef && fieldOrDatumDef['scale'];
if (isXorYOffset(channel)) {
const mainChannel = getMainChannelFromOffsetChannel(channel);
if (!channelHasNestedOffsetScale(encoding, mainChannel)) {
// Don't generate scale when the offset encoding shouldn't yield a nested scale
if (specifiedScale) {
log.warn(log.message.offsetEncodingScaleIgnored(channel));
}
continue;
}
}

if (fieldOrDatumDef && specifiedScale !== null && specifiedScale !== false) {
specifiedScale ??= {};
const hasNestedOffsetScale = channelHasNestedOffsetScale(encoding, channel);
Expand Down
50 changes: 36 additions & 14 deletions src/compile/scale/range.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,39 @@ function parseScheme(scheme: Scheme | SignalRef): RangeScheme {
return {scheme};
}

function fullWidthOrHeightRange(
channel: 'x' | 'y',
model: UnitModel,
scaleType: ScaleType,
{center}: {center?: boolean} = {}
) {
// If step is null, use zero to width or height.
// Note that we use SignalRefWrapper to account for potential merges and renames.
const sizeType = getSizeChannel(channel);
const sizeSignal = model.getName(sizeType);
const getSignalName = model.getSignalName.bind(model);

if (channel === Y && hasContinuousDomain(scaleType)) {
// For y continuous scale, we have to start from the height as the bottom part has the max value.
return center
? [
SignalRefWrapper.fromName(name => `0.5*${getSignalName(name)}`, sizeSignal),
SignalRefWrapper.fromName(name => `-0.5*${getSignalName(name)}`, sizeSignal)

Check warning on line 241 in src/compile/scale/range.ts

View check run for this annotation

Codecov / codecov/patch

src/compile/scale/range.ts#L239-L241

Added lines #L239 - L241 were not covered by tests
]
: [SignalRefWrapper.fromName(getSignalName, sizeSignal), 0];
} else {
return center
? [
SignalRefWrapper.fromName(name => `-0.5*${getSignalName(name)}`, sizeSignal),
SignalRefWrapper.fromName(name => `0.5*${getSignalName(name)}`, sizeSignal)

Check warning on line 248 in src/compile/scale/range.ts

View check run for this annotation

Codecov / codecov/patch

src/compile/scale/range.ts#L246-L248

Added lines #L246 - L248 were not covered by tests
]
: [0, SignalRefWrapper.fromName(getSignalName, sizeSignal)];
}
}

function defaultRange(channel: ScaleChannel, model: UnitModel): VgRange {
const {size, config, mark, encoding} = model;

const getSignalName = model.getSignalName.bind(model);

const {type} = getFieldOrDatumDef(encoding[channel]) as ScaleFieldDef<string> | ScaleDatumDef;

const mergedScaleCmpt = model.getScaleComponent(channel);
Expand All @@ -245,18 +273,7 @@ function defaultRange(channel: ScaleChannel, model: UnitModel): VgRange {
}
}

// If step is null, use zero to width or height.
// Note that we use SignalRefWrapper to account for potential merges and renames.

const sizeType = getSizeChannel(channel);
const sizeSignal = model.getName(sizeType);

if (channel === Y && hasContinuousDomain(scaleType)) {
// For y continuous scale, we have to start from the height as the bottom part has the max value.
return [SignalRefWrapper.fromName(getSignalName, sizeSignal), 0];
} else {
return [0, SignalRefWrapper.fromName(getSignalName, sizeSignal)];
}
return fullWidthOrHeightRange(channel, model, scaleType);
}

case XOFFSET:
Expand Down Expand Up @@ -374,6 +391,11 @@ function getOffsetStep(step: Step, offsetScaleType: ScaleType) {
function getOffsetRange(channel: string, model: UnitModel, offsetScaleType: ScaleType): VgRange {
const positionChannel = channel === XOFFSET ? 'x' : 'y';
const positionScaleCmpt = model.getScaleComponent(positionChannel);

if (!positionScaleCmpt) {
return fullWidthOrHeightRange(positionChannel, model, offsetScaleType, {center: true});

Check warning on line 396 in src/compile/scale/range.ts

View check run for this annotation

Codecov / codecov/patch

src/compile/scale/range.ts#L396

Added line #L396 was not covered by tests
}

const positionScaleType = positionScaleCmpt.get('type');
const positionScaleName = model.scaleName(positionChannel);

Expand Down
16 changes: 8 additions & 8 deletions src/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,16 +176,16 @@ export function stack(m: Mark | MarkDef, encoding: Encoding<string>): StackPrope
groupbyChannels.push(dimensionChannel);
groupbyFields.add(dimensionField);
}
}

const dimensionOffsetChannel = dimensionChannel === 'x' ? 'xOffset' : 'yOffset';
const dimensionOffsetDef = encoding[dimensionOffsetChannel];
const dimensionOffsetField = isFieldDef(dimensionOffsetDef) ? vgField(dimensionOffsetDef, {}) : undefined;
const dimensionOffsetChannel = dimensionChannel === 'x' ? 'xOffset' : 'yOffset';
const dimensionOffsetDef = encoding[dimensionOffsetChannel];
const dimensionOffsetField = isFieldDef(dimensionOffsetDef) ? vgField(dimensionOffsetDef, {}) : undefined;

if (dimensionOffsetField && dimensionOffsetField !== stackedField) {
// avoid grouping by the stacked field
groupbyChannels.push(dimensionOffsetChannel);
groupbyFields.add(dimensionOffsetField);
}
if (dimensionOffsetField && dimensionOffsetField !== stackedField) {
// avoid grouping by the stacked field
groupbyChannels.push(dimensionOffsetChannel);
groupbyFields.add(dimensionOffsetField);
}

// If the dimension has offset, don't stack anymore
Expand Down

0 comments on commit 14ca358

Please sign in to comment.