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

Treemap new trace type #4185

Merged
merged 63 commits into from
Sep 25, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
ec6a9de
bar textpad const
archmoj Aug 28, 2019
52ded31
refactor base_plot files
archmoj Aug 29, 2019
320f81a
changes to sunburst attributes - add percentages - add colorscale and…
archmoj Aug 29, 2019
06d93ae
prep to add treemap to the lib
archmoj Aug 29, 2019
6cc8590
treemap mocks
archmoj Aug 28, 2019
6a595b6
treemap jasmine tests
archmoj Aug 28, 2019
671e864
treemap code
archmoj Sep 11, 2019
648167d
correct current path when there is no parent
archmoj Sep 11, 2019
6dad958
various attribute revisions
archmoj Sep 13, 2019
cceee49
drop stroke-linejoin
archmoj Sep 13, 2019
f3d38b8
revisit pathbar draw as well as transition
archmoj Sep 13, 2019
088b0e0
revert sunburst hoverinfo defaults
archmoj Sep 13, 2019
e23a577
move attachFxHandlers to fx.js
archmoj Sep 13, 2019
eb54f24
pass hover and transition options to fx
archmoj Sep 13, 2019
0e3185f
expand setSliceCursor logic to handle treemap and pathbar cases
archmoj Sep 13, 2019
fdd69e7
drop stroke-linejoin again
archmoj Sep 13, 2019
a7b6074
use constant _hoverd.marker.line.width - improve a treemap mock and u…
archmoj Sep 14, 2019
4c85360
correct transition of text inside pathbar namely when side is bottom
archmoj Sep 14, 2019
65c0e02
display percent of root on leaf when it is an entry
archmoj Sep 14, 2019
ca4b385
some refRect clean up and rename interpolator functions in sunburst a…
archmoj Sep 14, 2019
0012fa1
make the opacity logic slightly more user-friendly by applying leaf.o…
archmoj Sep 14, 2019
544977e
thanks to a typo leading to apply a better easing option suitable for…
archmoj Sep 14, 2019
13e0449
dont use sunburst naming in treemap
archmoj Sep 14, 2019
57dec38
move getTransform function to Lib and use it in bar and treemap
archmoj Sep 14, 2019
65f0213
correct exit case of zoom out when maxdepth is set - avoid overlaps a…
archmoj Sep 15, 2019
ad24987
when exiting with maxdepth try to use parent if that is visible on th…
archmoj Sep 16, 2019
36fb510
early exit out of maxdepth elements for better transitions and avoid …
archmoj Sep 16, 2019
284bf6d
add constant zero fillet - could expose that in next version
archmoj Sep 16, 2019
4e2c328
add default tests of treemap and sunburst
archmoj Sep 16, 2019
0cf5bf6
remove the case that never happens now
archmoj Sep 16, 2019
2405ba4
revisit pathbar divider default
archmoj Sep 16, 2019
19c7a61
[wip] add test for sunburst tween edge cases
etpinard Sep 17, 2019
4e3b7f3
address various treemap transition case e.g. maxdepth, tweening and p…
archmoj Sep 19, 2019
e5181ac
texttemplate and other fixes for treemap and sunburst
archmoj Sep 19, 2019
e9c0202
add calc test for count branches and leaves
archmoj Sep 19, 2019
e4bc91e
treemap drop leaf.opacity
archmoj Sep 19, 2019
a54ca9c
better description regarding the position of pathbar also move the ga…
archmoj Sep 19, 2019
d7b06a1
change divider to edgeshape
archmoj Sep 19, 2019
c91f9db
revisit ancestors draw - avoid overlaps - end pathbar with edgeshape
archmoj Sep 19, 2019
ceb13f7
tween and animate text for new points on pathbar from their parent
archmoj Sep 19, 2019
1270d67
draw new points of pathbar below parents (to avoid text overlay) - im…
archmoj Sep 20, 2019
35ea4b3
correct current path function - reuse listPath in helpers i.e. to hav…
archmoj Sep 20, 2019
1de80d9
add mock to test with and without values - and pathbar fade with depth
archmoj Sep 20, 2019
e7e22c6
texttemplate and hovertemplate fixes
archmoj Sep 20, 2019
6130f96
rename key to
archmoj Sep 20, 2019
e3a8e91
check for falsy 0 numbers when searching for level and ids
archmoj Sep 20, 2019
14674d6
handle hover edge cases
archmoj Sep 20, 2019
01c5f68
use onPathbar everywhere referring to points on pathbar - this make l…
archmoj Sep 20, 2019
4c7a092
revisit marker opacities: no opacity with colorscale, now having
archmoj Sep 21, 2019
1860826
improve sunburst texttemplate tests - drop the comment for a fixed tr…
archmoj Sep 22, 2019
d2b07b5
improve mocks to show various percentage values and colorscale using …
archmoj Sep 22, 2019
1fdf574
correct displaying percentages
archmoj Sep 23, 2019
3004a9a
fix more percentage edge cases - handle branchvalues remainder
archmoj Sep 23, 2019
4d790a1
correct texttemplate and hover
archmoj Sep 23, 2019
1c6e47b
dont touch pt.parent
archmoj Sep 23, 2019
e339bf5
Avoid using random ids in recording position of empty root
archmoj Sep 23, 2019
1309bfd
improve treemap jasmine tests
archmoj Sep 23, 2019
217c0f4
revisit sunburst and treemap percentages
archmoj Sep 24, 2019
8416883
rework working with parent references in hover and plot
archmoj Sep 24, 2019
c918da9
Reference fixups
archmoj Sep 25, 2019
7d67c7a
try to make parent getter more consistent by using pt.data.data.pid i…
archmoj Sep 25, 2019
f8cef49
add tolerance to tween tests
etpinard Sep 25, 2019
3c12bf6
remove unnecessary check for NaNs
archmoj Sep 25, 2019
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
11 changes: 3 additions & 8 deletions src/traces/sunburst/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,14 @@ module.exports = {
CLICK_TRANSITION_EASING: 'linear',
eventDataKeys: [
// string
'currentPath',
'root',
'entry',
// 'parent', // no need to add parent here which is added somewhere else!
'currentPath',
// no need to add 'parent' here

// percentages i.e. ratios
'percentRoot',
'percentEntry',
'percentParent',

// sums
'sumRoot',
'sumEntry',
'sumParent'
'percentParent'
]
};
19 changes: 12 additions & 7 deletions src/traces/sunburst/fx.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ module.exports = function attachFxHandlers(sliceTop, entry, gd, cd, opts) {
var ptNumber = cdi.i;
var isRoot = helpers.isHierarchyRoot(pt);
var isEntry = helpers.isEntry(pt);
var parent = isEntry ? pt._parent : pt.parent;
var parentLabel = !parent ? '' : (parent.data && parent.data.data) ? parent.data.data.label : parent.label;

var _cast = function(astr) {
return Lib.castOption(traceNow, ptNumber, astr);
Expand All @@ -72,7 +74,10 @@ module.exports = function attachFxHandlers(sliceTop, entry, gd, cd, opts) {
var parts = [];
var thisText = [];
var hasFlag = function(flag) { return parts.indexOf(flag) !== -1; };
var getVal = function(d) { return d.hasOwnProperty('v') ? d.v : d.value; };
var getVal = function(d) {
if(d.hasOwnProperty('hierarchy')) return d.hierarchy.value;
return d.hasOwnProperty('v') ? d.v : d.value;
};

if(hoverinfo) {
parts = hoverinfo === 'all' ?
Expand Down Expand Up @@ -105,18 +110,18 @@ module.exports = function attachFxHandlers(sliceTop, entry, gd, cd, opts) {

var val = getVal(cdi);

var ref2 = isEntry ? pt._parent : pt.parent;
if(ref2 && getVal(ref2)) {
var ref2 = parent;
if(ref2) {
hoverPt.percentParent = pt.percentParent = val / getVal(ref2);
hoverPt.parent = pt.parentString = helpers.getLabelString(isEntry ? ref2.label : ref2.data.data.label);
hoverPt.parent = pt.parent = helpers.getLabelString(parentLabel);
if(hasFlag('percent parent')) {
tx = helpers.formatPercent(hoverPt.percentParent, separators) + ' of ' + hoverPt.parent;
insertPercent();
}
}

var ref1 = entry;
if(ref1 && getVal(ref1)) {
if(ref1) {
hoverPt.percentEntry = pt.percentEntry = val / getVal(ref1);
hoverPt.entry = pt.entry = helpers.getLabelString(ref1.data.data.label);
if(hasFlag('percent entry') && !isRoot && !pt.onPathbar) {
Expand All @@ -126,7 +131,7 @@ module.exports = function attachFxHandlers(sliceTop, entry, gd, cd, opts) {
}

var ref0 = hierarchy;
if(ref0 && getVal(ref0)) {
if(ref0) {
hoverPt.percentRoot = pt.percentRoot = val / getVal(ref0);
hoverPt.root = pt.root = helpers.getLabelString(ref0.data.data.label);
if(hasFlag('percent root') && !isRoot) {
Expand Down Expand Up @@ -310,7 +315,7 @@ function makeEventData(pt, trace, keys) {
if(key in pt) out[key] = pt[key];
}
// handle special case of parent
if('parentString' in pt) out.parent = pt.parentString;
if('parent' in pt) out.parent = pt.parent;

appendArrayPointValue(out, trace, cdi.i);

Expand Down
4 changes: 2 additions & 2 deletions src/traces/sunburst/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ var setCursor = require('../../lib/setcursor');
var pieHelpers = require('../pie/helpers');

function labelStr(label) {
return (label || label === 0) ? label + '' : '';
return (label || label === 0) ? String(label) : '';
}

exports.findEntryWithLevel = function(hierarchy, level) {
Expand Down Expand Up @@ -151,7 +151,7 @@ exports.getLabelStr = function(label) {

exports.getLabelString = function(label) { // used in hover to reference to the "root"
var str = exports.getLabelStr(label);
return str ? str : '"root"';
return str === '' ? '"root"' : str;
};

exports.listPath = function(d, keyStr) {
Expand Down
43 changes: 30 additions & 13 deletions src/traces/sunburst/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,10 +487,31 @@ exports.formatSliceLabel = function(pt, entry, trace, cd, fullLayout) {
var cd0 = cd[0];
var cdi = pt.data.data;
var hierarchy = cd0.hierarchy;
var isRoot = helpers.isHierarchyRoot(pt);
var isEntry = helpers.isEntry(pt);
var parent = isEntry ? pt._parent : pt.parent;

var ref;
var getLabel = function(d) {
if(d.hasOwnProperty('hierarchy')) return d.hierarchy.label;
if(!(d.data && d.data.data)) return '';
var id = helpers.getPtId(d);
for(var q = 0; q < cd.length; q++) {
if(cd[q].label === id) {
return id;
}
}
return '';
};

var getVal = function(d) {
if(d.hasOwnProperty('hierarchy')) return d.hierarchy.value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does d get a hierarchy key?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here:

entry._parent = (numAncestors ?
ancestorNodes[numAncestors - 1] :
hierarchy // parent of root is itself
).data;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in 8416883.

return d.hasOwnProperty('v') ? d.v : d.value;
};

var calcPercent = function() {
return (trace.branchvalues ? cdi.v : cdi.value) / (ref.value || ref.v);
var result = (trace.branchvalues ? cdi.v : cdi.value) / getVal(ref);
return isFinite(result) ? result : 1;
};

if(trace.type === 'treemap' && helpers.isHeader(pt, trace)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this if block. Header nodes don't appear to call formatSliceLabel from:

if(isHeader && noRoomForHeader) return;

Copy link
Contributor

Choose a reason for hiding this comment

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

... with the formatSliceLabel below:

if(isHeader && noRoomForHeader) return;
var tx = formatSliceLabel(pt, entry, trace, cd, fullLayout) || ' ';

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 call. Moved to treemap/draw_descendants.js in 8416883.

Expand All @@ -510,8 +531,6 @@ exports.formatSliceLabel = function(pt, entry, trace, cd, fullLayout) {
var hasFlag = function(flag) { return parts.indexOf(flag) !== -1; };
var thisText = [];
var tx;
var parent = helpers.isEntry(pt) ? pt._parent : pt.parent;
var isRoot = helpers.isHierarchyRoot(pt);

if(hasFlag('label') && cdi.label) {
thisText.push(cdi.label);
Expand Down Expand Up @@ -580,28 +599,26 @@ exports.formatSliceLabel = function(pt, entry, trace, cd, fullLayout) {

obj.currentPath = helpers.getPath(pt.data);

if(pt.parent) {
ref = pt.parent;
obj.percentParent = calcPercent();
obj.percentParentLabel = helpers.formatPercent(
obj.percentParent, separators
);
obj.parent = ref.data.data.label;
}
ref = parent;
obj.percentParent = calcPercent();
obj.percentParentLabel = helpers.formatPercent(
obj.percentParent, separators
);
obj.parent = helpers.getLabelString(getLabel(ref));

ref = entry;
obj.percentEntry = calcPercent();
obj.percentEntryLabel = helpers.formatPercent(
obj.percentEntry, separators
);
obj.entry = ref.data.data.label;
obj.entry = helpers.getLabelString(getLabel(ref));

ref = hierarchy;
obj.percentRoot = calcPercent();
obj.percentRootLabel = helpers.formatPercent(
obj.percentRoot, separators
);
obj.root = ref.data.data.label;
obj.root = helpers.getLabelString(getLabel(ref));

if(cdi.hasOwnProperty('color')) {
obj.color = cdi.color;
Expand Down
11 changes: 3 additions & 8 deletions src/traces/treemap/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,15 @@ module.exports = {
CLICK_TRANSITION_EASING: 'poly',
eventDataKeys: [
// string
'currentPath',
'root',
'entry',
// 'parent', // no need to add parent here which is added somewhere else!
'currentPath',
// no need to add 'parent' here

// percentages i.e. ratios
'percentRoot',
'percentEntry',
'percentParent',

// sums
'sumRoot',
'sumEntry',
'sumParent'
'percentParent'
],
gapWithPathbar: 1 // i.e. one pixel
};
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.
17 changes: 4 additions & 13 deletions test/jasmine/tests/sunburst_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ describe('Test sunburst hover:', function() {
curveNumber: 0,
pointNumber: 0,
label: 'Eve',
parent: ''
parent: '"root"'
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look right. parents[0] is '' in the input trace, so I think we should honour that in the event data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK if we return an empty string here?

return str === '' ? '"root"' : str;

Copy link
Contributor

Choose a reason for hiding this comment

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

What are the side-effects?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simply getting blank string (nothing) referring to the dummy root label in hover; which I think is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 8416883.

}
}
}]
Expand Down Expand Up @@ -1344,10 +1344,7 @@ describe('Test sunburst texttemplate without `values` should work:', function()
['text: %{text}', ['text: sixty-five', 'text: fourteen', 'text: twelve', 'text: ten', 'text: two', 'text: six', 'text: six', 'text: one', 'text: four']],
['%{percentRoot} of %{root}', ['100% of Eve', '33% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve']],
['%{percentEntry} of %{entry}', ['100% of Eve', '33% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve']],
['%{percentParent} of %{parent}', [
'%{percentParent} of %{parent}', // TODO: what should be printed for the parent of root?
'100% of Seth', '33% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '50% of Seth', '100% of Awan'
]],
['%{percentParent} of %{parent}', ['100% of "root"', '100% of Seth', '33% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '17% of Eve', '50% of Seth', '100% of Awan']],
[
[
'label: %{label}',
Expand Down Expand Up @@ -1390,10 +1387,7 @@ describe('Test sunburst texttemplate with *total* `values` should work:', functi
['text: %{text}', ['text: sixty-five', 'text: fourteen', 'text: twelve', 'text: ten', 'text: two', 'text: six', 'text: six', 'text: one', 'text: four']],
['%{percentRoot} of %{root}', ['100% of Eve', '22% of Eve', '18% of Eve', '9% of Eve', '9% of Eve', '6% of Eve', '15% of Eve', '3% of Eve', '2% of Eve']],
['%{percentEntry} of %{entry}', ['100% of Eve', '22% of Eve', '18% of Eve', '9% of Eve', '9% of Eve', '6% of Eve', '15% of Eve', '3% of Eve', '2% of Eve']],
['%{percentParent} of %{parent}', [
'%{percentParent} of %{parent}', // TODO: what should be printed for the parent of root?
'22% of Eve', '18% of Eve', '9% of Eve', '9% of Eve', '6% of Eve', '83% of Seth', '17% of Seth', '17% of Awan'
]],
['%{percentParent} of %{parent}', ['100% of "root"', '22% of Eve', '18% of Eve', '9% of Eve', '9% of Eve', '6% of Eve', '83% of Seth', '17% of Seth', '17% of Awan']],
[
[
'label: %{label}',
Expand Down Expand Up @@ -1436,10 +1430,7 @@ describe('Test sunburst texttemplate with *remainder* `values` should work:', fu
['text: %{text}', ['text: sixty-five', 'text: fourteen', 'text: twelve', 'text: ten', 'text: two', 'text: six', 'text: six', 'text: one', 'text: four']],
['%{percentRoot} of %{root}', ['54% of Eve', '10% of Eve', '12% of Eve', '5% of Eve', '5% of Eve', '3% of Eve', '8% of Eve', '2% of Eve', '1% of Eve']],
['%{percentEntry} of %{entry}', ['54% of Eve', '10% of Eve', '12% of Eve', '5% of Eve', '5% of Eve', '3% of Eve', '8% of Eve', '2% of Eve', '1% of Eve']],
['%{percentParent} of %{parent}', [
'%{percentParent} of %{parent}', // TODO: what should be printed for the parent of root?
'10% of Eve', '12% of Eve', '5% of Eve', '5% of Eve', '3% of Eve', '42% of Seth', '8% of Seth', '14% of Awan'
]],
['%{percentParent} of %{parent}', ['100% of "root"', '10% of Eve', '12% of Eve', '5% of Eve', '5% of Eve', '3% of Eve', '42% of Seth', '8% of Seth', '14% of Awan']],
[
[
'label: %{label}',
Expand Down
2 changes: 1 addition & 1 deletion test/jasmine/tests/treemap_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ describe('Test treemap hover:', function() {
curveNumber: 0,
pointNumber: 0,
label: 'Eve',
parent: ''
parent: '"root"'
}
}
}]
Expand Down