diff --git a/src/compile/axis/parse.ts b/src/compile/axis/parse.ts index 36c83070e5..c1c90c356d 100644 --- a/src/compile/axis/parse.ts +++ b/src/compile/axis/parse.ts @@ -3,7 +3,7 @@ import {POSITION_SCALE_CHANNELS, PositionScaleChannel, X, Y} from '../../channel import {FieldDefBase, toFieldDefBase} from '../../fielddef'; import {keys} from '../../util'; import {AxisOrient, VgAxis, VgAxisEncode} from '../../vega.schema'; -import {getSpecifiedOrDefaultValue, mergeTitleFieldDefs, numberFormat, titleMerger} from '../common'; +import {getSpecifiedOrDefaultValue, mergeTitle, mergeTitleComponent, mergeTitleFieldDefs, numberFormat} from '../common'; import {LayerModel} from '../layer'; import {parseGuideResolve} from '../resolve'; import {defaultTieBreaker, Explicit, mergeValuesWithExplicit} from '../split'; @@ -137,7 +137,7 @@ function mergeAxisComponent(merged: AxisComponent, child: AxisComponent): AxisCo (v1: Explicit, v2: Explicit) => { switch (prop) { case 'title': - return titleMerger(v1, v2); + return mergeTitleComponent(v1, v2); case 'gridScale': return { explicit: v1.explicit, // keep the old explicit @@ -152,6 +152,28 @@ function mergeAxisComponent(merged: AxisComponent, child: AxisComponent): AxisCo return merged; } +function getFieldDefTitle(model: UnitModel, channel: 'x' | 'y') { + const channel2 = channel === 'x' ? 'x2' : 'y2'; + const fieldDef = model.fieldDef(channel); + const fieldDef2 = model.fieldDef(channel2); + + const title1 = fieldDef ? fieldDef.title : undefined; + const title2 = fieldDef2 ? fieldDef2.title : undefined; + + if (title1 && title2) { + return mergeTitle(title1, title2); + } else if (title1) { + return title1; + } else if (title2) { + return title2; + } else if (title1 !== undefined) { // falsy value to disable config + return title1; + } else if (title2 !== undefined) { // falsy value to disable config + return title2; + } + + return undefined; +} function parseAxis(channel: PositionScaleChannel, model: UnitModel): AxisComponent { const axis = model.axis(channel); @@ -168,7 +190,7 @@ function parseAxis(channel: PositionScaleChannel, model: UnitModel): AxisCompone // both VL axis.encoding and axis.labelAngle affect VG axis.encode property === 'encode' ? !!axis.encoding || !!axis.labelAngle : // title can be explicit if fieldDef.title is set - property === 'title' && value === model.fieldDef(channel).title ? true : + property === 'title' && value === getFieldDefTitle(model, channel) ? true : // Otherwise, things are explicit if the returned value matches the specified property value === axis[property]; @@ -242,7 +264,8 @@ function getProperty(property: K, specifiedA const fieldDef2 = model.fieldDef(channel2); // Keep undefined so we use default if title is unspecified. // For other falsy value, keep them so we will hide the title. - const specifiedTitle = fieldDef.title !== undefined ? fieldDef.title : + const fieldDefTitle = getFieldDefTitle(model, channel); + const specifiedTitle = fieldDefTitle !== undefined ? fieldDefTitle : specifiedAxis.title === undefined ? undefined : specifiedAxis.title; return getSpecifiedOrDefaultValue[]>( diff --git a/src/compile/common.ts b/src/compile/common.ts index 7247b98b40..754efc16ab 100644 --- a/src/compile/common.ts +++ b/src/compile/common.ts @@ -181,7 +181,13 @@ export function mergeTitleFieldDefs(f1: FieldDefBase[], f2: FieldDefBase return merged; } -export function titleMerger( +export function mergeTitle(title1: string, title2: string) { + return title1 === title2 ? + title1 : // if title is the same just use one of them + title1 + ', ' + title2; // join title with comma if different +} + +export function mergeTitleComponent( v1: Explicit, v2: Explicit ) { if (isArray(v1.value) && isArray(v2.value)) { @@ -192,9 +198,7 @@ export function titleMerger( } else if (!isArray(v1.value) && !isArray(v2.value)) { return { explicit: v1.explicit, // keep the old explicit - value: v1.value === v2.value ? - v1.value : // if title is the same just use one of them - v1.value + ', ' + v2.value // join title with comma if different + value: mergeTitle(v1.value, v2.value) }; } /* istanbul ignore next: Condition should not happen -- only for warning in development. */ diff --git a/src/compile/legend/parse.ts b/src/compile/legend/parse.ts index f94e90e39c..60e0c5b6ce 100644 --- a/src/compile/legend/parse.ts +++ b/src/compile/legend/parse.ts @@ -4,7 +4,7 @@ import {Legend, LEGEND_PROPERTIES, VG_LEGEND_PROPERTIES} from '../../legend'; import {GEOJSON} from '../../type'; import {deleteNestedProperty, keys} from '../../util'; import {VgLegend, VgLegendEncode} from '../../vega.schema'; -import {getSpecifiedOrDefaultValue, numberFormat, titleMerger} from '../common'; +import {getSpecifiedOrDefaultValue, mergeTitleComponent, numberFormat} from '../common'; import {isUnitModel, Model} from '../model'; import {parseGuideResolve} from '../resolve'; import {Explicit, makeImplicit} from '../split'; @@ -184,7 +184,7 @@ export function mergeLegendComponent(mergedLegend: LegendComponent, childLegend: (v1: Explicit, v2: Explicit): any => { switch (prop) { case 'title': - return titleMerger(v1, v2); + return mergeTitleComponent(v1, v2); case 'type': // There are only two types. If we have different types, then prefer symbol over gradient. typeMerged = true; diff --git a/test/compile/axis/parse.test.ts b/test/compile/axis/parse.test.ts index 2bdc022e54..9dd8c5be41 100644 --- a/test/compile/axis/parse.test.ts +++ b/test/compile/axis/parse.test.ts @@ -130,6 +130,24 @@ describe('Axis', function() { } }); + it('should store the fieldDef title value if title = null, "", or false', function () { + for (const val of [null, '', false]) { + const model = parseUnitModelWithScale({ + mark: "point", + encoding: { + x: { + field: "a", + type: "quantitative", + title: val as any // Need to cast as false is not valid, but we want to fall back gracefully + } + } + }); + const axisComponent = parseUnitAxis(model); + assert.equal(axisComponent['x'].length, 1); + assert.equal(axisComponent['x'][0].explicit.title, val as any); + } + }); + it('should store fieldDef.title as explicit', function () { const model = parseUnitModelWithScale({ mark: "point", @@ -146,6 +164,47 @@ describe('Axis', function() { assert.equal(axisComponent['x'][0].explicit.title, 'foo'); }); + it('should merge title of fieldDef and fieldDef2', function () { + const model = parseUnitModelWithScale({ + mark: "bar", + encoding: { + x: { + field: "a", + type: "quantitative", + title: 'foo' + }, + x2: { + field: "b", + type: "quantitative", + title: 'bar' + } + } + }); + const axisComponent = parseUnitAxis(model); + assert.equal(axisComponent['x'].length, 1); + assert.equal(axisComponent['x'][0].explicit.title, 'foo, bar'); + }); + + it('should use title of fieldDef2', function () { + const model = parseUnitModelWithScale({ + mark: "bar", + encoding: { + x: { + field: "a", + type: "quantitative" + }, + x2: { + field: "b", + type: "quantitative", + title: 'bar' + } + } + }); + const axisComponent = parseUnitAxis(model); + assert.equal(axisComponent['x'].length, 1); + assert.equal(axisComponent['x'][0].explicit.title, 'bar'); + }); + it('should store both x and x2 for ranged mark', function () { const model = parseUnitModelWithScale({ mark: "rule",