Skip to content

implement hoverinfo 'none' and 'skip' in sankey #3096

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.

Sign up for GitHub

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 18 commits into from
Oct 23, 2018
Merged

implement hoverinfo 'none' and 'skip' in sankey #3096

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
69c11fe
implement hoverinfo 'none' and 'skip' in sankey
antoinerg Oct 11, 2018
880973a
sankey node and link each have their own hover attributes
antoinerg Oct 12, 2018
d247b8b
sankey remove unused value
antoinerg Oct 12, 2018
1919b72
fix sankey attributes definition
antoinerg Oct 12, 2018
e942bc5
fix sankey attribute to pass test-image
antoinerg Oct 12, 2018
1813d6c
sankey hide unsupported top-level properties from plot-schema
antoinerg Oct 12, 2018
9e4f1a0
hide unsupported properties in sankey more safely
antoinerg Oct 12, 2018
706e7ae
fix sankey attributes and add better test
antoinerg Oct 15, 2018
7c24d29
fix sankey attributes to pass plotschema test
antoinerg Oct 15, 2018
53d7ce3
fix lint issue
antoinerg Oct 15, 2018
87c30fe
fix sankey attribute key valtype -> valType
antoinerg Oct 15, 2018
7a32036
update description of hoverinfo in sankey
antoinerg Oct 15, 2018
b436003
remove editType on hoverinfo since it's overriden
antoinerg Oct 15, 2018
fe7ab49
sankey update defaults
antoinerg Oct 15, 2018
644c4e3
sankey improve attribute coercion
antoinerg Oct 15, 2018
ccaef04
test sankey hoverlabels defined in template
antoinerg Oct 16, 2018
dc8ba12
sankey move defaultLinkColor outside for loop to speed up coerce
antoinerg Oct 23, 2018
ea73c64
fix typo: defautLinkColor -> defaultLinkColor
antoinerg Oct 23, 2018
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
21 changes: 17 additions & 4 deletions src/traces/sankey/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ var colorAttrs = require('../../components/color/attributes');
var fxAttrs = require('../../components/fx/attributes');
var domainAttrs = require('../../plots/domain').attributes;

var extendFlat = require('../../lib/extend').extendFlat;
var overrideAll = require('../../plot_api/edit_types').overrideAll;

var attrs = module.exports = overrideAll({
Expand Down Expand Up @@ -122,7 +121,14 @@ var attrs = module.exports = overrideAll({
role: 'style',
description: 'Sets the thickness (in px) of the `nodes`.'
},
hoverinfo: extendFlat({}, plotAttrs.hoverinfo, {flags: []}),
hoverinfo: {
valtype: 'enumerated',
values: ['all', 'none', 'skip'],
dflt: 'all',
role: 'info',
editType: 'none',
description: plotAttrs.hoverinfo.description
},
hoverlabel: fxAttrs.hoverlabel, // needs editType override,
description: 'The nodes of the Sankey plot.'
},
Expand Down Expand Up @@ -182,10 +188,17 @@ var attrs = module.exports = overrideAll({
role: 'info',
description: 'A numeric value representing the flow volume value.'
},
hoverinfo: extendFlat({}, plotAttrs.hoverinfo, {flags: []}),
hoverinfo: {
valtype: 'enumerated',
values: ['all', 'none', 'skip'],
dflt: 'all',
role: 'info',
editType: 'none',
description: plotAttrs.hoverinfo.description
},
hoverlabel: fxAttrs.hoverlabel, // needs editType override,
description: 'The links of the Sankey plot.'
},
}
}, 'calc', 'nested');
// hide unsupported top-level properties from plot-schema
attrs.hoverinfo = undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh so, trace-level hoverinfo is no longer available for sankey traces? Luckily it never worked so this isn't breaking change.

Does this PR close #3097 ?

Copy link
Contributor Author

@antoinerg antoinerg Oct 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

It wouldn't close it.

Issue #3097 is about supporting hoverinfo options other than the basic skip and none. Although the current doc say that we support many:

Any combination of "label", "text", "value", "percent", "name" joined with a "+" OR "all" or "none" or "skip"

none actually worked except all. This PR is solely about adding support for skip and none. Those two are arguably more urgent to support (ie. whether to hide the hoverlabels and whether to emit hover events at the same time).

Copy link
Contributor

Choose a reason for hiding this comment

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

Good to know. Thanks!

Expand Down
12 changes: 3 additions & 9 deletions src/traces/sankey/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ var attributes = require('./attributes');
var Color = require('../../components/color');
var tinycolor = require('tinycolor2');
var handleDomainDefaults = require('../../plots/domain').defaults;
var Registry = require('../../registry');
var handleFxDefaults = require('../../components/fx/defaults');

module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
function coerce(attr, dflt) {
Expand All @@ -26,10 +26,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
coerce('node.line.color');
coerce('node.line.width');
coerce('node.hoverinfo');
Registry.getComponentMethod(
'fx',
'supplyDefaults'
)(traceIn.node, traceOut.node, defaultColor, layout);
handleFxDefaults(traceIn.node, traceOut.node, defaultColor, layout);

var colors = layout.colorway;

Expand All @@ -46,10 +43,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
coerce('link.line.color');
coerce('link.line.width');
coerce('link.hoverinfo');
Registry.getComponentMethod(
'fx',
'supplyDefaults'
)(traceIn.link, traceOut.link, defaultColor, layout);
handleFxDefaults(traceIn.link, traceOut.link, defaultColor, layout);

coerce('link.color', traceOut.link.value.map(function() {
return tinycolor(layout.paper_bgcolor).getLuminance() < 0.333 ?
Expand Down
53 changes: 36 additions & 17 deletions test/jasmine/tests/sankey_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,10 +440,10 @@ describe('sankey tests', function() {
'node.hoverlabel.bordercolor': 'blue',
'node.hoverlabel.font.size': 20,
'node.hoverlabel.font.color': 'black',
'link.hoverlabel.bgcolor': 'red',
'link.hoverlabel.bordercolor': 'blue',
'link.hoverlabel.font.size': 20,
'link.hoverlabel.font.color': 'black'
'link.hoverlabel.bgcolor': 'yellow',
'link.hoverlabel.bordercolor': 'magenta',
'link.hoverlabel.font.size': 18,
'link.hoverlabel.font.color': 'green'
});
})
.then(function() {
Expand All @@ -459,7 +459,7 @@ describe('sankey tests', function() {

assertLabel(
['source: Solid', 'target: Industry', '46TWh'],
['rgb(255, 0, 0)', 'rgb(0, 0, 255)', 20, 'Roboto', 'rgb(0, 0, 0)']
['rgb(255, 255, 0)', 'rgb(255, 0, 255)', 18, 'Roboto', 'rgb(0, 128, 0)']
);
})
.catch(failTest)
Expand Down Expand Up @@ -547,6 +547,21 @@ describe('sankey tests', function() {
.catch(failTest)
.then(done);
});

it('should not show link labels if link.hoverinfo is skip', function(done) {
var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);

Plotly.plot(gd, mockCopy).then(function() {
return Plotly.restyle(gd, 'link.hoverinfo', 'skip');
})
.then(function() {
_hover(link[0], link[1]);
assertNoLabel();
})
.catch(failTest)
.then(done);
});
});

describe('Test hover/click event data:', function() {
Expand Down Expand Up @@ -667,24 +682,26 @@ describe('sankey tests', function() {
});

function assertNoHoverEvents(type) {
return Promise.resolve()
.then(function() { return _hover(type); })
.then(failTest).catch(function(err) {
expect(err).toBe('plotly_hover did not get called!');
})
.then(function() { return _unhover(type); })
.then(failTest).catch(function(err) {
expect(err).toBe('plotly_unhover did not get called!');
});
return function() {
return Promise.resolve()
.then(function() { return _hover(type); })
.then(failTest).catch(function(err) {
expect(err).toBe('plotly_hover did not get called!');
})
.then(function() { return _unhover(type); })
.then(failTest).catch(function(err) {
expect(err).toBe('plotly_unhover did not get called!');
});
};
}

it('should not output hover/unhover event data when hovermoder is false', function(done) {
var fig = Lib.extendDeep({}, mock);

Plotly.plot(gd, fig)
.then(function() { return Plotly.relayout(gd, 'hovermode', false); })
.then(function() { return assertNoHoverEvents('node');})
.then(function() { return assertNoHoverEvents('link');})
.then(assertNoHoverEvents('node'))
.then(assertNoHoverEvents('link'))
.catch(failTest)
.then(done);
});
Expand All @@ -694,7 +711,9 @@ describe('sankey tests', function() {

Plotly.plot(gd, fig)
.then(function() { return Plotly.restyle(gd, 'link.hoverinfo', 'skip'); })
.then(function() { return assertNoHoverEvents('link');})
.then(assertNoHoverEvents('link'))
.then(function() { return Plotly.restyle(gd, 'node.hoverinfo', 'skip'); })
.then(assertNoHoverEvents('node'))
.catch(failTest)
.then(done);
});
Expand Down