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

Pie title #2987

Merged
merged 6 commits into from
Sep 28, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/traces/pie/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,33 @@ module.exports = {
description: 'Sets the font used for `textinfo` lying outside the pie.'
}),

title: {
valType: 'string',
dflt: '',
role: 'info',
editType: 'calc',
description: [
'Sets the title of the pie chart.',
'If it is empty, no title is displayed.'
].join(' ')
},
titleposition: {
valType: 'enumerated',
values: ['inhole', 'outside'],
dflt: 'outside',
codrut3 marked this conversation as resolved.
Show resolved Hide resolved
role: 'info',
editType: 'calc',
description: [
'Specifies the location of the `title`.',
'If `inhole` and the chart is a donut, the text is scaled',
'and displayed inside the hole.',
'If `outside`, the text is shown above the pie chart.'
].join(' ')
},
titlefont: extendFlat({}, textFontAttrs, {
description: 'Sets the font used for `title`.'
}),

// position and shape
domain: domainAttrs({name: 'pie', trace: true, editType: 'calc'}),

Expand Down
9 changes: 9 additions & 0 deletions src/traces/pie/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
}
}

var title = coerce('title');
if(title) {
var titlePosition = coerce('titleposition');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think we should make the default be 'middle center' when there's a hole (ie var titlePosition = coerce('titleposition', hole ? 'middle center' : 'top center'); and remove attributes.titleposition.dflt) - or maybe when hole > 0.5 or something... would be strange to put the title in the center when the hole is very small!

But I'm not wedded to that idea; if you've considered it and still think 'top center' should be the default in all cases we can keep it that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


if(titlePosition === 'inhole' || titlePosition === 'outside') {
codrut3 marked this conversation as resolved.
Show resolved Hide resolved
coerceFont(coerce, 'titlefont', layout.font);
}
}

handleDomainDefaults(traceOut, layout, coerce);

coerce('hole');
Expand Down
126 changes: 118 additions & 8 deletions src/traces/pie/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,53 @@ module.exports = function plot(gd, cdpie) {
});
});

// add the title
var hasTitle = trace.title &&
((trace.titleposition === 'inhole' && trace.hole > 0) ||
(trace.titleposition === 'outside'));
var titleTextGroup = d3.select(this).selectAll('g.titletext')
.data(hasTitle ? [0] : []);

titleTextGroup.enter().append('g')
.classed('titletext', true);
titleTextGroup.exit().remove();

titleTextGroup.each(function() {
var titleText = Lib.ensureSingle(d3.select(this), 'text', '', function(s) {
// prohibit tex interpretation as above
s.attr('data-notex', 1);
});

titleText.text(trace.title)
.attr({
'class': 'titletext',
transform: '',
'text-anchor': 'middle'
})
.call(Drawing.font, trace.titlefont)
.call(svgTextUtils.convertToTspans, gd);


var titleBB = Drawing.bBox(titleText.node());
// translation and scaling for the title text box.
// The translation is for the center point.
var transform;

if(trace.titleposition === 'outside') {
transform = positionTitleOutside(titleBB, cd0, fullLayout._size);
} else {
transform = positionTitleInside(titleBB, cd0);
}

titleText.attr('transform',
'translate(' + transform.x + ',' + transform.y + ')' +
(transform.scale < 1 ? ('scale(' + transform.scale + ')') : '') +
'translate(' +
(-(titleBB.left + titleBB.right) / 2) + ',' +
(-(titleBB.top + titleBB.bottom) / 2) +
')');
});

// now make sure no labels overlap (at least within one pie)
if(hasOutsideText) scootLabels(quadrants, trace);
slices.each(function(pt) {
Expand Down Expand Up @@ -454,6 +501,72 @@ function transformOutsideText(textBB, pt) {
};
}

function positionTitleInside(titleBB, cd0) {
var textDiameter = Math.sqrt(titleBB.width * titleBB.width + titleBB.height * titleBB.height);
return {
x: cd0.cx,
y: cd0.cy,
scale: cd0.trace.hole * cd0.r * 2 / textDiameter
};
}

function positionTitleOutside(titleBB, cd0, plotSize) {
var scaleX, scaleY, chartWidth, titleSpace, titleShift, maxPull;
var trace = cd0.trace;

maxPull = getMaxPull(trace);
chartWidth = plotSize.w * (trace.domain.x[1] - trace.domain.x[0]);
scaleX = chartWidth / titleBB.width;
if(isSinglePie(trace)) {
titleShift = trace.titlefont.size / 2;
// we need to leave enough free space for an outside label
if(trace.outsidetextfont) titleShift += 3 * trace.outsidetextfont.size / 2;
else titleShift += trace.titlefont.size / 4;
return {
x: cd0.cx,
y: cd0.cy - (1 + maxPull) * cd0.r - titleShift,
scale: scaleX
};
}
titleSpace = getTitleSpace(trace, plotSize);
// we previously left a free space of height titleSpace.
// The text must fit in this space.
scaleY = titleSpace / titleBB.height;
return {
x: cd0.cx,
y: cd0.cy - (1 + maxPull) * cd0.r - (titleSpace / 2),
scale: Math.min(scaleX, scaleY)
};
}

function isSinglePie(trace) {
// check if there is a single pie per y-column
if(trace.domain.y[0] === 0 && trace.domain.y[1] === 1) return true;
codrut3 marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

function getTitleSpace(trace, plotSize) {
var chartHeight = plotSize.h * (trace.domain.y[1] - trace.domain.y[0]);
// leave 3/2 * titlefont.size free space. We need at least titlefont.size
// space, and the 1/2 * titlefont.size is a small buffer to avoid the text
// touching the pie.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be based on titleBB.height - since you can include 'multi<br>line<br>titles'
But a buffer of half the font size is still good.

Actually, in principle anything that depends on the size of text needs to happen in the (potentially async) callback 3rd argument to convertToTspans, in order to handle MathJax (title: '$x+y=z$' etc). That may throw a bit of a wrench into this whole process, since scalePies happens so early on in the process... seems like perhaps all the pie titles need to be rendered upfront, stashed somewhere, and all the existing plotting happens in their combined completion callback. Super annoying, I know...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I handled multiline titles.
Making it work for MathJax is much harder. Maybe I could just use data-notex?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per #2987 (comment) - yes, just use data-notex.

var titleSpace = (trace.title && trace.titleposition === 'outside') ?
(3 * trace.titlefont.size / 2) : 0;
if(chartHeight > titleSpace) return titleSpace;
else return chartHeight / 2;
}

function getMaxPull(trace) {
var maxPull = trace.pull, j;
if(Array.isArray(maxPull)) {
maxPull = 0;
for(j = 0; j < trace.pull.length; j++) {
if(trace.pull[j] > maxPull) maxPull = trace.pull[j];
}
}
return maxPull;
}

function scootLabels(quadrants, trace) {
var xHalf, yHalf, equatorFirst, farthestX, farthestY,
xDiffSign, yDiffSign, thisQuad, oppositeQuad,
Expand Down Expand Up @@ -570,21 +683,18 @@ function scalePies(cdpie, plotSize) {
for(i = 0; i < cdpie.length; i++) {
cd0 = cdpie[i][0];
trace = cd0.trace;

pieBoxWidth = plotSize.w * (trace.domain.x[1] - trace.domain.x[0]);
pieBoxHeight = plotSize.h * (trace.domain.y[1] - trace.domain.y[0]);
// leave some space for the title, if it will be displayed outside
if(!isSinglePie(trace)) pieBoxHeight -= getTitleSpace(trace, plotSize);

maxPull = trace.pull;
if(Array.isArray(maxPull)) {
maxPull = 0;
for(j = 0; j < trace.pull.length; j++) {
if(trace.pull[j] > maxPull) maxPull = trace.pull[j];
}
}
maxPull = getMaxPull(trace);

cd0.r = Math.min(pieBoxWidth, pieBoxHeight) / (2 + 2 * maxPull);

cd0.cx = plotSize.l + plotSize.w * (trace.domain.x[1] + trace.domain.x[0]) / 2;
cd0.cy = plotSize.t + plotSize.h * (2 - trace.domain.y[1] - trace.domain.y[0]) / 2;
cd0.cy = plotSize.t + plotSize.h * (1 - trace.domain.y[0]) - pieBoxHeight / 2;

if(trace.scalegroup && scaleGroups.indexOf(trace.scalegroup) === -1) {
scaleGroups.push(trace.scalegroup);
Expand Down
Binary file added test/image/baselines/pie_title_inhole.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/image/baselines/pie_title_multiple.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/image/baselines/pie_title_pull.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
18 changes: 18 additions & 0 deletions test/image/mocks/pie_title_inhole.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"data": [
{
"values": [955, 405, 360, 310, 295],
"labels": ["Mandarin", "Spanish", "English", "Hindi", "Arabic"],
"textinfo": "label+percent",
"hole": 0.3,
"title": "Num. speakers",
codrut3 marked this conversation as resolved.
Show resolved Hide resolved
"titleposition": "inhole",
"type": "pie"
}
],
"layout": {
"title": "Top 5 languages by number of native speakers (2010, est.)",
"height": 600,
"width": 600
}
}
58 changes: 58 additions & 0 deletions test/image/mocks/pie_title_multiple.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
{
"data": [
{
"values": [38600, 83100, 11100, 15400, 1740, 77],
"labels": ["Platinum", "Palladium", "Rhodium", "Ruthenium", "Iridium", "Osmium"],
"type": "pie",
"name": "Year 2013",
"title": "Year 2013",
"domain": {
"x": [0, 0.5],
"y": [0.51, 1]
},
"hoverinfo": "label+percent+name",
"textinfo": "none"
},{
"values": [45800, 92900, 11100, 11000, 1960, 322],
"labels": ["Platinum", "Palladium", "Rhodium", "Ruthenium", "Iridium", "Osmium"],
"type": "pie",
"name": "Year 2014",
"title": "Year 2014",
"domain": {
"x": [0.51, 1],
"y": [0.51, 1]
},
"hoverinfo": "label+percent+name",
"textinfo": "none"
},{
"values": [42700, 85300, 10600, 8230, 1010, 8],
"labels": ["Platinum", "Palladium", "Rhodium", "Ruthenium", "Iridium", "Osmium"],
"type": "pie",
"name": "Year 2015",
"title": "Year 2015",
"domain": {
"x": [0, 0.5],
"y": [0, 0.5]
},
"hoverinfo": "label+percent+name",
"textinfo": "none"
},{
"values": [42300, 80400, 10700, 8410, 1300, 27],
"labels": ["Platinum", "Palladium", "Rhodium", "Ruthenium", "Iridium", "Osmium"],
"type": "pie",
"name": "Year 2016",
"title": "Year 2016",
"domain": {
"x": [0.51, 1],
"y": [0, 0.5]
},
"hoverinfo": "label+percent+name",
"textinfo": "none"
}
],
"layout": {
"title": "U.S. Imports for Platinum-group Metals",
"height": 400,
"width": 500
}
}
21 changes: 21 additions & 0 deletions test/image/mocks/pie_title_pull.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"data": [
{
"values": [
50, 49, 48, 47, 46, 45, 44, 43, 42, 41
],
"pull": [
0, 0.5, 0, 0.5, 0, 0.5, 0, 0.5, 0, 0.5
codrut3 marked this conversation as resolved.
Show resolved Hide resolved
],
"sort": false,
"type": "pie",
"textposition": "inside",
"title": "Withering Flower"
}
],
"layout": {
"height": 600,
"width": 400,
"showlegend": false
}
}
47 changes: 47 additions & 0 deletions test/jasmine/tests/pie_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,53 @@ describe('Pie traces:', function() {
.catch(failTest)
.then(done);
});

it('shows title in hole', function(done) {
Plotly.newPlot(gd, [{
values: [2, 2, 2, 2],
title: 'Pie',
titleposition: 'inhole',
hole: 0.5,
type: 'pie',
}], {height: 300, width: 300})
.then(function() {
var title = d3.selectAll('.titletext text');
expect(title.size()).toBe(1);
})
.catch(failTest)
.then(done);
});

it('does not show title inside if there is no hole', function(done) {
Plotly.newPlot(gd, [{
values: [2, 2, 2, 2],
title: 'Pie',
titleposition: 'inhole',
hole: 0,
type: 'pie',
}], {height: 300, width: 300})
.then(function() {
var title = d3.selectAll('.titletext text');
expect(title.size()).toBe(0);
codrut3 marked this conversation as resolved.
Show resolved Hide resolved
})
.catch(failTest)
.then(done);
});

it('shows title outside', function(done) {
Plotly.newPlot(gd, [{
values: [1, 1, 1, 1, 2],
title: 'Pie',
titleposition: 'outside',
type: 'pie',
}], {height: 300, width: 300})
.then(function() {
var title = d3.selectAll('.titletext text');
expect(title.size()).toBe(1);
})
.catch(failTest)
.then(done);
});
});

describe('pie hovering', function() {
Expand Down