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.

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

Jump to bottom

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

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 all 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
36 changes: 28 additions & 8 deletions src/traces/sankey/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,13 @@
'use strict';

var fontAttrs = require('../../plots/font_attributes');
var plotAttrs = require('../../plots/attributes');
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;

module.exports = overrideAll({
hoverinfo: extendFlat({}, plotAttrs.hoverinfo, {
flags: ['label', 'text', 'value', 'percent', 'name'],
}),
hoverlabel: fxAttrs.hoverlabel, // needs editType override

var attrs = module.exports = overrideAll({
domain: domainAttrs({name: 'sankey', trace: true}),

orientation: {
Expand Down Expand Up @@ -127,6 +120,18 @@ module.exports = overrideAll({
role: 'style',
description: 'Sets the thickness (in px) of the `nodes`.'
},
hoverinfo: {
valType: 'enumerated',
values: ['all', 'none', 'skip'],
dflt: 'all',
role: 'info',
description: [
'Determines which trace information appear when hovering nodes.',
'If `none` or `skip` are set, no information is displayed upon hovering.',
'But, if `none` is set, click and hover events are still fired.'
].join(' ')
},
hoverlabel: fxAttrs.hoverlabel, // needs editType override,
description: 'The nodes of the Sankey plot.'
},

Expand Down Expand Up @@ -185,6 +190,21 @@ module.exports = overrideAll({
role: 'info',
description: 'A numeric value representing the flow volume value.'
},
hoverinfo: {
valType: 'enumerated',
values: ['all', 'none', 'skip'],
dflt: 'all',
role: 'info',
description: [
'Determines which trace information appear when hovering links.',
'If `none` or `skip` are set, no information is displayed upon hovering.',
'But, if `none` is set, click and hover events are still fired.'
].join(' ')
},
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!

attrs.hoverlabel = undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

But wait, trace hoverlabel was working before this. Hmm, I'd call this a breaking change.

Copy link
Contributor

Choose a reason for hiding this comment

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

... I think we should either make node/link hoverlabel inherit from trace hoverlabel or add some cleanData logic.

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.

@etpinard It's great that you noticed that change and I agree this should be handled better!

FYI, having a hover(label|info) in each node and link was the fruit of discussions with @alexcjohnson. In the future, we could add a hovertemplate on each to provide control over their content. That's one way of dealing with point 2 of this comment #3126 (comment) which we can discuss about later on :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah indeed - good catch @etpinard - I'd go for:

make node/link hoverlabel inherit from trace hoverlabel

That would be more friendly for the (probably most common) case that users want node and link hover labels styled the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

... and maybe we should have a trace hoverinfo that acts similarly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

hoverinfo is trickier - could be nice to inherit skip/none, but when we get around to supporting a real flaglist, links and nodes will have completely different options.

Copy link
Contributor

Choose a reason for hiding this comment

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

e.g. trace.hoverinfo: 'none' would be equivalent to node.hoverinfo: 'none' + link.hoverinfo: 'none'.

Not all the flags to be implemented in #3097 should be available from the trace hoverinfo, but I think setting trace-wide 'none' and 'skip' could be useful.

52 changes: 35 additions & 17 deletions src/traces/sankey/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,55 @@ var attributes = require('./attributes');
var Color = require('../../components/color');
var tinycolor = require('tinycolor2');
var handleDomainDefaults = require('../../plots/domain').defaults;
var handleHoverLabelDefaults = require('../../components/fx/hoverlabel_defaults');
var Template = require('../../plot_api/plot_template');

module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
function coerce(attr, dflt) {
return Lib.coerce(traceIn, traceOut, attributes, attr, dflt);
}

coerce('node.label');
coerce('node.pad');
coerce('node.thickness');
coerce('node.line.color');
coerce('node.line.width');
// node attributes
var nodeIn = traceIn.node, nodeOut = Template.newContainer(traceOut, 'node');
function coerceNode(attr, dflt) {
return Lib.coerce(nodeIn, nodeOut, attributes.node, attr, dflt);
}
coerceNode('label');
coerceNode('pad');
coerceNode('thickness');
coerceNode('line.color');
coerceNode('line.width');
coerceNode('hoverinfo');
handleHoverLabelDefaults(nodeIn, nodeOut, coerceNode, layout.hoverlabel);

var colors = layout.colorway;

var defaultNodePalette = function(i) {return colors[i % colors.length];};

coerce('node.color', traceOut.node.label.map(function(d, i) {
coerceNode('color', nodeOut.label.map(function(d, i) {
return Color.addOpacity(defaultNodePalette(i), 0.8);
}));

coerce('link.label');
coerce('link.source');
coerce('link.target');
coerce('link.value');
coerce('link.line.color');
coerce('link.line.width');

coerce('link.color', traceOut.link.value.map(function() {
return tinycolor(layout.paper_bgcolor).getLuminance() < 0.333 ?
'rgba(255, 255, 255, 0.6)' :
'rgba(0, 0, 0, 0.2)';
// link attributes
var linkIn = traceIn.link, linkOut = Template.newContainer(traceOut, 'link');
function coerceLink(attr, dflt) {
return Lib.coerce(linkIn, linkOut, attributes.link, attr, dflt);
}
coerceLink('label');
coerceLink('source');
coerceLink('target');
coerceLink('value');
coerceLink('line.color');
coerceLink('line.width');
coerceLink('hoverinfo');
handleHoverLabelDefaults(linkIn, linkOut, coerceLink, layout.hoverlabel);

var defaultLinkColor = tinycolor(layout.paper_bgcolor).getLuminance() < 0.333 ?
'rgba(255, 255, 255, 0.6)' :
'rgba(0, 0, 0, 0.2)';

coerceLink('color', linkOut.value.map(function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

For the next time you're working in these files @antoinerg, here's Lib.repeat:

plotly.js/src/lib/index.js

Lines 167 to 181 in 7ae6fd6

/**
* create an array of length 'cnt' filled with 'v' at all indices
*
* @param {any} v
* @param {number} cnt
* @return {array}
*/
lib.repeat = function(v, cnt) {
var out = new Array(cnt);
for(var i = 0; i < cnt; i++) {
out[i] = v;
}
return out;
};

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 to know about the existence Lib.repeat! I just mindlessly reused the .map iterator that was already there! I should know better, for loops are much faster than map 🐎 !

return defaultLinkColor;
}));

handleDomainDefaults(traceOut, layout, coerce);
Expand Down
68 changes: 40 additions & 28 deletions src/traces/sankey/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,13 @@ module.exports = function plot(gd, calcData) {
var linkHover = function(element, d, sankey) {
if(gd._fullLayout.hovermode === false) return;
d3.select(element).call(linkHoveredStyle.bind(0, d, sankey, true));
gd.emit('plotly_hover', {
event: d3.event,
points: [d.link]
});
if(d.link.trace.link.hoverinfo !== 'skip') {
gd.emit('plotly_hover', {
event: d3.event,
points: [d.link]
});
}

};

var sourceLabel = _(gd, 'source:') + ' ';
Expand All @@ -145,7 +148,8 @@ module.exports = function plot(gd, calcData) {

var linkHoverFollow = function(element, d) {
if(gd._fullLayout.hovermode === false) return;
var trace = d.link.trace;
var obj = d.link.trace.link;
if(obj.hoverinfo === 'none' || obj.hoverinfo === 'skip') return;
var rootBBox = gd._fullLayout._paperdiv.node().getBoundingClientRect();
var boundingBox = element.getBoundingClientRect();
var hoverCenterX = boundingBox.left + boundingBox.width / 2;
Expand All @@ -160,11 +164,11 @@ module.exports = function plot(gd, calcData) {
sourceLabel + d.link.source.label,
targetLabel + d.link.target.label
].filter(renderableValuePresent).join('<br>'),
color: castHoverOption(trace, 'bgcolor') || Color.addOpacity(d.tinyColorHue, 1),
borderColor: castHoverOption(trace, 'bordercolor'),
fontFamily: castHoverOption(trace, 'font.family'),
fontSize: castHoverOption(trace, 'font.size'),
fontColor: castHoverOption(trace, 'font.color'),
color: castHoverOption(obj, 'bgcolor') || Color.addOpacity(d.tinyColorHue, 1),
borderColor: castHoverOption(obj, 'bordercolor'),
fontFamily: castHoverOption(obj, 'font.family'),
fontSize: castHoverOption(obj, 'font.size'),
fontColor: castHoverOption(obj, 'font.color'),
idealAlign: d3.event.x < hoverCenterX ? 'right' : 'left'
}, {
container: fullLayout._hoverlayer.node(),
Expand All @@ -179,10 +183,12 @@ module.exports = function plot(gd, calcData) {
var linkUnhover = function(element, d, sankey) {
if(gd._fullLayout.hovermode === false) return;
d3.select(element).call(linkNonHoveredStyle.bind(0, d, sankey, true));
gd.emit('plotly_unhover', {
event: d3.event,
points: [d.link]
});
if(d.link.trace.link.hoverinfo !== 'skip') {
gd.emit('plotly_unhover', {
event: d3.event,
points: [d.link]
});
}

Fx.loneUnhover(fullLayout._hoverlayer.node());
};
Expand All @@ -198,15 +204,19 @@ module.exports = function plot(gd, calcData) {
var nodeHover = function(element, d, sankey) {
if(gd._fullLayout.hovermode === false) return;
d3.select(element).call(nodeHoveredStyle, d, sankey);
gd.emit('plotly_hover', {
event: d3.event,
points: [d.node]
});
if(d.node.trace.node.hoverinfo !== 'skip') {
gd.emit('plotly_hover', {
event: d3.event,
points: [d.node]
});
}
};

var nodeHoverFollow = function(element, d) {
if(gd._fullLayout.hovermode === false) return;
var trace = d.node.trace;

var obj = d.node.trace.node;
if(obj.hoverinfo === 'none' || obj.hoverinfo === 'skip') return;
var nodeRect = d3.select(element).select('.' + cn.nodeRect);
var rootBBox = gd._fullLayout._paperdiv.node().getBoundingClientRect();
var boundingBox = nodeRect.node().getBoundingClientRect();
Expand All @@ -224,11 +234,11 @@ module.exports = function plot(gd, calcData) {
incomingLabel + d.node.targetLinks.length,
outgoingLabel + d.node.sourceLinks.length
].filter(renderableValuePresent).join('<br>'),
color: castHoverOption(trace, 'bgcolor') || d.tinyColorHue,
borderColor: castHoverOption(trace, 'bordercolor'),
fontFamily: castHoverOption(trace, 'font.family'),
fontSize: castHoverOption(trace, 'font.size'),
fontColor: castHoverOption(trace, 'font.color'),
color: castHoverOption(obj, 'bgcolor') || d.tinyColorHue,
borderColor: castHoverOption(obj, 'bordercolor'),
fontFamily: castHoverOption(obj, 'font.family'),
fontSize: castHoverOption(obj, 'font.size'),
fontColor: castHoverOption(obj, 'font.color'),
idealAlign: 'left'
}, {
container: fullLayout._hoverlayer.node(),
Expand All @@ -243,10 +253,12 @@ module.exports = function plot(gd, calcData) {
var nodeUnhover = function(element, d, sankey) {
if(gd._fullLayout.hovermode === false) return;
d3.select(element).call(nodeNonHoveredStyle, d, sankey);
gd.emit('plotly_unhover', {
event: d3.event,
points: [d.node]
});
if(d.node.trace.node.hoverinfo !== 'skip') {
gd.emit('plotly_unhover', {
event: d3.event,
points: [d.node]
});
}

Fx.loneUnhover(fullLayout._hoverlayer.node());
};
Expand Down
Loading