Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: scale-bound selections should respect reversals #7163

Merged
merged 3 commits into from
Jan 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/compile/data/bin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {BinTransform} from '../../transform';
import {Dict, duplicate, hash, isEmpty, keys, replacePathInField, unique, vals, varName} from '../../util';
import {binFormatExpression} from '../format';
import {isUnitModel, Model, ModelWithField} from '../model';
import {parseSelectionBinExtent} from '../selection/parse';
import {parseSelectionExtent} from '../selection/parse';
import {NonPositionScaleChannel, PositionChannel} from './../../channel';
import {DataFlowNode} from './dataflow';

Expand Down Expand Up @@ -69,7 +69,7 @@ function createBinComponent(t: TypedFieldDef<string> | BinTransform, bin: boolea
if (isSelectionExtent(normalizedBin.extent)) {
const ext = normalizedBin.extent;
const selName = ext.selection;
span = parseSelectionBinExtent(model.getSelectionComponent(varName(selName), selName), ext);
span = parseSelectionExtent(model.getSelectionComponent(varName(selName), selName), ext);
delete normalizedBin.extent; // Vega-Lite selection extent map to Vega's span property.
}

Expand Down
8 changes: 3 additions & 5 deletions src/compile/scale/assemble.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@ export function assembleScalesForModel(model: Model): VgScale[] {
const {name, type, selectionExtent, domains: _d, range: _r, reverse, ...otherScaleProps} = scale;
const range = assembleScaleRange(scale.range, name, channel, model);

let domainRaw;
if (selectionExtent) {
domainRaw = assembleSelectionScaleDomain(model, selectionExtent);
}

const domain = assembleDomain(model, channel);
const domainRaw = selectionExtent
? assembleSelectionScaleDomain(model, selectionExtent, scaleComponent, domain)
: null;

scales.push({
name,
Expand Down
22 changes: 18 additions & 4 deletions src/compile/selection/assemble.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@ import {selector as parseSelector} from 'vega-event-selector';
import {identity, isArray, stringValue} from 'vega-util';
import {MODIFY, STORE, unitName, VL_SELECTION_RESOLVE, TUPLE, selectionCompilers} from '.';
import {dateTimeToExpr, isDateTime, dateTimeToTimestamp} from '../../datetime';
import {hasContinuousDomain} from '../../scale';
import {SelectionInit, SelectionInitInterval, SelectionExtent} from '../../selection';
import {keys, vals, varName} from '../../util';
import {VgData} from '../../vega.schema';
import {VgData, VgDomain} from '../../vega.schema';
import {FacetModel} from '../facet';
import {LayerModel} from '../layer';
import {isUnitModel, Model} from '../model';
import {ScaleComponent} from '../scale/component';
import {UnitModel} from '../unit';
import {parseSelectionBinExtent} from './parse';
import {parseSelectionExtent} from './parse';

export function assembleInit(
init: readonly (SelectionInit | readonly SelectionInit[] | SelectionInitInterval)[] | SelectionInit,
Expand Down Expand Up @@ -157,10 +159,22 @@ export function assembleLayerSelectionMarks(model: LayerModel, marks: any[]): an
return marks;
}

export function assembleSelectionScaleDomain(model: Model, extent: SelectionExtent): SignalRef {
export function assembleSelectionScaleDomain(
model: Model,
extent: SelectionExtent,
scaleCmpt: ScaleComponent,
domain: VgDomain
): SignalRef {
const name = extent.selection;
const selCmpt = model.getSelectionComponent(name, varName(name));
return {signal: parseSelectionBinExtent(selCmpt, extent)};
const parsedExtent = parseSelectionExtent(selCmpt, extent);

return {
signal:
hasContinuousDomain(scaleCmpt.get('type')) && isArray(domain) && domain[0] > domain[1]
? `isValid(${parsedExtent}) && reverse(${parsedExtent})`
: parsedExtent
};
}

function cleanupEmptyOnArray(signals: Signal[]) {
Expand Down
2 changes: 1 addition & 1 deletion src/compile/selection/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export function parseSelectionPredicate(
);
}

export function parseSelectionBinExtent(selCmpt: SelectionComponent, extent: SelectionExtent) {
export function parseSelectionExtent(selCmpt: SelectionComponent, extent: SelectionExtent) {
const encoding = extent['encoding'];
let field = extent['field'];

Expand Down
15 changes: 10 additions & 5 deletions src/compile/selection/translate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ function onDelta(
const sizeSg = model.getSizeSignalRef(size).signal;
const scaleCmpt = model.getScaleComponent(channel);
const scaleType = scaleCmpt.get('type');
const sign = hasScales && channel === X ? '-' : ''; // Invert delta when panning x-scales.
const reversed = scaleCmpt.get('reverse'); // scale parsing sets this flag for fieldDef.sort
const sign = !hasScales ? '' : channel === X ? (reversed ? '' : '-') : reversed ? '-' : '';
const extent = `${anchor}.extent_${channel}`;
const offset = `${sign}${delta}.${channel} / ` + (hasScales ? `${sizeSg}` : `span(${extent})`);
const panFn = !hasScales
Expand All @@ -96,10 +97,14 @@ function onDelta(
: scaleType === 'pow'
? 'panPow'
: 'panLinear';
const update =
`${panFn}(${extent}, ${offset}` +
(hasScales && scaleType === 'pow' ? `, ${scaleCmpt.get('exponent') ?? 1}` : '') +
')';
const arg = !hasScales
? ''
: scaleType === 'pow'
? `, ${scaleCmpt.get('exponent') ?? 1}`
: scaleType === 'symlog'
? `, ${scaleCmpt.get('constant') ?? 1}`
: '';
const update = `${panFn}(${extent}, ${offset}${arg})`;

signal.on.push({
events: {signal: delta},
Expand Down
12 changes: 8 additions & 4 deletions src/compile/selection/zoom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,14 @@ function onDelta(
: scaleType === 'pow'
? 'zoomPow'
: 'zoomLinear';
const update =
`${zoomFn}(${base}, ${anchor}, ${delta}` +
(hasScales && scaleType === 'pow' ? `, ${scaleCmpt.get('exponent') ?? 1}` : '') +
')';
const arg = !hasScales
? ''
: scaleType === 'pow'
? `, ${scaleCmpt.get('exponent') ?? 1}`
: scaleType === 'symlog'
? `, ${scaleCmpt.get('constant') ?? 1}`
: '';
const update = `${zoomFn}(${base}, ${anchor}, ${delta}${arg})`;

signal.on.push({
events: {signal: delta},
Expand Down
36 changes: 29 additions & 7 deletions test/compile/selection/scales.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {assembleTopLevelSignals, assembleUnitSelectionSignals} from '../../../sr
import {UnitModel} from '../../../src/compile/unit';
import * as log from '../../../src/log';
import {Domain} from '../../../src/scale';
import {parseConcatModel, parseModel, parseUnitModelWithScale} from '../../util';
import {parseConcatModel, parseModel, parseUnitModelWithScaleAndSelection} from '../../util';

describe('Selection + Scales', () => {
describe('selectionExtent', () => {
Expand Down Expand Up @@ -128,7 +128,7 @@ describe('Selection + Scales', () => {
});

it('should handle nested field references', () => {
let model: Model = parseUnitModelWithScale({
let model: Model = parseUnitModelWithScaleAndSelection({
params: [
{
name: 'grid',
Expand All @@ -151,7 +151,6 @@ describe('Selection + Scales', () => {
}
}
});
model.parseSelections();

let scales = assembleScalesForModel(model);
expect(scales[0]).toHaveProperty('domainRaw');
Expand Down Expand Up @@ -222,6 +221,31 @@ describe('Selection + Scales', () => {
expect(scales[0]).toHaveProperty('domainRaw');
expect(scales[0].domainRaw).toEqual({signal: 'brush["nested\\\\.a"]'});
});

it('should respect ordering of explicit scale domains', () => {
const model = parseUnitModelWithScaleAndSelection({
mark: 'point',
encoding: {
x: {
type: 'quantitative',
field: 'Horsepower',
scale: {domain: [250, 0]}
},
y: {type: 'quantitative', field: 'Miles_per_Gallon'}
},
params: [
{
name: 'pan',
select: 'interval',
bind: 'scales'
}
]
});

const scales = assembleScalesForModel(model);
expect(scales[0]).toHaveProperty('domainRaw');
expect(scales[0].domainRaw).toEqual({signal: 'isValid(pan["Horsepower"]) && reverse(pan["Horsepower"])'});
});
});

describe('signals', () => {
Expand Down Expand Up @@ -364,7 +388,7 @@ describe('Selection + Scales', () => {
it(
'should not bind for unavailable/unsupported scales',
log.wrap(localLogger => {
let model = parseUnitModelWithScale({
parseUnitModelWithScaleAndSelection({
data: {url: 'data/cars.json'},
params: [
{
Expand All @@ -378,10 +402,9 @@ describe('Selection + Scales', () => {
y: {field: 'Miles_per_Gallon', type: 'quantitative'}
}
});
model.parseSelections();
expect(localLogger.warns[0]).toEqual(log.message.cannotProjectOnChannelWithoutField(X));

model = parseUnitModelWithScale({
parseUnitModelWithScaleAndSelection({
data: {url: 'data/cars.json'},
params: [
{
Expand All @@ -396,7 +419,6 @@ describe('Selection + Scales', () => {
y: {field: 'Miles_per_Gallon', type: 'quantitative'}
}
});
model.parseSelections();
expect(localLogger.warns[1]).toEqual(log.message.SCALE_BINDINGS_CONTINUOUS);
})
);
Expand Down
117 changes: 91 additions & 26 deletions test/compile/selection/translate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,21 @@ import {selector as parseSelector} from 'vega-event-selector';
import {assembleUnitSelectionSignals} from '../../../src/compile/selection/assemble';
import {parseUnitSelection} from '../../../src/compile/selection/parse';
import translate from '../../../src/compile/selection/translate';
import {ScaleType} from '../../../src/scale';
import {Scale} from '../../../src/scale';
import {Sort} from '../../../src/sort';
import {parseUnitModel} from '../../util';

function getModel(xscale?: ScaleType, yscale?: ScaleType) {
function getModel(
xscale: Scale = {type: 'linear'},
yscale: Scale = {type: 'linear'},
xsort?: Sort<string>,
ysort?: Sort<string>
) {
const model = parseUnitModel({
mark: 'circle',
encoding: {
x: {field: 'Horsepower', type: 'quantitative', scale: {type: xscale ?? 'linear'}},
y: {field: 'Miles_per_Gallon', type: 'quantitative', scale: {type: yscale ?? 'linear'}},
x: {field: 'Horsepower', type: 'quantitative', scale: xscale, ...(xsort ? {sort: xsort} : {})},
y: {field: 'Miles_per_Gallon', type: 'quantitative', scale: yscale, ...(ysort ? {sort: ysort} : {})},
color: {field: 'Origin', type: 'nominal'}
}
});
Expand Down Expand Up @@ -184,7 +190,7 @@ describe('Translate Selection Transform', () => {
'clampRange(panLinear(four_translate_anchor.extent_y, four_translate_delta.y / span(four_translate_anchor.extent_y)), 0, height)'
});

const model2 = getModel('log', 'pow').model;
const model2 = getModel({type: 'log'}, {type: 'pow'}).model;
model2.component.selection = {four: selCmpts['four']};
signals = assembleUnitSelectionSignals(model2, []);
expect(signals.filter(s => s.name === 'four_x')[0].on).toContainEqual({
Expand All @@ -200,35 +206,94 @@ describe('Translate Selection Transform', () => {
});
});

it('builds panLinear exprs for scale-bound intervals', () => {
const {model, selCmpts} = getModel();
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);
describe('scale-bound intervals', () => {
it('builds panLinear exprs', () => {
const {model, selCmpts} = getModel();
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_x, -six_translate_delta.x / width)'
expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_x, -six_translate_delta.x / width)'
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_y, six_translate_delta.y / height)'
});
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_y, six_translate_delta.y / height)'
it('builds panLog exprs', () => {
const {model, selCmpts} = getModel({type: 'log'});
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLog(six_translate_anchor.extent_x, -six_translate_delta.x / width)'
});
});
});

it('builds panLog/panPow exprs for scale-bound intervals', () => {
const {model, selCmpts} = getModel('log', 'pow');
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);
it('builds panSymlog exprs', () => {
const {model, selCmpts} = getModel({type: 'symlog'}, {type: 'symlog', constant: 0.5});
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panSymlog(six_translate_anchor.extent_x, -six_translate_delta.x / width, 1)'
});

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLog(six_translate_anchor.extent_x, -six_translate_delta.x / width)'
expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panSymlog(six_translate_anchor.extent_y, six_translate_delta.y / height, 0.5)'
});
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panPow(six_translate_anchor.extent_y, six_translate_delta.y / height, 1)'
it('builds panPow exprs', () => {
const {model, selCmpts} = getModel({type: 'pow'}, {type: 'pow', exponent: 2});
model.component.selection = {six: selCmpts['six']};
const signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panPow(six_translate_anchor.extent_x, -six_translate_delta.x / width, 1)'
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panPow(six_translate_anchor.extent_y, six_translate_delta.y / height, 2)'
});
});

it('respects reversals', () => {
let {model, selCmpts} = getModel({type: 'linear', reverse: true}, {type: 'linear', reverse: true});
model.component.selection = {six: selCmpts['six']};
let signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_x, six_translate_delta.x / width)'
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_y, -six_translate_delta.y / height)'
});

({model, selCmpts} = getModel({type: 'linear'}, {type: 'linear'}, 'descending', 'descending'));
model.component.selection = {six: selCmpts['six']};
signals = assembleUnitSelectionSignals(model, []);

expect(signals.filter(s => s.name === 'six_Horsepower')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_x, six_translate_delta.x / width)'
});

expect(signals.filter(s => s.name === 'six_Miles_per_Gallon')[0].on).toContainEqual({
events: {signal: 'six_translate_delta'},
update: 'panLinear(six_translate_anchor.extent_y, -six_translate_delta.y / height)'
});
});
});
});
Expand Down
Loading