Skip to content

Commit

Permalink
Do not ignore x2's title
Browse files Browse the repository at this point in the history
Fix #3742
  • Loading branch information
kanitw committed May 19, 2018
1 parent d4c2415 commit 5a2d7b8
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 2 deletions.
24 changes: 22 additions & 2 deletions src/compile/axis/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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];

Expand Down Expand Up @@ -242,7 +261,8 @@ function getProperty<K extends keyof AxisComponentProps>(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<string | FieldDefBase<string>[]>(
Expand Down
59 changes: 59 additions & 0 deletions test/compile/axis/parse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down

0 comments on commit 5a2d7b8

Please sign in to comment.