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

sankey hover improvements #3150

Merged
merged 3 commits into from
Oct 25, 2018
Merged

sankey hover improvements #3150

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27f379f
sankey: introduce trace-level `hover(info|lable)` and improve tests
antoinerg Oct 24, 2018
4453455
replace `.map` call by `Lib.repeat`
antoinerg Oct 24, 2018
6ab718d
remove `hoverinfo` flags that aren't implemented
antoinerg Oct 25, 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
8 changes: 4 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 @@ -9,13 +9,16 @@
'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 overrideAll = require('../../plot_api/edit_types').overrideAll;

var attrs = module.exports = overrideAll({
module.exports = overrideAll({
hoverinfo: plotAttrs.hoverinfo,
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to extend the default hoverinfo declaration

plotly.js/src/plots/attributes.js

Lines 117 to 130 in 36d9fb4

hoverinfo: {
valType: 'flaglist',
role: 'info',
flags: ['x', 'y', 'z', 'text', 'name'],
extras: ['all', 'none', 'skip'],
arrayOk: true,
dflt: 'all',
editType: 'none',
description: [
'Determines which trace information appear on hover.',
'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(' ')
},

with new flags. I'm thinking 'none', 'skip' and 'all'(the default) which I think should resolve @alexcjohnson 's concern #3096 (comment)

We should probably have a special description too to explain how trace hoverinfo can propagate to the node/link hoverinfo.

Copy link
Contributor Author

@antoinerg antoinerg Oct 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@etpinard I'm having trouble setting hoverinfo.flags to an empty array [ ]. Running Plotly.PlotSchema.get().traces.sankey.attributes.hoverinfo always return 5 flags.

The following doesn't work:

var attrs = module.exports = overrideAll({
    hoverinfo: plotAttrs.hoverinfo,
    ...
})
attrs.hoverinfo.flags = []

The following also doesn't work:

var attrs = module.exports = overrideAll({
    hoverinfo: extendFlat({}, plotAttrs.hoverinfo, {
        flags: ['random']
    }),
    ...
})

although it replaces the first array element with 'random'.

I'm confused any help/pointer would be greatly appreciated 😕

Copy link
Collaborator

Choose a reason for hiding this comment

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

see #3058 (comment) - it's just a problem with the schema, the modules themselves are fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

the modules themselves are fine.

Right, gd._fullData[0]._module.attributes.hoverinfo.flags should have the new list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you guys for the quick reply! But won't this be an issue for the automatically generated documentation?

Copy link
Collaborator

Choose a reason for hiding this comment

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

sure will 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

and already is - feel like tackling #3058, or at least that part of it?

Copy link
Contributor

@etpinard etpinard Oct 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

That, or remove hoverinfo from the global (plots/attributes.js) trace attributes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexcjohnson I will look into #3058! I actually wanted to improve on plot.ly/javascript/reference so I guess that would be a good start for me since they're related :)

hoverlabel: fxAttrs.hoverlabel,
domain: domainAttrs({name: 'sankey', trace: true}),

orientation: {
Expand Down Expand Up @@ -205,6 +208,3 @@ var attrs = module.exports = overrideAll({
description: 'The links of the Sankey plot.'
}
}, 'calc', 'nested');
// hide unsupported top-level properties from plot-schema
attrs.hoverinfo = undefined;
attrs.hoverlabel = undefined;
14 changes: 7 additions & 7 deletions src/traces/sankey/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
return Lib.coerce(traceIn, traceOut, attributes, attr, dflt);
}

var hoverlabelDefault = Lib.extendDeep(layout.hoverlabel, traceIn.hoverlabel);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice touch 🖌️


// node attributes
var nodeIn = traceIn.node, nodeOut = Template.newContainer(traceOut, 'node');
function coerceNode(attr, dflt) {
Expand All @@ -31,8 +33,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
coerceNode('thickness');
coerceNode('line.color');
coerceNode('line.width');
coerceNode('hoverinfo');
handleHoverLabelDefaults(nodeIn, nodeOut, coerceNode, layout.hoverlabel);
coerceNode('hoverinfo', traceIn.hoverinfo);
handleHoverLabelDefaults(nodeIn, nodeOut, coerceNode, hoverlabelDefault);

var colors = layout.colorway;

Expand All @@ -53,16 +55,14 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
coerceLink('value');
coerceLink('line.color');
coerceLink('line.width');
coerceLink('hoverinfo');
handleHoverLabelDefaults(linkIn, linkOut, coerceLink, layout.hoverlabel);
coerceLink('hoverinfo', traceIn.hoverinfo);
handleHoverLabelDefaults(linkIn, linkOut, coerceLink, hoverlabelDefault);

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() {
return defaultLinkColor;
}));
coerceLink('color', Lib.repeat(defaultLinkColor, linkOut.value.length));
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks 🍻


handleDomainDefaults(traceOut, layout, coerce);

Expand Down
136 changes: 94 additions & 42 deletions test/jasmine/tests/sankey_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,9 @@ describe('sankey tests', function() {
['source: Solid', 'target: Industry', '46TWh'],
['rgb(0, 0, 96)', 'rgb(255, 255, 255)', 13, 'Arial', 'rgb(255, 255, 255)']
);

})
// Test layout.hoverlabel
.then(function() {
return Plotly.relayout(gd, 'hoverlabel.font.family', 'Roboto');
})
.then(function() {
Expand All @@ -434,16 +436,45 @@ describe('sankey tests', function() {
['source: Solid', 'target: Industry', '46TWh'],
['rgb(0, 0, 96)', 'rgb(255, 255, 255)', 13, 'Roboto', 'rgb(255, 255, 255)']
);
})
// Test trace-level hoverlabel
.then(function() {
return Plotly.restyle(gd, {
'hoverlabel.bgcolor': 'blue',
'hoverlabel.bordercolor': 'red',
'hoverlabel.font.size': 22,
'hoverlabel.font.color': 'magenta'
});
})
.then(function() {
_hover(404, 302);

assertLabel(
['Solid', 'incoming flow count: 4', 'outgoing flow count: 3', '447TWh'],
['rgb(0, 0, 255)', 'rgb(255, 0, 0)', 22, 'Roboto', 'rgb(255, 0, 255)']
);
})
.then(function() {
_hover(450, 300);

assertLabel(
['source: Solid', 'target: Industry', '46TWh'],
['rgb(0, 0, 255)', 'rgb(255, 0, 0)', 22, 'Roboto', 'rgb(255, 0, 255)']
);
})
// Test (node|link).hoverlabel
.then(function() {
return Plotly.restyle(gd, {
'node.hoverlabel.bgcolor': 'red',
'node.hoverlabel.bordercolor': 'blue',
'node.hoverlabel.font.size': 20,
'node.hoverlabel.font.color': 'black',
'node.hoverlabel.font.family': 'Roboto',
'link.hoverlabel.bgcolor': 'yellow',
'link.hoverlabel.bordercolor': 'magenta',
'link.hoverlabel.font.size': 18,
'link.hoverlabel.font.color': 'green'
'link.hoverlabel.font.color': 'green',
'link.hoverlabel.font.family': 'Roboto'
});
})
.then(function() {
Expand Down Expand Up @@ -556,49 +587,59 @@ describe('sankey tests', function() {
.then(done);
});

it('should not show node labels if node.hoverinfo is none', function(done) {
var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);
['skip', 'none'].forEach(function(hoverinfoFlag) {
it('should not show node labels if node.hoverinfo is ' + hoverinfoFlag, function(done) {
var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);

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

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

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

it('should not show node labels if node.hoverinfo is skip', function(done) {
var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);
['skip', 'none'].forEach(function(hoverinfoFlag) {
it('should not show labels if trace hoverinfo is ' + hoverinfoFlag + ' and (node|link).hoverinfo is undefined', function(done) {
var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);

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

it('should not show link labels if link.hoverinfo is skip', function(done) {
Expand Down Expand Up @@ -748,7 +789,7 @@ describe('sankey tests', function() {
};
}

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

Plotly.plot(gd, fig)
Expand All @@ -759,17 +800,28 @@ describe('sankey tests', function() {
.then(done);
});

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

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

['node', 'link'].forEach(function(obj) {
it('should not output hover/unhover event data when ' + obj + '.hoverinfo is skip', function(done) {
var fig = Lib.extendDeep({}, mock);

Plotly.plot(gd, fig)
.then(function() { return Plotly.restyle(gd, obj + '.hoverinfo', 'skip'); })
.then(assertNoHoverEvents(obj))
.catch(failTest)
.then(done);
});
});
});
});

Expand Down