Skip to content

Commit

Permalink
fix: correctly merge object-valued config properties (#7435)
Browse files Browse the repository at this point in the history
  • Loading branch information
arvind authored May 5, 2021
1 parent a24d6e2 commit 27e9328
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
12 changes: 9 additions & 3 deletions src/config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Color, InitSignal, Locale, NewSignal, RangeConfig, RangeScheme, SignalRef} from 'vega';
import {Color, InitSignal, Locale, NewSignal, RangeConfig, RangeScheme, SignalRef, writeConfig} from 'vega';
import {isObject, mergeConfig} from 'vega-util';
import {Axis, AxisConfig, AxisConfigMixins, AXIS_CONFIGS, isConditionalAxisValue} from './axis';
import {signalOrValueRefWithCondition, signalRefOrValue} from './compile/common';
Expand Down Expand Up @@ -504,15 +504,21 @@ const configPropsWithExpr = [
* then replace all expressions with signals
*/
export function initConfig(specifiedConfig: Config = {}): Config<SignalRef> {
const {color, font, fontSize, ...restConfig} = specifiedConfig;
const {color, font, fontSize, selection, ...restConfig} = specifiedConfig;
const mergedConfig = mergeConfig(
{},
defaultConfig,
duplicate(defaultConfig),
font ? fontConfig(font) : {},
color ? colorSignalConfig(color) : {},
fontSize ? fontSizeSignalConfig(fontSize) : {},
restConfig || {}
);

// mergeConfig doesn't recurse and overrides object values.
if (selection) {
writeConfig(mergedConfig, 'selection', selection, true);
}

const outputConfig: Config<SignalRef> = omit(mergedConfig, configPropsWithExpr);

for (const prop of ['background', 'lineBreak', 'padding']) {
Expand Down
14 changes: 14 additions & 0 deletions test/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,20 @@ describe('config', () => {
}
]);
});

it('correctly merges object-valued configs', () => {
const cfg = initConfig({selection: {point: {on: 'mouseover'}, interval: {encodings: ['x']}}});
expect(cfg.selection).toHaveProperty('point');
expect(cfg.selection).toHaveProperty('interval');

// Overrides correctly
expect(cfg.selection.point).toHaveProperty('on', 'mouseover');
expect(cfg.selection.interval).toHaveProperty('encodings', ['x']);

// Preserves defaults
expect(cfg.selection.point).toHaveProperty('toggle', 'event.shiftKey');
expect(cfg.selection.interval).toHaveProperty('zoom', 'wheel!');
});
});

describe('stripAndRedirectConfig', () => {
Expand Down

0 comments on commit 27e9328

Please sign in to comment.