From 4849728864b935eaaad2f42b3acbf26d4322cfae Mon Sep 17 00:00:00 2001 From: Kanit Wongsuphasawat Date: Thu, 17 May 2018 21:10:07 -0700 Subject: [PATCH] Do not ignore x2's title Fix https://github.com/vega/vega-lite/issues/3742 --- src/compile/axis/parse.ts | 24 ++++++++++++++++++-- test/compile/axis/parse.test.ts | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/compile/axis/parse.ts b/src/compile/axis/parse.ts index 36c83070e5d..d51da08f7bc 100644 --- a/src/compile/axis/parse.ts +++ b/src/compile/axis/parse.ts @@ -152,6 +152,25 @@ 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 titles = [ + ...(fieldDef && fieldDef.title ? [fieldDef.title] : []), + ...(fieldDef2 && fieldDef2.title ? [fieldDef2.title] : []) + ]; + + if (titles.length > 0) { + return titles.join(', '); + } else if (fieldDef && fieldDef.title !== undefined) { // explicit falsy value + return fieldDef.title; + } else if (fieldDef2 && fieldDef2.title !== undefined) { // explicit falsy value + return fieldDef2.title; + } + return undefined; +} function parseAxis(channel: PositionScaleChannel, model: UnitModel): AxisComponent { const axis = model.axis(channel); @@ -168,7 +187,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 +261,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/test/compile/axis/parse.test.ts b/test/compile/axis/parse.test.ts index 2bdc022e54f..145000e2c82 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,27 @@ 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 store both x and x2 for ranged mark', function () { const model = parseUnitModelWithScale({ mark: "rule",