fix: reverse selection extent for descending-ordered scale domains

vega · Jan 7, 2021 · da14f12 · da14f12
1 parent e891457
commit da14f12
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 19 deletions.
diff --git 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<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.
   }
 

diff --git 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
@@ -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
@@ -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
@@ -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);
     })
   );