Skip to content

Fix uniformtext and enable coloraxis for sunburst and treemap as well as pathbar.textfont #4444

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

Merged
merged 16 commits into from
Jan 6, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
10 changes: 9 additions & 1 deletion src/traces/bar/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ function handleText(traceIn, traceOut, layout, coerce, textposition, opts) {
var moduleHasCliponaxis = !(opts.moduleHasCliponaxis === false);
var moduleHasTextangle = !(opts.moduleHasTextangle === false);
var moduleHasInsideanchor = !(opts.moduleHasInsideanchor === false);
var hasPathbar = !!opts.hasPathbar;

var hasBoth = Array.isArray(textposition) || textposition === 'auto';
var hasInside = hasBoth || textposition === 'inside';
Expand All @@ -147,8 +148,15 @@ function handleText(traceIn, traceOut, layout, coerce, textposition, opts) {
}
coerceFont(coerce, 'insidetextfont', insideTextFontDefault);

if(hasOutside) coerceFont(coerce, 'outsidetextfont', dfltFont);
if(hasPathbar) {
var pathbarTextFontDefault = Lib.extendFlat({}, dfltFont);
if(isColorInheritedFromLayoutFont) {
delete pathbarTextFontDefault.color;
}
coerceFont(coerce, 'pathbar.textfont', pathbarTextFontDefault);
}

if(hasOutside) coerceFont(coerce, 'outsidetextfont', dfltFont);

if(moduleHasSelected) coerce('selected.textfont.color');
if(moduleHasUnselected) coerce('unselected.textfont.color');
Expand Down
8 changes: 4 additions & 4 deletions src/traces/bar/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,19 @@ function resizeText(gd, gTrace, traceType) {
if(minSize) {
var shouldHide = fullLayout.uniformtext.mode === 'hide';

var t;
var selector;
switch(traceType) {
case 'funnelarea' :
case 'pie' :
case 'sunburst' :
case 'treemap' :
t = gTrace.selectAll('g.slice');
selector = 'g.slice';
break;
default :
t = gTrace.selectAll('g.points > g.point');
selector = 'g.points > g.point';
}

t.each(function(d) {
gTrace.selectAll(selector).each(function(d) {
var transform = d.transform;
if(transform) {
transform.scale = (shouldHide && transform.hide) ? 0 : minSize / transform.fontSize;
Expand Down
23 changes: 15 additions & 8 deletions src/traces/sunburst/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,16 @@ function determineOutsideTextFont(trace, pt, layoutFont) {
};
}

function determineInsideTextFont(trace, pt, layoutFont, cont) {
function determineInsideTextFont(trace, pt, layoutFont, opts) {
var onPathbar = (opts || {}).onPathbar;

var cdi = pt.data.data;
var ptNumber = cdi.i;

var customColor = Lib.castOption(trace, ptNumber, 'insidetextfont.color');
var customColor = Lib.castOption(trace, ptNumber,
(onPathbar ? 'pathbar.textfont' : 'insidetextfont') + '.color'
);

if(!customColor && trace._input.textfont) {
// Why not simply using trace.textfont? Because if not set, it
// defaults to layout.font which has a default color. But if
Expand All @@ -98,16 +103,18 @@ function determineInsideTextFont(trace, pt, layoutFont, cont) {

return {
color: customColor || Color.contrast(cdi.color),
family: exports.getInsideTextFontKey('family', cont || trace, pt, layoutFont),
size: exports.getInsideTextFontKey('size', cont || trace, pt, layoutFont)
family: exports.getInsideTextFontKey('family', trace, pt, layoutFont, opts),
size: exports.getInsideTextFontKey('size', trace, pt, layoutFont, opts)
};
}

exports.getInsideTextFontKey = function(keyStr, trace, pt, layoutFont) {
exports.getInsideTextFontKey = function(keyStr, trace, pt, layoutFont, opts) {
var onPathbar = (opts || {}).onPathbar;
var cont = onPathbar ? 'pathbar.textfont' : 'insidetextfont';
var ptNumber = pt.data.data.i;

return (
Lib.castOption(trace, ptNumber, 'insidetextfont.' + keyStr) ||
Lib.castOption(trace, ptNumber, cont + '.' + keyStr) ||
Lib.castOption(trace, ptNumber, 'textfont.' + keyStr) ||
layoutFont.size
);
Expand All @@ -127,10 +134,10 @@ exports.isOutsideText = function(trace, pt) {
return !trace._hasColorscale && exports.isHierarchyRoot(pt);
};

exports.determineTextFont = function(trace, pt, layoutFont, cont) {
exports.determineTextFont = function(trace, pt, layoutFont, opts) {
return exports.isOutsideText(trace, pt) ?
determineOutsideTextFont(trace, pt, layoutFont) :
determineInsideTextFont(trace, pt, layoutFont, cont);
determineInsideTextFont(trace, pt, layoutFont, opts);
};

exports.hasTransition = function(transitionOpts) {
Expand Down
6 changes: 3 additions & 3 deletions src/traces/treemap/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
coerce('hovertext');
coerce('hovertemplate');

var hasPathbar = coerce('pathbar.visible');

var textposition = 'auto';
handleText(traceIn, traceOut, layout, coerce, textposition, {
hasPathbar: hasPathbar,
moduleHasSelected: false,
moduleHasUnselected: false,
moduleHasConstrain: false,
Expand Down Expand Up @@ -103,10 +106,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
}
};

var hasPathbar = coerce('pathbar.visible');
if(hasPathbar) {
Lib.coerceFont(coerce, 'pathbar.textfont', layout.font);

// This works even for multi-line labels as treemap pathbar trim out line breaks
coerce('pathbar.thickness', traceOut.pathbar.textfont.size + 2 * TEXTPAD);

Expand Down
18 changes: 4 additions & 14 deletions src/traces/treemap/draw_ancestors.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ module.exports = function drawAncestors(gd, cd, entry, slices, opts) {
}

updateSlices.each(function(pt) {
pt._hoverX = viewX(pt.x1 - height / 2);
pt._hoverX = viewX(pt.x1 - Math.min(width, height) / 2);
pt._hoverY = viewY(pt.y1 - height / 2);

var sliceTop = d3.select(this);
Expand Down Expand Up @@ -141,7 +141,9 @@ module.exports = function drawAncestors(gd, cd, entry, slices, opts) {
s.attr('data-notex', 1);
});

var font = Lib.ensureUniformFontSize(gd, helpers.determineTextFont(trace, pt, fullLayout.font, trace.pathdir));
var font = Lib.ensureUniformFontSize(gd, helpers.determineTextFont(trace, pt, fullLayout.font, {
onPathbar: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why this commit made three existing mocks change (treemap_level-depth, treemap_packings and treemap_textposition) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question.
And a bad mistake here.
There is no such a thing as pathdir!
It should have been written pathbar.

}));

sliceText.text(pt._text || ' ') // use one space character instead of a blank string to avoid jumps during transition
.classed('slicetext', true)
Expand All @@ -154,20 +156,8 @@ module.exports = function drawAncestors(gd, cd, entry, slices, opts) {
fontSize: font.size,
onPathbar: true
});

pt.transform.fontSize = font.size;

if(helpers.isOutsideText(trace, pt)) {
// consider in/out diff font sizes
var outsideFont = helpers.getOutsideTextFontKey('size', trace, pt, fullLayout.font);
var insideFont = helpers.getInsideTextFontKey('size', trace, pt, fullLayout.font);

var diffFontSize = outsideFont - insideFont;

pt.transform.targetY -= diffFontSize;
pt.transform.fontSize -= diffFontSize;
}

if(hasTransition) {
sliceText.transition().attrTween('transform', function(pt2) {
var interp = makeUpdateTextInterpolator(pt2, onPathbar, refRect, [width, height]);
Expand Down
Binary file added test/image/baselines/treemap_fonts_nocolor.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/treemap_level-depth.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 modified test/image/baselines/treemap_packings.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 modified test/image/baselines/treemap_textposition.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading