Skip to content

Commit a27dc29

Browse files
archmojarchmoj
committed
Consider first review
- make pie insidetextorientation coerce condition more readable - no need for extra check before recompute bounding box - revise uniformtext attribute descriptions - remove boolean from getTextTransform arguments - add js doc description - create and use Lib.ensureUniformFontSize - make the hide/show logic independent of _module.plot - refactor textposition coerce logic - add test for inside-text-orientation coerce logic with various textposition scenarios - use radial tangential and horizontal in inside-text-orientation keys
1 parent 4d5fb3d commit a27dc29

18 files changed

+101
-91
lines changed

‎src/lib/index.js

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,13 +1182,17 @@ lib.isHidden = function(gd) {
11821182
return !display || display === 'none';
11831183
};
11841184

1185-
lib.getTextTransform = function(opts, isCenter) {
1186-
var textX = opts.textX;
1187-
var textY = opts.textY;
1188-
var targetX = opts.targetX;
1189-
var targetY = opts.targetY;
1190-
var rotate = opts.rotate;
1191-
var scale = opts.scale;
1185+
/** Return transform text for bar bar-like rectangles and pie-like slices
1186+
* @param {object} transform
1187+
*/
1188+
lib.getTextTransform = function(transform) {
1189+
var noCenter = transform.noCenter;
1190+
var textX = transform.textX;
1191+
var textY = transform.textY;
1192+
var targetX = transform.targetX;
1193+
var targetY = transform.targetY;
1194+
var rotate = transform.rotate;
1195+
var scale = transform.scale;
11921196
if(!scale) scale = 0;
11931197
else if(scale > 1) scale = 1;
11941198

@@ -1198,14 +1202,21 @@ lib.getTextTransform = function(opts, isCenter) {
11981202
(targetY - scale * textY) +
11991203
')' +
12001204
(scale < 1 ?
1201-
'scale(' + scale + ')' :
1202-
''
1205+
'scale(' + scale + ')' : ''
12031206
) +
12041207
(rotate ?
12051208
'rotate(' + rotate +
1206-
(isCenter ? '' : ' ' + textX + ' ' + textY) +
1207-
')' :
1208-
''
1209+
(noCenter ? '' : ' ' + textX + ' ' + textY) +
1210+
')' : ''
12091211
)
12101212
);
12111213
};
1214+
1215+
lib.ensureUniformFontSize = function(gd, baseFont) {
1216+
var out = lib.extendFlat({}, baseFont);
1217+
out.size = Math.max(
1218+
baseFont.size,
1219+
gd._fullLayout.uniformtext.minsize || 0
1220+
);
1221+
return out;
1222+
};

‎src/plots/layout_attributes.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,11 @@ module.exports = {
151151
'Determines how the font size for various text',
152152
'elements are uniformed between each trace type.',
153153
'If the computed text sizes were smaller than',
154-
'the minimum size defined by `minsize`',
154+
'the minimum size defined by `uniformtext.minsize`',
155155
'using *hide* option hides the text; and',
156156
'using *show* option shows the text without further downscaling.',
157157
'Please note that if the size defined by `minsize` is greater than',
158-
'the font size defined by trace, the `minsize` would be used.'
158+
'the font size defined by trace, then the `minsize` is used.'
159159
].join(' ')
160160
},
161161
minsize: {

‎src/traces/bar/plot.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,8 +329,7 @@ function appendBarText(gd, plotinfo, bar, cd, i, x0, x1, y0, y1, opts, makeOnCom
329329
// draw text using insideTextFont and check if it fits inside bar
330330
textPosition = 'inside';
331331

332-
font = Lib.extendFlat({}, insideTextFont, {});
333-
font.size = Math.max(font.size, fullLayout.uniformtext.minsize || 0);
332+
font = Lib.ensureUniformFontSize(gd, insideTextFont);
334333

335334
textSelection = appendTextNode(bar, text, font);
336335

@@ -362,8 +361,7 @@ function appendBarText(gd, plotinfo, bar, cd, i, x0, x1, y0, y1, opts, makeOnCom
362361
}
363362

364363
if(!textSelection) {
365-
font = Lib.extendFlat({}, (textPosition === 'outside') ? outsideTextFont : insideTextFont, {});
366-
font.size = Math.max(font.size, fullLayout.uniformtext.minsize || 0);
364+
font = Lib.ensureUniformFontSize(gd, (textPosition === 'outside') ? outsideTextFont : insideTextFont);
367365

368366
textSelection = appendTextNode(bar, text, font);
369367

‎src/traces/bar/style.js

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,11 @@ function resizeText(gd, gTrace, traceType) {
3838
t = gTrace.selectAll('g.points').selectAll('g.point').selectAll('text');
3939
}
4040

41-
var isCenter = (
42-
traceType === 'pie' ||
43-
traceType === 'sunburst'
44-
);
45-
4641
t.each(function(d) {
4742
var transform = d.transform;
48-
4943
transform.scale = minSize / transform.fontSize;
50-
d3.select(this).attr('transform', Lib.getTextTransform(transform, isCenter));
51-
52-
if(shouldHide && transform.hide) {
53-
d3.select(this).text(' ');
54-
}
44+
d3.select(this).attr('transform', Lib.getTextTransform(transform));
45+
d3.select(this).attr('display', shouldHide && transform.hide ? 'none' : null);
5546
});
5647
}
5748
}
@@ -95,8 +86,7 @@ function stylePoints(sel, trace, gd) {
9586
function styleTextPoints(sel, trace, gd) {
9687
sel.selectAll('text').each(function(d) {
9788
var tx = d3.select(this);
98-
var font = Lib.extendFlat({}, determineFont(tx, d, trace, gd), {});
99-
font.size = Math.max(font.size, gd._fullLayout.uniformtext.minsize || 0);
89+
var font = Lib.ensureUniformFontSize(gd, determineFont(tx, d, trace, gd));
10090

10191
Drawing.font(tx, font);
10292
});
@@ -124,8 +114,7 @@ function styleTextInSelectionMode(txs, trace, gd) {
124114
var font;
125115

126116
if(d.selected) {
127-
font = Lib.extendFlat({}, determineFont(tx, d, trace, gd));
128-
font.size = Math.max(font.size, gd._fullLayout.uniformtext.minsize || 0);
117+
font = Lib.ensureUniformFontSize(gd, determineFont(tx, d, trace, gd));
129118

130119
var selectedFontColor = trace.selected.textfont && trace.selected.textfont.color;
131120
if(selectedFontColor) {

‎src/traces/funnelarea/plot.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,7 @@ module.exports = function plot(gd, cdModule) {
9696
s.attr('data-notex', 1);
9797
});
9898

99-
var font = Lib.extendFlat({}, determineInsideTextFont(trace, pt, fullLayout.font), {});
100-
font.size = Math.max(font.size, fullLayout.uniformtext.minsize || 0);
99+
var font = Lib.ensureUniformFontSize(gd, determineInsideTextFont(trace, pt, fullLayout.font));
101100

102101
sliceText.text(pt.text)
103102
.attr({

‎src/traces/pie/attributes.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,16 +184,16 @@ module.exports = {
184184
insidetextorientation: {
185185
valType: 'enumerated',
186186
role: 'info',
187-
values: ['h', 'r', 't', 'auto'],
187+
values: ['horizontal', 'radial', 'tangential', 'auto'],
188188
dflt: 'auto',
189189
editType: 'plot',
190190
description: [
191191
'Determines the orientation of text inside slices.',
192192
'With *auto* the texts may automatically be',
193193
'rotated to fit with the maximum size inside the slice.',
194-
'Using *h* option forces text to be horizontal.',
195-
'Using *r* option forces text to be radial.',
196-
'Using *t* option forces text to be tangential.'
194+
'Using *horizontal* option forces text to be horizontal.',
195+
'Using *radial* option forces text to be radial.',
196+
'Using *tangential* option forces text to be tangential.'
197197
].join(' ')
198198
},
199199
insidetextfont: extendFlat({}, textFontAttrs, {

‎src/traces/pie/defaults.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
7171
coerce('automargin');
7272
}
7373

74-
if(textposition !== 'none' && textposition !== 'outside') {
74+
if(textposition === 'inside' || textposition === 'auto' || Array.isArray(textposition)) {
7575
coerce('insidetextorientation');
7676
}
7777
}

‎src/traces/pie/plot.js

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,10 @@ function plot(gd, cdModule) {
145145
s.attr('data-notex', 1);
146146
});
147147

148-
var font = Lib.extendFlat({}, textPosition === 'outside' ?
148+
var font = Lib.ensureUniformFontSize(gd, textPosition === 'outside' ?
149149
determineOutsideTextFont(trace, pt, fullLayout.font) :
150-
determineInsideTextFont(trace, pt, fullLayout.font), {}
150+
determineInsideTextFont(trace, pt, fullLayout.font)
151151
);
152-
font.size = Math.max(font.size, fullLayout.uniformtext.minsize || 0);
153152

154153
sliceText.text(pt.text)
155154
.attr({
@@ -169,14 +168,11 @@ function plot(gd, cdModule) {
169168
} else {
170169
transform = transformInsideText(textBB, pt, cd0);
171170
if(textPosition === 'auto' && transform.scale < 1) {
172-
var newFont = Lib.extendFlat({}, trace.outsidetextfont, {});
173-
newFont.size = Math.max(newFont.size, fullLayout.uniformtext.minsize || 0);
171+
var newFont = Lib.ensureUniformFontSize(gd, trace.outsidetextfont);
174172

175173
sliceText.call(Drawing.font, newFont);
176-
if(newFont.family !== font.family || newFont.size !== font.size) {
177-
// recompute bounding box
178-
textBB = Drawing.bBox(sliceText.node());
179-
}
174+
textBB = Drawing.bBox(sliceText.node());
175+
180176
transform = transformOutsideText(textBB, pt);
181177
}
182178
}
@@ -201,7 +197,7 @@ function plot(gd, cdModule) {
201197
recordMinTextSize(trace.type, transform, fullLayout);
202198
cd[i].transform = transform;
203199

204-
sliceText.attr('transform', Lib.getTextTransform(transform, true));
200+
sliceText.attr('transform', Lib.getTextTransform(transform));
205201
});
206202
});
207203

@@ -565,11 +561,14 @@ function transformInsideText(textBB, pt, cd0) {
565561
var rInscribed = pt.rInscribed;
566562
var r = cd0.r || pt.rpx1;
567563
var orientation = cd0.trace.insidetextorientation;
568-
var allTransforms = [];
569-
564+
var isHorizontal = orientation === 'horizontal';
565+
var isTangential = orientation === 'tangential';
566+
var isRadial = orientation === 'radial';
567+
var isAuto = orientation === 'auto';
570568
var isCircle = (ring === 1) && (Math.abs(pt.startangle - pt.stopangle) === Math.PI * 2);
569+
var allTransforms = [];
571570

572-
if(isCircle || orientation === 'auto' || orientation === 'h') {
571+
if(isCircle || isAuto || isHorizontal) {
573572
// max size text can be inserted inside without rotating it
574573
// this inscribes the text rectangle in a circle, which is then inscribed
575574
// in the slice, so it will be an underestimate, which some day we may want
@@ -587,7 +586,7 @@ function transformInsideText(textBB, pt, cd0) {
587586
allTransforms.push(transform);
588587
}
589588

590-
if(orientation === 'h') {
589+
if(isHorizontal) {
591590
// max size if text is placed (horizontally) at the top or bottom of the arc
592591

593592
var considerCrossing = function(angle, key) {
@@ -603,32 +602,33 @@ function transformInsideText(textBB, pt, cd0) {
603602
} else { // case of 'rad'
604603
newT = calcRadTransform(textBB, r, ring, closestEdge, Math.PI / 2);
605604
}
606-
newT._repos = getCoords(r, angle);
605+
newT.pxtxt = getCoords(r, angle);
607606

608607
allTransforms.push(newT);
609608
}
610609
};
611610

612611
for(var i = 3; i >= -3; i--) { // to cover all cases with trace.rotation added
613-
considerCrossing(Math.PI * (i + 0.0), 'tan');
612+
considerCrossing(Math.PI * i, 'tan');
614613
considerCrossing(Math.PI * (i + 0.5), 'rad');
615614
}
616615
}
617616

618-
if(orientation === 'auto' || orientation === 'r') {
617+
if(isAuto || isRadial) {
619618
allTransforms.push(calcRadTransform(textBB, r, ring, halfAngle, midAngle));
620619
}
621620

622-
if(orientation === 'auto' || orientation === 't') {
621+
if(isAuto || isTangential) {
623622
allTransforms.push(calcTanTransform(textBB, r, ring, halfAngle, midAngle));
624623
}
625624

626625
var maxScaleTransform = allTransforms.sort(function(a, b) {
627626
return b.scale - a.scale;
628627
})[0];
629628

630-
if(maxScaleTransform._repos) {
631-
pt.pxtxt = maxScaleTransform._repos;
629+
if(maxScaleTransform.pxtxt) {
630+
// copy text position if not at the middle
631+
pt.pxtxt = maxScaleTransform.pxtxt;
632632
}
633633

634634
return maxScaleTransform;
@@ -1119,6 +1119,7 @@ function computeTransform(
11191119
var midY = (textBB.top + textBB.bottom) / 2;
11201120
transform.textX = midX * cosA - midY * sinA;
11211121
transform.textY = midX * sinA + midY * cosA;
1122+
transform.noCenter = true;
11221123
}
11231124

11241125
module.exports = {

‎src/traces/sunburst/plot.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,7 @@ function plotOne(gd, cd, element, transitionOpts) {
253253
s.attr('data-notex', 1);
254254
});
255255

256-
var font = Lib.extendFlat({}, helpers.determineTextFont(trace, pt, fullLayout.font), {});
257-
font.size = Math.max(font.size, fullLayout.uniformtext.minsize || 0);
256+
var font = Lib.ensureUniformFontSize(gd, helpers.determineTextFont(trace, pt, fullLayout.font));
258257

259258
sliceText.text(exports.formatSliceLabel(pt, entry, trace, cd, fullLayout))
260259
.classed('slicetext', true)

‎src/traces/treemap/draw_ancestors.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,7 @@ module.exports = function drawAncestors(gd, cd, entry, slices, opts) {
141141
s.attr('data-notex', 1);
142142
});
143143

144-
var font = Lib.extendFlat({}, helpers.determineTextFont(trace, pt, fullLayout.font, trace.pathdir), {});
145-
font.size = Math.max(font.size, fullLayout.uniformtext.minsize || 0);
144+
var font = Lib.ensureUniformFontSize(gd, helpers.determineTextFont(trace, pt, fullLayout.font, trace.pathdir));
146145

147146
sliceText.text(pt._text || ' ') // use one space character instead of a blank string to avoid jumps during transition
148147
.classed('slicetext', true)

‎src/traces/treemap/draw_descendants.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,7 @@ module.exports = function drawDescendants(gd, cd, entry, slices, opts) {
182182
s.attr('data-notex', 1);
183183
});
184184

185-
var font = Lib.extendFlat({}, helpers.determineTextFont(trace, pt, fullLayout.font), {});
186-
font.size = Math.max(font.size, fullLayout.uniformtext.minsize || 0);
185+
var font = Lib.ensureUniformFontSize(gd, helpers.determineTextFont(trace, pt, fullLayout.font));
187186

188187
sliceText.text(pt._text || ' ') // use one space character instead of a blank string to avoid jumps during transition
189188
.classed('slicetext', true)

‎test/image/mocks/pie_inside-text-orientation.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"size": 20
99
}
1010
},
11-
"insidetextorientation": "h",
11+
"insidetextorientation": "horizontal",
1212
"textposition": "inside",
1313
"type": "pie",
1414
"hole": 0.5,
@@ -117,7 +117,7 @@
117117
"size": 20
118118
}
119119
},
120-
"insidetextorientation": "r",
120+
"insidetextorientation": "radial",
121121
"textposition": "inside",
122122
"type": "pie",
123123
"hole": 0.5,
@@ -197,7 +197,7 @@
197197
"size": 20
198198
}
199199
},
200-
"insidetextorientation": "t",
200+
"insidetextorientation": "tangential",
201201
"textposition": "inside",
202202
"type": "pie",
203203
"hole": 0.5,

‎test/image/mocks/sunburst_inside-text-orientation.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"data": [
33
{
44
"name": "horizontal",
5-
"insidetextorientation": "h",
5+
"insidetextorientation": "horizontal",
66
"type": "sunburst",
77
"level": "Juliet",
88
"parents": [
@@ -105,7 +105,7 @@
105105
},
106106
{
107107
"name": "radial",
108-
"insidetextorientation": "r",
108+
"insidetextorientation": "radial",
109109
"type": "sunburst",
110110
"level": "Juliet",
111111
"parents": [
@@ -179,7 +179,7 @@
179179
},
180180
{
181181
"name": "tangential",
182-
"insidetextorientation": "t",
182+
"insidetextorientation": "tangential",
183183
"type": "sunburst",
184184
"level": "Juliet",
185185
"parents": [

0 commit comments

Comments
 (0)