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 percentage flags to control funnel hoverinfo and hovertemplete #3958

Merged
merged 3 commits into from
Jun 13, 2019
Merged

Implement percentage flags to control funnel hoverinfo and hovertemplete #3958

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
fbc03a3
fix issue 3954 - implement hoverinfo and hovertemplate for funnel per…
archmoj Jun 12, 2019
1701325
revise funnel hovertemplate and hoverinfo based on comments
archmoj Jun 13, 2019
ea54842
clean up fx helpers from funnel info - cleanup custom_assertions from…
archmoj Jun 13, 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
3 changes: 3 additions & 0 deletions src/components/fx/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ exports.makeEventData = function makeEventData(pt, trace, cd) {
if(pt.xa) out.xaxis = pt.xa;
if(pt.ya) out.yaxis = pt.ya;
if(pt.zLabelVal !== undefined) out.z = pt.zLabelVal;
if(pt.percentInitial !== undefined) out.z = pt.percentInitial;
if(pt.percentPrevious !== undefined) out.z = pt.percentPrevious;
if(pt.percentTotal !== undefined) out.z = pt.percentTotal;
etpinard marked this conversation as resolved.
Show resolved Hide resolved
}

exports.appendArrayPointValue(out, trace, pointNumber);
Expand Down
11 changes: 10 additions & 1 deletion src/traces/funnel/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@

var barAttrs = require('../bar/attributes');
var lineAttrs = require('../scatter/attributes').line;
var plotAttrs = require('../../plots/attributes');
var hovertemplateAttrs = require('../../components/fx/hovertemplate_attributes');
var constants = require('./constants');
var extendFlat = require('../../lib/extend').extendFlat;
var Color = require('../../components/color');

Expand All @@ -22,7 +25,13 @@ module.exports = {
dy: barAttrs.dy,

hovertext: barAttrs.hovertext,
hovertemplate: barAttrs.hovertemplate,
hovertemplate: hovertemplateAttrs({}, {
keys: constants.eventDataKeys
}),

hoverinfo: extendFlat({}, plotAttrs.hoverinfo, {
flags: ['name', 'x', 'y', 'text', 'percent initial', 'percent previous', 'percent total']
}),

textinfo: {
valType: 'flaglist',
Expand Down
17 changes: 17 additions & 0 deletions src/traces/funnel/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Copyright 2012-2019, Plotly, Inc.
* All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

module.exports = {
eventDataKeys: [
'percentInitial',
'percentPrevious',
'percentTotal'
]
};
25 changes: 25 additions & 0 deletions src/traces/funnel/event_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Copyright 2012-2019, Plotly, Inc.
* All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

module.exports = function eventData(out, pt /* , trace, cd, pointNumber */) {
// standard cartesian event data
out.x = 'xVal' in pt ? pt.xVal : pt.x;
out.y = 'yVal' in pt ? pt.yVal : pt.y;

// for funnel
if('percentInitial' in pt) out.percentInitial = pt.percentInitial;
Copy link
Contributor

Choose a reason for hiding this comment

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

here out.percentInitial should be the raw number (i.e. unformatted).

Copy link
Member

Choose a reason for hiding this comment

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

is percentInitial between 0-100 or between 0-1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now with formatting it is between 0-100. The raw ratios are between 0-1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI - in case of pie the pt.percent contains the fraction not the percent.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is what pie does:

plotly.js/src/traces/pie/plot.js

Lines 365 to 367 in 3e3b22d

pt.percent = pt.v / cd0.vTotal;
pt.percentLabel = helpers.formatPiePercent(pt.percent, separators);
if(hoverinfo && hoverinfo.indexOf('percent') !== -1) text.push(pt.percentLabel);

that is:

  • percent is in fact a fraction
  • percentLabel is a percentage and it is how hoverinfo flag 'percent' shows up in the hover label. It is also how hovertemplate: '%{percent}' shows up by default.

For example,

Plotly.newPlot(gd, [{
  type: 'pie',
  labels: ['a', 'b', 'c'],
  values: [1, 2, 3]
}])

shows 16.7%, 33.3% and 50% in the hover labels when 'percent' is part of the hoverinfo OR when hovertemplate: '%{percent}' (i.e. it uses percentLabel behind the scenes).

But if hovertemplate: '%{y:.3f}' we get 0.167, 0.333 and 0.500 in the hover labels (i.e. we format the fraction behind the scenes).

if('percentPrevious' in pt) out.percentPrevious = pt.percentPrevious;
if('percentTotal' in pt) out.percentTotal = pt.percentTotal;
etpinard marked this conversation as resolved.
Show resolved Hide resolved

if(pt.xa) out.xaxis = pt.xa;
if(pt.ya) out.yaxis = pt.ya;

return out;
};
36 changes: 28 additions & 8 deletions src/traces/funnel/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,36 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
var di = cd[index];

var sizeLetter = isHorizontal ? 'x' : 'y';

point[sizeLetter + 'LabelVal'] = di.s;

// display ratio to initial value
point.extraText = [
formatPercent(di.begR, 1) + ' of initial',
formatPercent(di.difR, 1) + ' of previous',
formatPercent(di.sumR, 1) + ' of total'
].join('<br>');
// TODO: Should we use pieHelpers.formatPieValue instead ?
point.percentInitial = di.begR;
point.percentInitialLabel = formatPercent(di.begR, 1);

point.percentPrevious = di.difR;
point.percentPreviousLabel = formatPercent(di.difR, 1);

point.percentTotal = di.sumR;
point.percentTotalLabel = formatPercent(di.sumR, 1);

var hoverinfo = di.hi || trace.hoverinfo;
var text = [];
if(hoverinfo && hoverinfo !== 'none' && hoverinfo !== 'skip') {
var isAll = (hoverinfo === 'all');
var parts = hoverinfo.split('+');

var hasFlag = function(flag) { return isAll || parts.indexOf(flag) !== -1; };

if(hasFlag('percent initial')) {
text.push(point.percentInitialLabel + ' of initial');
}
if(hasFlag('percent previous')) {
text.push(point.percentPreviousLabel + ' of previous');
}
if(hasFlag('percent total')) {
text.push(point.percentTotalLabel + ' of total');
}
}
point.extraText = text.join('<br>');

point.color = getTraceColor(trace, di);

Expand Down
2 changes: 2 additions & 0 deletions src/traces/funnel/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ module.exports = {
plot: require('./plot'),
style: require('./style').style,
hoverPoints: require('./hover'),
eventData: require('./event_data'),

selectPoints: require('../bar/select'),

moduleType: 'trace',
Expand Down
15 changes: 10 additions & 5 deletions test/jasmine/assets/custom_assertions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ function count(selector) {
* - vOrder {array of number}
* - hOrder {array of number}
* - isRotated {boolean}
* - isEmpty {boolean}
* @param {string} msg
*/
exports.assertHoverLabelContent = function(expectation, msg) {
Expand Down Expand Up @@ -192,11 +193,15 @@ exports.assertHoverLabelContent = function(expectation, msg) {
}
});
} else {
if(expectation.nums) {
fail(ptMsg + ': expecting *nums* labels, did not find any.');
}
if(expectation.name) {
fail(ptMsg + ': expecting *nums* labels, did not find any.');
if(expectation.isEmpty) {
return true;
} else {
if(expectation.nums) {
fail(ptMsg + ': expecting *nums* labels, did not find any.');
}
if(expectation.name) {
fail(ptMsg + ': expecting *nums* labels, did not find any.');
}
}
}

Expand Down
71 changes: 68 additions & 3 deletions test/jasmine/tests/funnel_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1326,14 +1326,76 @@ describe('funnel hover', function() {
.then(done);
});

it('should turn off percentages with hoveinfo none or skip', function(done) {
gd = createGraphDiv();

var mock = Lib.extendDeep({}, require('@mocks/text_chart_arrays'));
mock.data.forEach(function(t, i) {
t.type = 'funnel';
t.orientation = 'v';
if(i === 0) {
t.hoverinfo = 'none';
} else {
t.hoverinfo = 'skip';
}
});

function _hover() {
var evt = { xpx: 125, ypx: 150 };
Fx.hover('graph', evt, 'xy');
}

Plotly.plot(gd, mock)
.then(_hover)
.then(function() {
assertHoverLabelContent({
isEmpty: true
etpinard marked this conversation as resolved.
Show resolved Hide resolved
});
})
.catch(failTest)
.then(done);
});

it('should turn on percentages with hoveinfo all', function(done) {
gd = createGraphDiv();

var mock = Lib.extendDeep({}, require('@mocks/text_chart_arrays'));
mock.data.forEach(function(t) {
t.type = 'funnel';
t.orientation = 'v';
t.hoverinfo = 'all';
});

function _hover() {
var evt = { xpx: 125, ypx: 150 };
Fx.hover('graph', evt, 'xy');
}

Plotly.plot(gd, mock)
.then(_hover)
.then(function() {
assertHoverLabelContent({
nums: [
'1\nHover text A\n100% of initial\n100% of previous\n33.3% of total',
'2\nHover text G\n100% of initial\n100% of previous\n33.3% of total',
'1.5\na (hover)\n100% of initial\n100% of previous\n33.3% of total'
],
name: ['Lines, Marke...', 'Lines and Text', 'missing text'],
axis: '0'
});
})
.catch(failTest)
.then(done);
});

it('should use hovertemplate if specified', function(done) {
gd = createGraphDiv();

var mock = Lib.extendDeep({}, require('@mocks/text_chart_arrays'));
mock.data.forEach(function(t) {
t.type = 'funnel';
t.orientation = 'v';
t.hovertemplate = '%{y}<extra></extra>';
t.hovertemplate = 'Value: %{y}<br>Total percentage: %{percentTotal}<br>Initial percentage: %{percentInitial}<br>Previous percentage: %{percentPrevious}<extra></extra>';
});

function _hover() {
Expand All @@ -1345,11 +1407,14 @@ describe('funnel hover', function() {
.then(_hover)
.then(function() {
assertHoverLabelContent({
nums: ['1', '2', '1.5'],
nums: [
'Value: 1\nTotal percentage: 33.3%\nInitial percentage: 100%\nPrevious percentage: 100%',
'Value: 2\nTotal percentage: 33.3%\nInitial percentage: 100%\nPrevious percentage: 100%',
'Value: 1.5\nTotal percentage: 33.3%\nInitial percentage: 100%\nPrevious percentage: 100%'
etpinard marked this conversation as resolved.
Show resolved Hide resolved
],
name: ['', '', ''],
axis: '0'
});
// return Plotly.restyle(gd, 'text', ['APPLE', 'BANANA', 'ORANGE']);
})
.catch(failTest)
.then(done);
Expand Down