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: interval selection should gracefully fail if no valid projections #7442

Merged
merged 5 commits into from
May 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 0 additions & 14 deletions build/vega-lite-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -11959,13 +11959,6 @@
},
"type": "array"
},
"fields": {
"description": "An array of field names whose values must match for a data tuple to fall within the selection.\n\n__See also:__ The [projection with `encodings` and `fields` section](https://vega.github.io/vega-lite/docs/selection.html#project) in the documentation.",
"items": {
"$ref": "#/definitions/FieldName"
},
"type": "array"
},
"mark": {
"$ref": "#/definitions/BrushConfig",
"description": "An interval selection also adds a rectangle mark to depict the extents of the interval. The `mark` property can be used to customize the appearance of the mark.\n\n__See also:__ [`mark` examples](https://vega.github.io/vega-lite/docs/selection.html#mark) in the documentation."
Expand Down Expand Up @@ -12034,13 +12027,6 @@
},
"type": "array"
},
"fields": {
"description": "An array of field names whose values must match for a data tuple to fall within the selection.\n\n__See also:__ The [projection with `encodings` and `fields` section](https://vega.github.io/vega-lite/docs/selection.html#project) in the documentation.",
"items": {
"$ref": "#/definitions/FieldName"
},
"type": "array"
},
"mark": {
"$ref": "#/definitions/BrushConfig",
"description": "An interval selection also adds a rectangle mark to depict the extents of the interval. The `mark` property can be used to customize the appearance of the mark.\n\n__See also:__ [`mark` examples](https://vega.github.io/vega-lite/docs/selection.html#mark) in the documentation."
Expand Down
2 changes: 0 additions & 2 deletions site/_includes/docs_toc.md
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,6 @@
- [Nominal]({{site.baseurl}}/docs/type.html#nominal)
- [GeoJSON]({{site.baseurl}}/docs/type.html#geojson)
- [Value]({{site.baseurl}}/docs/value.html)
- [Examples]({{site.baseurl}}/docs/value.html#examples)
- [Projection]({{site.baseurl}}/docs/projection.html)
- [Documentation Overview]({{site.baseurl}}/docs/projection.html#documentation-overview)
- [Projection Properties]({{site.baseurl}}/docs/projection.html#projection-properties)
Expand Down Expand Up @@ -328,7 +327,6 @@
- [Using Parameters]({{site.baseurl}}/docs/parameter.html#using-parameters)
- [Selection Configuration]({{site.baseurl}}/docs/parameter.html#config)
- [Value]({{site.baseurl}}/docs/value.html)
- [Examples]({{site.baseurl}}/docs/value.html#examples)
- [Expr]({{site.baseurl}}/docs/parameter.html)
- [Documentation Overview]({{site.baseurl}}/docs/parameter.html#documentation-overview)
- [Defining a Parameter]({{site.baseurl}}/docs/parameter.html#defining-a-parameter)
Expand Down
23 changes: 14 additions & 9 deletions src/compile/selection/interval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const interval: SelectionCompiler<'interval'> = {

// Proxy scale reactions to ensure that an infinite loop doesn't occur
// when an interval selection filter touches the scale.
if (!hasScales) {
if (!hasScales && scaleTriggers.length) {
signals.push({
name: name + SCALE_TRIGGER,
value: {},
Expand All @@ -88,12 +88,16 @@ const interval: SelectionCompiler<'interval'> = {
return signals.concat({
name: name + TUPLE,
...(init ? {init: `{${update}: ${assembleInit(init)}}`} : {}),
on: [
{
events: [{signal: dataSignals.join(' || ')}], // Prevents double invocation, see https://github.com/vega/vega#1672.
update: `${dataSignals.join(' && ')} ? {${update}: [${dataSignals}]} : null`
}
]
...(dataSignals.length
? {
on: [
{
events: [{signal: dataSignals.join(' || ')}], // Prevents double invocation, see https://github.com/vega/vega#1672.
update: `${dataSignals.join(' && ')} ? {${update}: [${dataSignals}]} : null`
}
]
}
: {})
});
},

Expand All @@ -104,8 +108,9 @@ const interval: SelectionCompiler<'interval'> = {
const yvname = y && y.signals.visual;
const store = `data(${stringValue(selCmpt.name + STORE)})`;

// Do not add a brush if we're binding to scales.
if (scales.defined(selCmpt)) {
// Do not add a brush if we're binding to scales
// or we don't have a valid interval projection
if (scales.defined(selCmpt) || (!x && !y)) {
return marks;
}

Expand Down
9 changes: 7 additions & 2 deletions src/compile/selection/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,14 @@ export function parseUnitSelection(model: UnitModel, selDefs: SelectionParameter
// Set default values from config if a property hasn't been specified,
// or if it is true. E.g., "translate": true should use the default
// event handlers for translate. However, true may be a valid value for
// a property (e.g., "nearest": true). Project transform applies its defaults.
const {fields, encodings, ...cfg} = selectionConfig[type];
// a property (e.g., "nearest": true).
const cfg = selectionConfig[type];
for (const key in cfg) {
// Project transform applies its defaults.
if (key === 'fields' || key === 'encodings') {
continue;
}

if (key === 'mark') {
defaults[key] = {...cfg[key], ...defaults[key]};
}
Expand Down
9 changes: 5 additions & 4 deletions src/compile/selection/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {array, isObject} from 'vega-util';
import {isSingleDefUnitChannel, ScaleChannel, SingleDefUnitChannel} from '../../channel';
import * as log from '../../log';
import {hasContinuousDomain} from '../../scale';
import {BaseSelectionConfig, SelectionInitIntervalMapping, SelectionInitMapping} from '../../selection';
import {PointSelectionConfig, SelectionInitIntervalMapping, SelectionInitMapping} from '../../selection';
import {Dict, hash, keys, replacePathInField, varName, isEmpty} from '../../util';
import {TimeUnitComponent, TimeUnitNode} from '../data/timeunit';
import {SelectionCompiler} from '.';
Expand Down Expand Up @@ -71,7 +71,7 @@ const project: SelectionCompiler = {

// If no explicit projection (either fields or encodings) is specified, set some defaults.
// If an initial value is set, try to infer projections.
let {fields, encodings} = isObject(selDef.select) ? selDef.select : ({} as BaseSelectionConfig);
let {fields, encodings} = (isObject(selDef.select) ? selDef.select : {}) as PointSelectionConfig;
if (!fields && !encodings && init) {
for (const initVal of init) {
// initVal may be a scalar value to smoothen varParam -> pointSelection gradient.
Expand Down Expand Up @@ -99,7 +99,9 @@ const project: SelectionCompiler = {
// to account for unprojected point selections that have scalar initial values
if (!fields && !encodings) {
encodings = cfg.encodings;
fields = cfg.fields;
if ('fields' in cfg) {
fields = cfg.fields;
}
}

for (const channel of encodings ?? []) {
Expand Down Expand Up @@ -156,7 +158,6 @@ const project: SelectionCompiler = {
}
}

// TODO: find a possible channel mapping for these fields.
for (const field of fields ?? []) {
if (proj.hasField[field]) continue;
const p: SelectionProjection = {type: 'E', field};
Expand Down
4 changes: 2 additions & 2 deletions src/selection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,17 @@ export interface BaseSelectionConfig<T extends SelectionType = SelectionType> {
* __See also:__ The [projection with `encodings` and `fields` section](https://vega.github.io/vega-lite/docs/selection.html#project) in the documentation.
*/
encodings?: SingleDefUnitChannel[];
}

export interface PointSelectionConfig extends BaseSelectionConfig<'point'> {
/**
* An array of field names whose values must match for a data tuple to
* fall within the selection.
*
* __See also:__ The [projection with `encodings` and `fields` section](https://vega.github.io/vega-lite/docs/selection.html#project) in the documentation.
*/
fields?: FieldName[];
}

export interface PointSelectionConfig extends BaseSelectionConfig<'point'> {
/**
* Controls whether data values should be toggled (inserted or removed from a point selection)
* or only ever inserted into multi selections.
Expand Down