From da14f121af727da63d85c158caaf83293644395e Mon Sep 17 00:00:00 2001 From: Arvind Satyanarayan Date: Thu, 7 Jan 2021 12:27:22 -0500 Subject: [PATCH] fix: reverse selection extent for descending-ordered scale domains --- src/compile/data/bin.ts | 4 +-- src/compile/scale/assemble.ts | 8 +++--- src/compile/selection/assemble.ts | 22 +++++++++++++--- src/compile/selection/parse.ts | 2 +- test/compile/selection/scales.test.ts | 36 +++++++++++++++++++++------ 5 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/compile/data/bin.ts b/src/compile/data/bin.ts index 87bba39ee14..969b064d323 100644 --- a/src/compile/data/bin.ts +++ b/src/compile/data/bin.ts @@ -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'; @@ -69,7 +69,7 @@ function createBinComponent(t: TypedFieldDef | 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. } diff --git a/src/compile/scale/assemble.ts b/src/compile/scale/assemble.ts index 8e025672d07..8bd448b46de 100644 --- a/src/compile/scale/assemble.ts +++ b/src/compile/scale/assemble.ts @@ -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, diff --git a/src/compile/selection/assemble.ts b/src/compile/selection/assemble.ts index 5883f9b88c2..3c5c361884f 100644 --- a/src/compile/selection/assemble.ts +++ b/src/compile/selection/assemble.ts @@ -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, @@ -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[]) { diff --git a/src/compile/selection/parse.ts b/src/compile/selection/parse.ts index b50b397f562..a7f4f29ed6b 100644 --- a/src/compile/selection/parse.ts +++ b/src/compile/selection/parse.ts @@ -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']; diff --git a/test/compile/selection/scales.test.ts b/test/compile/selection/scales.test.ts index e786b1c388e..7898a3f1386 100644 --- a/test/compile/selection/scales.test.ts +++ b/test/compile/selection/scales.test.ts @@ -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', () => { @@ -128,7 +128,7 @@ describe('Selection + Scales', () => { }); it('should handle nested field references', () => { - let model: Model = parseUnitModelWithScale({ + let model: Model = parseUnitModelWithScaleAndSelection({ params: [ { name: 'grid', @@ -151,7 +151,6 @@ describe('Selection + Scales', () => { } } }); - model.parseSelections(); let scales = assembleScalesForModel(model); expect(scales[0]).toHaveProperty('domainRaw'); @@ -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', () => { @@ -364,7 +388,7 @@ describe('Selection + Scales', () => { it( 'should not bind for unavailable/unsupported scales', log.wrap(localLogger => { - let model = parseUnitModelWithScale({ + let model = parseUnitModelWithScaleAndSelection({ data: {url: 'data/cars.json'}, params: [ { @@ -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({ + model = parseUnitModelWithScaleAndSelection({ data: {url: 'data/cars.json'}, params: [ { @@ -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); }) );