fix: correctly flatten nested field references for scale bindings

vega · Jan 6, 2021 · 5cb42c9 · 5cb42c9
1 parent ebb8e27
commit 5cb42c9
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 9 deletions.
diff --git a/src/compile/selection/parse.ts b/src/compile/selection/parse.ts
@@ -4,7 +4,7 @@ import {selectionCompilers, SelectionComponent, STORE} from '.';
 import {warn} from '../../log';
 import {LogicalComposition} from '../../logical';
 import {BaseSelectionConfig, SelectionDef, SelectionExtent} from '../../selection';
-import {Dict, duplicate, entries, logicalExpr, varName} from '../../util';
+import {Dict, duplicate, entries, logicalExpr, replacePathInField, varName} from '../../util';
 import {DataFlowNode, OutputNode} from '../data/dataflow';
 import {FilterNode} from '../data/filter';
 import {Model} from '../model';
@@ -120,7 +120,7 @@ export function parseSelectionBinExtent(selCmpt: SelectionComponent, extent: Sel
     }
   }
 
-  return `${selCmpt.name}[${stringValue(field)}]`;
+  return `${selCmpt.name}[${stringValue(replacePathInField(field))}]`;
 }
 
 export function materializeSelections(model: UnitModel, main: OutputNode) {

diff --git a/src/compile/selection/scales.ts b/src/compile/selection/scales.ts
@@ -7,6 +7,7 @@ import {isLayerModel, Model} from '../model';
 import {UnitModel} from '../unit';
 import {SelectionProjection} from './project';
 import {SelectionCompiler} from '.';
+import {replacePathInField} from '../../util';
 
 const scaleBindings: SelectionCompiler<'interval'> = {
   defined: selCmpt => {
@@ -55,10 +56,12 @@ const scaleBindings: SelectionCompiler<'interval'> = {
     const namedSg = signals.filter(s => s.name === selCmpt.name)[0];
     let update = namedSg.update;
     if (update.indexOf(VL_SELECTION_RESOLVE) >= 0) {
-      namedSg.update = `{${bound.map(proj => `${stringValue(proj.field)}: ${proj.signals.data}`).join(', ')}}`;
+      namedSg.update = `{${bound
+        .map(proj => `${stringValue(replacePathInField(proj.field))}: ${proj.signals.data}`)
+        .join(', ')}}`;
     } else {
       for (const proj of bound) {
-        const mapping = `${stringValue(proj.field)}: ${proj.signals.data}`;
+        const mapping = `${stringValue(replacePathInField(proj.field))}: ${proj.signals.data}`;
         if (!update.includes(mapping)) {
           update = `${update.substring(0, update.length - 1)}, ${mapping}}`;
         }

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, parseModelWithScale, parseUnitModelWithScale} from '../../util';
 
 describe('Selection + Scales', () => {
   describe('selectionExtent', () => {
@@ -155,9 +155,9 @@ describe('Selection + Scales', () => {
 
       let scales = assembleScalesForModel(model);
       expect(scales[0]).toHaveProperty('domainRaw');
-      expect(scales[0].domainRaw).toEqual({signal: 'grid["nested.b"]'});
+      expect(scales[0].domainRaw).toEqual({signal: 'grid["nested\\\\.b"]'});
       expect(scales[1]).toHaveProperty('domainRaw');
-      expect(scales[1].domainRaw).toEqual({signal: 'grid["nested.a"]'});
+      expect(scales[1].domainRaw).toEqual({signal: 'grid["nested\\\\.a"]'});
 
       model = parseConcatModel({
         vconcat: [
@@ -216,11 +216,11 @@ describe('Selection + Scales', () => {
 
       scales = assembleScalesForModel(model.children[1]);
       expect(scales[0]).toHaveProperty('domainRaw');
-      expect(scales[0].domainRaw).toEqual({signal: 'brush["nested.a"]'});
+      expect(scales[0].domainRaw).toEqual({signal: 'brush["nested\\\\.a"]'});
 
       scales = assembleScalesForModel(model.children[2]);
       expect(scales[0]).toHaveProperty('domainRaw');
-      expect(scales[0].domainRaw).toEqual({signal: 'brush["nested.a"]'});
+      expect(scales[0].domainRaw).toEqual({signal: 'brush["nested\\\\.a"]'});
     });
   });
 
@@ -325,6 +325,40 @@ describe('Selection + Scales', () => {
         '{"Miles_per_Gallon": selector001_Miles_per_Gallon, "Weight_in_lbs": selector001_Weight_in_lbs, "Acceleration": selector001_Acceleration, "Horsepower": selector001_Horsepower}'
       );
     });
+
+    it('should correctly nest fields for top-level signals', () => {
+      const model = parseModel({
+        repeat: {
+          row: ['p.x', 'p.y'],
+          column: ['p.x', 'p.y']
+        },
+        spec: {
+          mark: 'point',
+          params: [
+            {
+              name: 'sel11',
+              select: 'interval',
+              bind: 'scales'
+            }
+          ],
+          encoding: {
+            x: {field: {repeat: 'column'}, type: 'quantitative'},
+            y: {field: {repeat: 'row'}, type: 'quantitative'}
+          },
+          data: {
+            values: [{p: {x: 1, y: 1}}, {p: {x: 2, y: 1}}, {p: {x: 1, y: 2}}, {p: {x: 3, y: 3}}, {p: {x: 3, y: 2}}]
+          }
+        }
+      });
+
+      model.parseScale();
+      model.parseSelections();
+
+      const signals = assembleTopLevelSignals(model.children[2] as UnitModel, []);
+      const named = signals.filter(s => s.name === 'sel11') as NewSignal[];
+      expect(named).toHaveLength(1);
+      expect(named[0].update).toEqual('{"p\\\\.x": sel11_p_x, "p\\\\.y": sel11_p_y}');
+    });
   });
 
   it(